-
Notifications
You must be signed in to change notification settings - Fork 202
[Access] Refactor the collection indexing and syncing #8114
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
[Access] Refactor the collection indexing and syncing #8114
Conversation
| DefaultMissingCollsRequestInterval = 1 * time.Minute | ||
| DefaultMissingCollectionRequestInterval = 1 * time.Minute | ||
|
|
||
| // DefaultMissingCollsForBlockThreshold is the threshold number of blocks with missing collections |
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.
removed this because we had redundant checks. if there are MissingCollsForBlockThreshold blocks with missing collections, then the last full block is at least that many blocks behind the last finalized. Updated to only check the difference between last finalized and last full
|
|
||
| initialCatchupComplete := false | ||
| for { | ||
| err := s.requestMissingCollections(ctx, !initialCatchupComplete) |
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.
Changes looks good.
I have one concern for syncer's requestMissingCollections :
The syncing asks the requester to request missing collections, after a few seconds, the execution data indexer indexed the execution data, then the syncer is able to index the collections, and the indexer will move forward the last full block height. However, the requester didn't know about the fact that the collections have been received, so I think over time, it will build up a long list of zombie collections in the fetch list, we need to check how the requester deals with it, I'm afraid those zombie collections will affect the actual missing collection requests from being sent.
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.
that's a good point. the requester should eventually download the collections, but it is wasted effort. it may be worth adding a timeout for AN collection syncing
…v0.43 [Access] Refactor the collection indexing and syncing
Expands on #8108
The original implementation had 2 issues discovered when we deployed it to live nodes:
Addressed 1 by making the syncer block when indexing from execution data.
Addressed 2 by refactoring the syncer to process each block sequentially.