refactor: separate writeBlockInputToDb into parallel block and column writes - #8974
Merged
Conversation
… writes Add IDataColumnsInput interface and extract writeDataColumnsToDb() as a standalone exported function accepting a narrow sub-interface of IBlockInput. Block+blobs (pre-fulu) and data columns (fulu+) are now written in parallel, enabling reuse of writeDataColumnsToDb() for the Gloas fork later. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refactors the block input writing process to the database, separating the writing of block and blobs from data columns. This change prepares the codebase for the integration of gloas by introducing a reusable interface and simplifying function signatures, ultimately improving code maintainability and reusability across different forks. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
…Input For Gloas (BlockInputNoData), the execution payload and columns arrive separately after the beacon block. The serialized cache must not be cleared when the block is persisted — it remains available for writeDataColumnsToDb when the payload arrives later. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request refactors the writeBlockInputToDb function to separate the writing of block and blob data from data columns, allowing for parallel execution and reusability across different forks. It introduces a new IDataColumnsInput interface for handling data columns and simplifies the writeDataColumnsToDb function. The changes aim to improve code modularity and prepare for future extensions like Gloas.
Contributor
Performance Report🚀🚀 Significant benchmark improvement detected
Full benchmark results
|
twoeths
marked this pull request as ready for review
March 3, 2026 06:58
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8cef8fd2c9
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Comment on lines
+128
to
+129
| if (!isBlockInputNoData(blockInput)) { | ||
| this.serializedCache.clear(); |
There was a problem hiding this comment.
On post-gloas blocks (BlockInputNoData), this new guard skips serializedCache.clear(), but there is currently no alternative clear path in beacon-node, so the cache is left to GC timing instead of being force-pruned after persistence. In sustained sync on a gloas network, serialized block/sidecar objects can accumulate and increase memory pressure (the prior behavior cleared on every persisted block), so this should either clear here with a safe handoff strategy or ensure a guaranteed cleanup in the payload-envelope persistence path.
Useful? React with 👍 / 👎.
ensi321
reviewed
Mar 4, 2026
| slot, | ||
| root: blockRootHex, | ||
| dataColumnSidecars: dataColumnSidecars.length, | ||
| custodyColumns: custodyColumns.length, |
Member
There was a problem hiding this comment.
Looks like numBlobs is removed from this log because it's not in IDataColumnsInput. Is it useful to include it?
Member
Author
There was a problem hiding this comment.
yes it's helpful for debug, will get it from the 1st DataColumnSidecar
Member
Author
There was a problem hiding this comment.
I also added numBlobs to other logs to make it consistent
| await blockInput.waitForAllData(BLOB_AVAILABILITY_TIMEOUT); | ||
| } | ||
| const blobSidecars = blockInput.getBlobs(); | ||
| await this.db.blobSidecars.add({blockRoot, slot, blobSidecars}); |
Member
There was a problem hiding this comment.
In the old code, we have
fnPromises.push(this.db.block.putBinary(this.db.block.getId(block), blockBytes));
...
fnPromises.push(this.db.blobSidecars.add({blockRoot, slot: block.message.slot, blobSidecars}));
...
await Promise.all(fnPromises);
So block and blob write happen in parallel in the old code. Here we do it sequentially. Is this intended?
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## unstable #8974 +/- ##
============================================
+ Coverage 52.30% 52.35% +0.04%
============================================
Files 848 848
Lines 62689 62626 -63
Branches 4615 4615
============================================
- Hits 32790 32788 -2
+ Misses 29833 29772 -61
Partials 66 66 🚀 New features to boost your workflow:
|
lodekeeper
pushed a commit
to lodekeeper/lodestar
that referenced
this pull request
Mar 13, 2026
… writes (ChainSafe#8974) **Motivation** - in preparation for [gloas](https://github.com/ChainSafe/lodestar/pull/8962/changes#diff-31556a76147b927f6429470ca658ba8ea013b8a869566247b9410f791988454fR32), columns and block could/should be written separately - the nice thing of having a `writeDataColumnsToDb()` function is gloas has to come up with the same interface, and reuse the code, especially `waitForComputedAllData()` to keep the same functionality we introduced in [allow block import after 64 columns](ChainSafe#8818) **Description** - separate`writeBlockInputToDb()` into `writeBlockAndBlobsToDb()` and `writeDataColumnsToDb()` - new `IDataColumnsInput` interface, gloas [PayloadEnvelopeInput](https://github.com/ChainSafe/lodestar/pull/8962/changes#diff-39f10cf452f670b2fe7018b493ee4c550bb6bf19d0eac2fed0a238ed7511cf36R54) is expected to implement this cc @ensi321 - only prune `this.serializedCache.clear();` for pre-gloas, need to call that after we write `PayloadEnvelopeInput` in gloas - simplify `writeDataColumnsToDb()` to accept a single `IDataColumnsInput` instead of an array because that's what the consumer need **AI Assistance Disclosure** 🤖 Generated with [Claude Code](https://claude.ai/code) --------- Co-authored-by: Tuyen Nguyen <twoeths@users.noreply.github.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> Co-authored-by: NC <17676176+ensi321@users.noreply.github.com>
Member
|
🎉 This PR is included in v1.41.0 🎉 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
writeDataColumnsToDb()function is gloas has to come up with the same interface, and reuse the code, especiallywaitForComputedAllData()to keep the same functionality we introduced in allow block import after 64 columnsDescription
writeBlockInputToDb()intowriteBlockAndBlobsToDb()andwriteDataColumnsToDb()IDataColumnsInputinterface, gloas PayloadEnvelopeInput is expected to implement this cc @ensi321this.serializedCache.clear();for pre-gloas, need to call that after we writePayloadEnvelopeInputin gloaswriteDataColumnsToDb()to accept a singleIDataColumnsInputinstead of an array because that's what the consumer needAI Assistance Disclosure
🤖 Generated with Claude Code