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

cometindex: 📇 add a minimal indexer prototype #4610

Merged
merged 15 commits into from
Jun 24, 2024
Merged

Conversation

cratelyn
Copy link
Contributor

Describe your changes

this is a continuation of #4573.

Adds the skeleton of a minimal but extensible indexer. The indexer consumes events from a source (raw event) database and writes to a destination database with compiled / processed tables. The indexer is a library that other binaries import and slot their indexes into.

The cometindex library is intended to be generic over any cometbft events. The pindexer binary is for penumbra specifically and we can add all our own code to it. This structure mirrors the existing distinction between comet (generic) and the app. The code is intended to be shaped so that it's easy for users to plug in additional indexes. In particular, all of the "meat" of the binary is in the cometindex::Indexer type, which builds an indexer and handles all arg parsing. This way, the external interface of the program, how it's invoked, etc, is the same no matter what app views are present, and orchestration code like containers doesn't need to be concerned with them.

Before merging:

  • Rename the Index trait to AppView (it's a view of some application data)
  • Rename the create_tables method to init_chain
  • Add polling for ingesting / processing new events after the initial catch-up (500ms?)
  • Make sure the pindexer AppView traits are public so that pindexer can be pulled in as a library
  • Add an extension trait in the pindexer crate that adds an Indexer::with_default_penumbra_app_views() method that hooks in all the AppView impls, so we can add to it as we fill in impls

Followup:

  • Change init_chain to take a serde_json::Value and add plumbing to the Indexer builder to take a genesis.json file in a standardized way so that every instance of the binary doesn't need to implement that -- ideally this is the exact genesis.json that was used by comet, and the plumbing in the Indexer builder pulls out the app_state key and saves it to be able to pass it into each AppView. Longer term we could explore other options but for now I think it's simplest if we just make it a required parameter every time.

Closes #4604

Checklist before requesting a review

  • If this code contains consensus-breaking changes, I have added the "consensus-breaking" label. Otherwise, I declare my belief that there are not consensus-breaking changes, for the following reason:

    New tool outside of consensus

@cratelyn
Copy link
Contributor Author

opened #4611 to track the follow-on work related to init_chain().

@cratelyn cratelyn marked this pull request as ready for review June 13, 2024 20:08
@cratelyn cratelyn requested a review from hdevalence June 13, 2024 20:08
@cratelyn
Copy link
Contributor Author

@hdevalence this is ready for review, when you are able to take a look

@aubrika aubrika self-requested a review June 24, 2024 18:18
Copy link
Contributor

@aubrika aubrika left a comment

Choose a reason for hiding this comment

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

LGTM!

@aubrika aubrika merged commit fa8535c into main Jun 24, 2024
13 checks passed
@aubrika aubrika deleted the kate/cometindex branch June 24, 2024 18:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-indexing Area: Relates to event indexing. C-enhancement Category: an enhancement to the codebase
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Deliver minimal indexer
3 participants