-
Notifications
You must be signed in to change notification settings - Fork 17
Add and expose latest version as watermark #116
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
Conversation
Codecov Report❌ Patch coverage is
❌ Your patch status has failed because the patch coverage (38.19%) is below the target coverage (70.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #116 +/- ##
==========================================
+ Coverage 40.00% 40.06% +0.06%
==========================================
Files 59 59
Lines 7510 7533 +23
==========================================
+ Hits 3004 3018 +14
- Misses 4161 4168 +7
- Partials 345 347 +2
🚀 New features to boost your workflow:
|
|
|
||
| for _, kvPair := range cs.Changeset.Pairs { | ||
| if kvPair.Value == nil { | ||
| if err := b.Delete(cs.Name, kvPair.Key); err != nil { |
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.
As a general practice i recommend against overriding error like this, specially in cases like this since the scope of err is tighly limited to the if block. Ditto for next if statement.
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.
Fixed, thanks for pointing that out!
ss/pebbledb/db.go
Outdated
|
|
||
| return b.Write() | ||
| // Update latest version | ||
| err = b.Write() |
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.
Ditto re keeping the scope of err variable tight.
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.
Fixed
| func (db *Database) GetLatestMigratedModule() (string, error) { | ||
| //TODO implement me | ||
| panic("implement me") | ||
| } |
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.
Thinking out loud/unrelated to this PR: when was the last time we used the sqlite backing store? If heavily unused, my vote is to remove it entirely.
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.
Yea agreed, we don’t use it
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.
Gonna do that in a separate PR
* main: Bump version 0.0.55 (#120)
## Describe your changes and provide context A common workflow that users do is to query the latest height and then query information about that height such as events/logs. However, since we have an option to do async writes for receipts, the events/logs may not be available yet for that height, causing the query to fail. In general, we want all of our heights outputted by our RPC endpoints to be aligned with each other. We have introduced the concept of a watermark (sei-protocol/sei-db#116) across Tendermint block store, SS Store, and receipt store. Pulling this watermark across all 3 dbs should allow the evmrpc to output the minimum height available across these dbs for a latest height. This will ensure that all data for a given height outputted by our RPCs is available across all our endpoints. This PR introduces a WatermarkManager that manages the heights of these DBs, which the evmrpc endpoints can use to get the latest heights that are ready. ## Testing performed to validate your change unit tests + manual testing
## Describe your changes and provide context This PR add an in memory latestVersion for each database, change interface to handle getLatestVersion always from in memory to avoid any I/O read. Add logic to retrieve latestVersion during database initialization. ## Testing performed to validate your change Added and fix unit test for latestVersion
Describe your changes and provide context
This PR add an in memory latestVersion for each database, change interface to handle getLatestVersion always from in memory to avoid any I/O read.
Add logic to retrieve latestVersion during database initialization.
Testing performed to validate your change
Added and fix unit test for latestVersion