-
Notifications
You must be signed in to change notification settings - Fork 21
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
Events db backend #230
Events db backend #230
Conversation
* Ingest events into DB * Update tests * Update tests * Fix tests * Ignore ingestion when events are empty
* Ingest events into DB * Update tests * Update tests * Fix tests * Ignore ingestion when events are empty * Add getEvents backed by DB * Refactor getEvents to call fetch events from db * Remove in-memory events store * Make use of cursor in pagination and combine filters * Update scan function logic in order to test * Move NewTestDb util * Trim events db and remove eventTypes in SELECT query as event types are not indexed * Introduce cursor range and add logs for latency * remove event memory store * remove event memory store from latest merge * Fix lint issues part 1 * Fix lint issues part 2 * Fix more lint and add ledger range code for events * Update eventHandler mock * Fix 2 major errors in tests: nil pointer reference and unknown hash * Fix contract Id filter logic and add cursor set to avoid duplicates * Validate requested start ledger with stored ledger range * Fix lint error part 4 * Add migration for events table * Fix lint error part 5 * Fix lint error part 6 * Address review comments pt1 * Make contract id a blob type * Remove events package and move cursor.go to db package * Fix lint error part 6 * Fix lint error part 7 * Optimize migration code * Introduce endLedger and remove Cursor id from schema * Use LedgerReader to get Ledger Range in events * Fix lint errors * Add benchmark for testing various load parameter * update benchmark * Comment benchmark events * Reduce allocs pt1 * Benchmark with 30 million events * Refactor getEvents to backed only by Events table * change test db path * Use Binary encoding for saving events into DB * Correct number of topics * reduce events in benchmark so that tests run * update events schema * Fix topic count * Fix fetch query to not stop if null topic * Fix lint pt1 * Fix trimEvents and lint errors * update log info * Fix more lint errors * Fix more lint errors pt 11 * Fix format in error * Fix linter error pt 12 * Add nolint for GetEvents as a temp fix. * Add events table migration (#262) * Fix migrations - 1 * Make migrations sequential - 1 * Make migrations sequential - 2 * Fix failing unittest - 1 * Fix linting errors - 1 * Fix failing integration test - 1 * Remove %w from Fatal strings * refactor migrationApplierFactoryF * Add ledger seq to fatal error string * Add comments - 1 * Fix - 1 * Optimise migrations - 1 * Optimise migrations - 2 * Optimise migrations - 3 * Fix linting - 1 * Fix linting - 2 * Remove dupicate latest ledger fetch code * Rollback db in daemon instead of migration * Remove unused constant * Remove unused constant - 2 * Add rollback() statement * Small change * Abstract transaction and rollback management inside migration code * Fix failing unittest * Address review comments * Store binary of topics instead of string * Unify min/max topic count in event.go * Address review comments for event Types and fix unit tests * cleanup * Fix linter errors for one last time --------- Co-authored-by: Aditya Vyas <[email protected]>
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.
Great job!!
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.
So stoked to see this getting closer to the finish line 👏
Buuuuut here's another comment dump 😝
@Shaptic I am just converting all fmt.Errorf to errors.New in PR. linter is just very annoying particularly with this rule. |
@psheth9 ugh, sorry for the extra load. I thought adding my diff suggestions would be linter-fighting-free but I guess not. You can just ignore my suggestions re: that too if you'd like - whatever is easier. |
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.
Oh yeah also pls add a detailed CHANGELOG entry 🙏 esp. around migration time and storage requirements.
This PR consolidates all the events related changes into single one !!
Add Events Schema: #192
Refactor getEvents #215
Fix migration failures (#262)
Overall design:
eventXDR
along with metadata like contract_ids, topics etc..contract_ids
andtopics
from all the filters for a given request and pass it along to DB (to save multiple DB calls)Other Refactor:
endLedgerSeq
Data Flow:
Design Docs:
Eng doc
Benchmarking doc here
Parent Epic #115
Fixes: #190