-
Notifications
You must be signed in to change notification settings - Fork 758
fix(spice): fix memory leak in executor pending receipts #14789
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
There is a race condition in which when block is being processed incoming receipts would be stored as pending but may never be processed. If block was already processed we would try applying it and would process receipts before detecting that it's already applied.
darioush
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
approving to unblock
| simulate_single_outgoing_message(&mut actors, &message); | ||
| } | ||
| record_endorsements(&mut actors, &first_block); | ||
| let second_block = produce_block(&mut actors, &first_block); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the role of second_block in this test?
| // Some additional receipts can arrive after we have already started executing the block. (For | ||
| // example when we are tracking several shards for which we aren't chunk producer or we get | ||
| // receipts from different peers with different commitments in which case data distributor | ||
| // wouldn't know which is right.) In this case we want to make sure we process pending | ||
| // receipts to make sure don't leak memory by storing unverified receipts indefinitely. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems to me that we could have receipts arrive after block in the case where the chunk producer is also requesting the missing data (i.e., receive receipts from pull & push path).
I didn't understand the part about tracking several shards.
Also to check my understanding, receiving data with different commitments, would not currently be possible since the distributor_actor has waiting_on_data and recently_decoded_data, but in the future it will be possible?
There is a race condition in which when block is being processed incoming receipts would be stored as pending but may never be processed. If block was already processed we would try applying it and would process receipts before detecting that it's already applied.