-
Notifications
You must be signed in to change notification settings - Fork 196
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
feat(store-sync): wait for idle after each chunk of logs in a block #2254
Conversation
🦋 Changeset detectedLatest commit: a0cd05c The changes in this PR will be included in the next version bump. This PR includes changesets to release 30 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Wonder if we should wait on every block or some number of iterations. Just thinking about how hydrating thousands of blocks will take significantly longer to complete if we have to wait for idle on each block. |
@@ -145,11 +145,16 @@ export async function createStoreSync<TConfig extends StoreConfig = StoreConfig> | |||
await storageAdapter({ blockNumber, logs: chunk }); | |||
onProgress?.({ | |||
step: SyncStep.SNAPSHOT, | |||
percentage: ((i + chunk.length) / chunks.length) * 100, | |||
percentage: (i / chunks.length) * 100, |
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.
I think both of these code snippets might be wrong. The original intent of this was to have the percentage represent the completed state of the chunk (since we called storageAdapter
above this).
I think the deleted snippet was meant to be ((i * chunkSize + chunk.length) / logs.length) * 100
And the new snippet is changing this to the "start" of the chunk (rather than "end"), so this probably should be ((i + 1) / chunks.length) * 100
waiting on #2264 just so we can have a test that this does what we expect |
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.
I added a spec and tested on Sky Strife testnet, works great 👍
Screen.Recording.2024-02-16.at.11.24.43.mov
…2254) Co-authored-by: Kevin Ingersoll <[email protected]> Co-authored-by: yonada <[email protected]>
don't ask me why or how, but adding
sleep(1)
after calling thestorageAdapter
made hydration updates show up in the client. i was going crazy trying to figure out why i wasn't receiving them in the main thread, but then remembered having a similar problem in DF with how async calls were being handled. if someone actually knows the mechanism that is being abused here lmk and we can come up with a less obscure solution.also once i started receiving updates i realized the % calculation was wrong, so fixed that too.
getting these updates is important so i can properly benchmark how long hydration takes on the sky strife end. my manual calculations on my pc are ~5 seconds.