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

Archive Node Online Migration #1863

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open

Archive Node Online Migration #1863

wants to merge 12 commits into from

Conversation

Kbhat1
Copy link
Contributor

@Kbhat1 Kbhat1 commented Sep 18, 2024

Describe your changes and provide context

Testing performed to validate your change

  • Verifying on migration node

tools/migration/ss/migrator.go Fixed Show fixed Hide fixed
tools/migration/ss/migrator.go Fixed Show fixed Hide fixed
return fmt.Errorf("failed to create iterator: %w", err)
}
defer itr.Close()

startTimeBatch := time.Now() // Measure time for every 10,000 iterations

Check warning

Code scanning / CodeQL

Calling the system time Warning

Calling the system time may be a possible source of non-determinism
tools/migration/ss/migrator.go Fixed Show fixed Hide fixed
go func() {
homeDir := cast.ToString(appOpts.Get(flags.FlagHome))
stateStore := app.GetStateStore()
latestVersion := rootmulti.GetLatestVersion(db)
Copy link
Contributor

Choose a reason for hiding this comment

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

This latest version would keep changing as we restart right? Is there any concern ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yea this will change to the migrate-height flag

Kbhat1 and others added 2 commits September 24, 2024 23:35
* Add QMS for online migration

* Fix lint

---------

Co-authored-by: kbhat1 <[email protected]>
cmd/seid/cmd/root.go Fixed Show fixed Hide fixed
Comment on lines +311 to +319
go func() {
homeDir := cast.ToString(appOpts.Get(flags.FlagHome))
stateStore := app.GetStateStore()
migrationHeight := cast.ToInt64(appOpts.Get("migrate-height"))
migrator := ss.NewMigrator(homeDir, db, stateStore)
if err := migrator.Migrate(migrationHeight, homeDir); err != nil {
panic(err)
}
}()

Check notice

Code scanning / CodeQL

Spawning a Go routine Note

Spawning a Go routine may be a possible source of non-determinism
Copy link

codecov bot commented Sep 25, 2024

Codecov Report

Attention: Patch coverage is 30.76923% with 18 lines in your changes missing coverage. Please review.

Project coverage is 61.29%. Comparing base (6c8b336) to head (a586dbc).
Report is 14 commits behind head on main.

Files with missing lines Patch % Lines
app/test_state_store.go 0.00% 12 Missing ⚠️
app/seidb.go 54.54% 4 Missing and 1 partial ⚠️
app/app.go 66.66% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1863      +/-   ##
==========================================
- Coverage   61.33%   61.29%   -0.05%     
==========================================
  Files         261      263       +2     
  Lines       23152    23334     +182     
==========================================
+ Hits        14201    14302     +101     
- Misses       7949     8022      +73     
- Partials     1002     1010       +8     
Flag Coverage Δ
?

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
app/app.go 66.32% <66.66%> (+0.13%) ⬆️
app/seidb.go 80.35% <54.54%> (-7.65%) ⬇️
app/test_state_store.go 56.14% <0.00%> (-2.91%) ⬇️

... and 21 files with indirect coverage changes

fmt.Println("Scanning database and exporting leaf nodes...")
fmt.Println("SeiDB Archive Migration: Scanning database and exporting leaf nodes...")

startTimeTotal := time.Now() // Start measuring total time

Check warning

Code scanning / CodeQL

Calling the system time Warning

Calling the system time may be a possible source of non-determinism
if module != startModule && startModule != "" {
continue
}
startTimeModule := time.Now() // Measure time for each module

Check warning

Code scanning / CodeQL

Calling the system time Warning

Calling the system time may be a possible source of non-determinism
})

batchLeafNodeCount = 0
startTimeBatch = time.Now()

Check warning

Code scanning / CodeQL

Calling the system time Warning

Calling the system time may be a possible source of non-determinism
The overall process will work as follows:

1. Stop archive node and note down its height, call it MIGRATION_HEIGHT
2. Update config to enable SeiDB (state committment + state store)
Copy link
Contributor

Choose a reason for hiding this comment

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

This should probably be the last step?


The overall process will work as follows:

1. Stop archive node and note down its height, call it MIGRATION_HEIGHT
Copy link
Contributor

Choose a reason for hiding this comment

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

Actually we don't need this step, we can get the height when we run SC migration


1. Stop archive node and note down its height, call it MIGRATION_HEIGHT
2. Update config to enable SeiDB (state committment + state store)
3. Run sc migration at the MIGRATION_HEIGHT
Copy link
Contributor

Choose a reason for hiding this comment

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

Default it will run at latest height, which we should note it down

3. Run sc migration at the MIGRATION_HEIGHT
4. Re start seid with `--migrate-iavl` enabled (migrating state store in background)
5. Verify migration at various sampled heights once state store is complete
6. Stop seid, clear out iavl and restart seid normally, now only using SeiDB fully
Copy link
Contributor

Choose a reason for hiding this comment

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

We should not delete IAVL data here but launch with SeiDB first, and only delete the IAVL data after we successfully run the node for a while

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants