Skip to content
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

Introduce BlobsSidecar storage #6587

Merged
merged 9 commits into from
Dec 14, 2022
Merged

Conversation

tbenr
Copy link
Contributor

@tbenr tbenr commented Dec 12, 2022

Introduces two new columns:

  1. blobsSidecar column: store blobs for the entire lifecycle
    <Slot,BlockRoot> -> BlobsSidecar

  2. unconfirmed blobsSidecar column: tracks which blobsSidecar conteined in the previous column is yet to be confirmed (which happens when the block is fully validated and imported)
    <Slot,BlockRoot> -> Void

Allows blobs to be streamed ordered by slot (ascending)

Introduces two pruning methods.

  • One for unconfirmed blobs (will be used to prune unconfirmed blobs referring to finalized slots).
  • Another for all (confirmed and unconfirmed) to be used to prune blobs outside our availability window (MIN_EPOCHS_FOR_BLOBS_SIDECARS_REQUESTS)

Fixed Issue(s)

fixes #6586

Documentation

  • I thought about documentation and added the doc-change-required label to this PR if updates are required.

Changelog

  • I thought about adding a changelog entry, and added one if I deemed necessary.

@tbenr tbenr changed the title Introduce blobs storage Introduce BlobsSidecar storage Dec 12, 2022
Copy link
Contributor

@zilm13 zilm13 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some comments and question on usage

import tech.pegasys.teku.infrastructure.unsigned.UInt64;
import tech.pegasys.teku.spec.datastructures.blocks.SlotAndBlockRoot;

class SlotAndBlockRootKeySerializer implements KvStoreSerializer<SlotAndBlockRoot> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A bit unclear what does this mean. Maybe BigEndianSlotAndBlockRootSerializer?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand that core idea is to have a strream of it by slots, but it's not clear by this

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a comment on the class. I don't likeBigEndianSlotAndBlockRootSerializer :)


void removeBlobsSidecar(SlotAndBlockRoot slotAndBlockRoot);

void pruneOldestBlobsSidecar(UInt64 endSlot, int pruneLimit);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm confused with this interface. What is a direction? What is start slot? Why do we need pruneLimit?

Copy link
Contributor Author

@tbenr tbenr Dec 13, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it always start from the oldest, prunes up to endSlot but limits the pruning to maximum pruneLimit entries.
Naming endSlot differently might improve readability?


void confirmBlobsSidecar(SlotAndBlockRoot slotAndBlockRoot);

Optional<BlobsSidecar> getBlobsSidecar(SlotAndBlockRoot slotAndBlockRoot);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So to answer, for example, on BeaconBlockAndBlobsSidecarByRoot we will need

  1. Query DB for slot for each root
  2. Query unconfirmed blobsSidecar for each pair to avoid it
  3. Get BlobsSidecar for remaining pairs using this method

not sure that such complex flow is needed.
What are current cases we have with BlobsSidecar:
write:

  • gossip receive (coupled, goes to import)
  • sync receive (probably coupled too, in discussion, goes to import)

read:

  • BeaconBlockAndBlobsSidecarByRoot (root list)
  • BlobsSidecarsByRange (slot range)

delete:

  • fork pruning (drop BlobsSidecars from noncanonical chain)
  • every epoch remove 1 epoch blobsSidecars

Do I miss something or wrong with it? What potential future flows could we have with it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So to answer, for example, on BeaconBlockAndBlobsSidecarByRoot we will need

since we have to respond with a coupled block and blobs, we will start looking up blocks first. Once we have an existing blockRoot we can lookup in blobsSidecar without checking the unconfirmed column.

delete:

  • fork pruning (drop BlobsSidecars from noncanonical chain)
  • every epoch remove 1 epoch blobsSidecars

there is one more. The sequence will be:

  • prune all blobs that has not being confirmed (related to blocks that remain in pending\future until discarded - ie never actually imported (successfully or with error))
  • fork pruning (pruning blobs for noncanonical blocks up to the most recent finalized slot)
  • prune blobs outside the data availability window (regardless finalization or not, just to avoid db overload when chain is not finalizing)

@tbenr tbenr force-pushed the eip-4844-blobs-storage branch 2 times, most recently from e1f7621 to d0c2fea Compare December 13, 2022 21:51
Copy link
Contributor

@zilm13 zilm13 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@tbenr tbenr merged commit 61bb752 into Consensys:master Dec 14, 2022
@tbenr tbenr deleted the eip-4844-blobs-storage branch December 14, 2022 12:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

update Storage and KvStore classes to support BlobsSidecar lifecycle
2 participants