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

Events db backend #230

Merged
merged 17 commits into from
Aug 24, 2024
Merged

Events db backend #230

merged 17 commits into from
Aug 24, 2024

Conversation

psheth9
Copy link
Contributor

@psheth9 psheth9 commented Jun 20, 2024

This PR consolidates all the events related changes into single one !!

Add Events Schema: #192
Refactor getEvents #215
Fix migration failures (#262)

Overall design:

  • Ingest events into DB: store entire eventXDR along with metadata like contract_ids, topics etc..
  • Combine contract_ids and topics from all the filters for a given request and pass it along to DB (to save multiple DB calls)
  • Fetch events and apply filter again based on the request (on less data comparatively)

Other Refactor:

  • Remove in-memory events store
  • Introduce endLedgerSeq
  • Add migration related logic for events

Data Flow:

image

Design Docs:

Parent Epic #115
Fixes: #190

psheth9 and others added 6 commits June 18, 2024 10:12
* 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]>
@psheth9 psheth9 changed the title [Draft] Events db backend Events db backend Aug 20, 2024
@psheth9 psheth9 marked this pull request as ready for review August 21, 2024 17:59
@psheth9 psheth9 self-assigned this Aug 21, 2024
@psheth9 psheth9 added the api-change Any changes to existing API or addition of new API label Aug 21, 2024
Copy link
Contributor

@aditya1702 aditya1702 left a comment

Choose a reason for hiding this comment

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

Great job!!

Copy link
Contributor

@Shaptic Shaptic left a 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 😝

cmd/soroban-rpc/internal/db/db.go Show resolved Hide resolved
cmd/soroban-rpc/internal/db/event.go Outdated Show resolved Hide resolved
cmd/soroban-rpc/internal/db/event.go Outdated Show resolved Hide resolved
cmd/soroban-rpc/internal/db/event.go Outdated Show resolved Hide resolved
cmd/soroban-rpc/internal/db/event.go Outdated Show resolved Hide resolved
cmd/soroban-rpc/internal/methods/get_events.go Outdated Show resolved Hide resolved
cmd/soroban-rpc/internal/methods/get_events.go Outdated Show resolved Hide resolved
@psheth9
Copy link
Contributor Author

psheth9 commented Aug 21, 2024

  Running [/home/runner/golangci-lint-1.59.1-linux-amd64/golangci-lint run --new-from-patch=/tmp/tmp-4738-XJ8wnGKFOZhU/pull.patch --new=false --new-from-rev=] in [/home/runner/work/soroban-rpc/soroban-rpc] ...
  Error: cmd/soroban-rpc/internal/db/event.go:236:27: fmt.Errorf can be replaced with errors.New (perfsprint)
  		return errors.Join(err, fmt.Errorf("db read failed for requested parameter"))
  		                        ^
  Error: cmd/soroban-rpc/internal/db/event.go:260:28: fmt.Errorf can be replaced with errors.New (perfsprint)
  			return errors.Join(err, fmt.Errorf("failed to parse cursor"))
  			                        ^
  Error: cmd/soroban-rpc/internal/db/event.go:266:28: fmt.Errorf can be replaced with errors.New (perfsprint)
  			return errors.Join(err, fmt.Errorf("failed to decode event"))
  			                        ^

@Shaptic I am just converting all fmt.Errorf to errors.New in PR. linter is just very annoying particularly with this rule.

@Shaptic
Copy link
Contributor

Shaptic commented Aug 22, 2024

@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.

Copy link
Contributor

@Shaptic Shaptic left a 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.

CHANGELOG.md Show resolved Hide resolved
@psheth9 psheth9 merged commit dbad81b into main Aug 24, 2024
19 checks passed
@psheth9 psheth9 deleted the events-db-backend branch August 24, 2024 18:55
@psheth9 psheth9 mentioned this pull request Aug 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-change Any changes to existing API or addition of new API
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Refactor getEvents RPC to fetch events from DB
3 participants