-
Notifications
You must be signed in to change notification settings - Fork 878
migration routers #3336
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
Open
cody-littley
wants to merge
49
commits into
main
Choose a base branch
from
cjl/migration-integration
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
migration routers #3336
Changes from all commits
Commits
Show all changes
49 commits
Select commit
Hold shift + click to select a range
662b214
build routers
da2a545
remove dual write test
865ef84
remove split write tests
879dac7
incremental progress
6c4927f
expand route API
1b7c9ab
add helper method to build a route to a migration manager
af35c76
Merge branch 'cjl/expanded-route-api' into cjl/migration-integration
fa89b45
cleanup
75a3b7c
made suggested change
3710f59
Merge branch 'cjl/expanded-route-api' into cjl/migration-integration
e0744b9
revert lots of changes
94c5e57
test utilities
78eb3bb
incremental progress on tests
2a6c03e
test evm migration
8a303e9
determinism
3900541
cleanup
d9fefc8
test post evm migration
eee79af
added next test
0b3cc6e
all migrated but bank
4f140de
final test
175b0f5
add thread safe wrapper
65c31dd
various fixes
2fc730a
Merge branch 'main' into cjl/migration-integration
97d0acc
implement thread safe router
ecd5a92
Add router based implementation of kvstore
8c9961c
made suggested changes
d0aead6
Merge branch 'cjl/thread-safe-router' into cjl/migration-integration
ee043b8
remove unused methods
ddafa97
Merge branch 'main' into cjl/migration-integration
9449881
test framework for migration tests
34d0dbe
made suggested changes
2f50f90
Merge branch 'cjl/migration-test-framework' into cjl/migration-integr…
cc367cd
Merge branch 'main' into cjl/migration-integration
247308f
Add replacement APIs for methods we intend to deprecate.
976c934
Merge branch 'main' into cjl/replacement-apis
0939d8d
fix tests
68a9dc1
Merge branch 'main' into cjl/migration-integration
4265c58
remove ctx
17915ce
Constants for migration code
d548214
made suggested change
5c2b89d
made suggested change
5e8524a
Merge branch 'cjl/migration-constants' into cjl/migration-integration
a864d0f
fix merge issue
01e41e2
Merge branch 'main' into cjl/migration-integration
76bfdf4
Merge branch 'cjl/replacement-apis' into cjl/migration-integration
95e7e03
add routers for steady state operation
bc1842c
add duplicating router
fd86181
add router for dual write mode
009bf03
Merge branch 'main' into cjl/migration-integration
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,102 @@ | ||
| package migration | ||
|
|
||
| import ( | ||
| "fmt" | ||
|
|
||
| ics23 "github.com/confio/ics23/go" | ||
| "github.com/sei-protocol/sei-chain/sei-db/proto" | ||
| db "github.com/tendermint/tm-db" | ||
| ) | ||
|
|
||
| var _ Router = (*TestOnlyDualWriteRouter)(nil) | ||
|
|
||
| // A router that dual-writes traffic, sending each batch of changesets to both backends. Read | ||
| // requests, requests for proofs, and requests for iteration are not dual-written, and are instead | ||
| // served exclusively by the primary backend. | ||
| // | ||
| // CRITICAL: this is a test-only router and should never be deployed to production machines. | ||
| type TestOnlyDualWriteRouter struct { | ||
| primary *Route | ||
| secondary DBWriter | ||
| } | ||
|
|
||
| // Create a new test-only dual-write router. | ||
| // | ||
| // CRITICAL: this is a test-only router and should never be deployed to production machines. | ||
| func NewTestOnlyDualWriteRouter( | ||
| // Read, proof, and iteration traffic is served by this route, and writes are also sent here. | ||
| // Module names associated with this route are ignored; this route forwards all regardless of the module names. | ||
| primary *Route, | ||
| // Write traffic is dual-written and also sent here. Reads, proofs, and iteration are not sent here. | ||
| secondary DBWriter, | ||
| ) (*TestOnlyDualWriteRouter, error) { | ||
|
|
||
| if primary == nil { | ||
| return nil, fmt.Errorf("primary must not be nil") | ||
| } | ||
| if primary.proofBuilder == nil { | ||
| return nil, fmt.Errorf("primary proof builder must not be nil") | ||
| } | ||
| if primary.iteratorBuilder == nil { | ||
| return nil, fmt.Errorf("primary iterator builder must not be nil") | ||
| } | ||
| if secondary == nil { | ||
| return nil, fmt.Errorf("secondary must not be nil") | ||
| } | ||
|
|
||
| return &TestOnlyDualWriteRouter{primary: primary, secondary: secondary}, nil | ||
| } | ||
|
|
||
| func (t *TestOnlyDualWriteRouter) ApplyChangeSets(changesets []*proto.NamedChangeSet) error { | ||
| err := t.primary.writer(changesets) | ||
| if err != nil { | ||
| return fmt.Errorf("primary writer: %w", err) | ||
| } | ||
|
|
||
| err = t.secondary(changesets) | ||
| if err != nil { | ||
| return fmt.Errorf("secondary writer: %w", err) | ||
| } | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
| func (t *TestOnlyDualWriteRouter) GetProof(store string, key []byte) (*ics23.CommitmentProof, error) { | ||
| proof, err := t.primary.proofBuilder(store, key) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("primary proof builder: %w", err) | ||
| } | ||
| return proof, nil | ||
| } | ||
|
|
||
| func (t *TestOnlyDualWriteRouter) Iterator( | ||
| store string, | ||
| start []byte, | ||
| end []byte, | ||
| ascending bool, | ||
| ) (db.Iterator, error) { | ||
| iterator, err := t.primary.iteratorBuilder(store, start, end, ascending) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("primary iterator builder: %w", err) | ||
| } | ||
| return iterator, nil | ||
| } | ||
|
|
||
| func (t *TestOnlyDualWriteRouter) Read(store string, key []byte) ([]byte, bool, error) { | ||
| value, found, err := t.primary.reader(store, key) | ||
| if err != nil { | ||
| return nil, false, fmt.Errorf("primary reader: %w", err) | ||
| } | ||
| return value, found, nil | ||
| } | ||
|
|
||
| // BuildRoute returns a Route that dispatches the given module names to | ||
| // this DualWriteRouter. Reads, writes, iteration and proof requests | ||
| // for those modules will all flow through this dual-write router. | ||
| // | ||
| // Module names must be unique; NewRoute's validation rules apply. The | ||
| // returned Route may be passed to NewModuleRouter alongside other | ||
| // Routes to compose multi-database setups. | ||
| func (t *TestOnlyDualWriteRouter) BuildRoute(moduleNames ...string) (*Route, error) { // TODO LLM you need to write a unit test for this | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. stale TODO comment |
||
| return NewRoute(t.Read, t.ApplyChangeSets, t.Iterator, t.GetProof, moduleNames...) | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Stale LLM instruction left in source code
Low Severity
The comment
// TODO LLM you need to write a unit test for thisonBuildRouteis an accidentally committed prompt/instruction to an LLM. The unit tests it requests already exist extensively indual_write_router_test.go(13+ test functions coveringBuildRoute), making this TODO stale and confusing.Reviewed by Cursor Bugbot for commit 009bf03. Configure here.