From f3858e52def8051575db7a0fea907e8f10bfb506 Mon Sep 17 00:00:00 2001 From: "M. J. Fromberger" Date: Thu, 7 Apr 2022 15:09:09 -0700 Subject: [PATCH 1/2] scmigrate: ensure target key is correctly renamed (#8276) Prior to v0.35, the keys for seen-commit records included the applicable height. In v0.35 and beyond, we only keep the record for the latest height, and its key does not include the height. Update the seen-commit migration to ensure that the record we retain after migration is correctly renamed to omit the height from its key. Update the test cases to check for this condition after migrating. --- CHANGELOG_PENDING.md | 1 + go.mod | 1 + go.sum | 2 + scripts/keymigrate/migrate.go | 82 +++++++----------------------- scripts/keymigrate/migrate_test.go | 17 ++----- scripts/scmigrate/migrate.go | 51 ++++++++++++++++--- scripts/scmigrate/migrate_test.go | 26 +++++----- 7 files changed, 81 insertions(+), 99 deletions(-) diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index cc71a5a5e..f0409a545 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -81,3 +81,4 @@ Special thanks to external contributors on this release: - [light] \#7640 Light Client: fix absence proof verification (@ashcherbakov) - [light] \#7641 Light Client: fix querying against the latest height (@ashcherbakov) - [cli] [#7837](https://github.com/tendermint/tendermint/pull/7837) fix app hash in state rollback. (@yihuang) +- [cli] \#8276 scmigrate: ensure target key is correctly renamed. (@creachadair) diff --git a/go.mod b/go.mod index 3b0004ba0..49db1ba5e 100644 --- a/go.mod +++ b/go.mod @@ -72,6 +72,7 @@ require ( github.com/charithe/durationcheck v0.0.9 // indirect github.com/chavacava/garif v0.0.0-20210405164556-e8a0a408d6af // indirect github.com/containerd/continuity v0.2.1 // indirect + github.com/creachadair/taskgroup v0.3.2 // indirect github.com/daixiang0/gci v0.3.3 // indirect github.com/davecgh/go-spew v1.1.1 // indirect github.com/denis-tingaikin/go-header v0.4.3 // indirect diff --git a/go.sum b/go.sum index 7374c90d0..c0388b7af 100644 --- a/go.sum +++ b/go.sum @@ -223,6 +223,8 @@ github.com/cpuguy83/go-md2man/v2 v2.0.0/go.mod h1:maD7wRr/U5Z6m/iR4s+kqSMx2CaBsr github.com/cpuguy83/go-md2man/v2 v2.0.1/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o= github.com/creachadair/atomicfile v0.2.4 h1:GRjpQLmz/78I4+nBQpGMFrRa9yrL157AUTrA6hnF0YU= github.com/creachadair/atomicfile v0.2.4/go.mod h1:BRq8Une6ckFneYXZQ+kO7p1ZZP3I2fzVzf28JxrIkBc= +github.com/creachadair/taskgroup v0.3.2 h1:zlfutDS+5XG40AOxcHDSThxKzns8Tnr9jnr6VqkYlkM= +github.com/creachadair/taskgroup v0.3.2/go.mod h1:wieWwecHVzsidg2CsUnFinW1faVN4+kq+TDlRJQ0Wbk= github.com/creack/pty v1.1.7/go.mod h1:lj5s0c3V2DBrqTV7llrYr5NG6My20zk30Fl46Y7DoTY= github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E= github.com/cyphar/filepath-securejoin v0.2.2/go.mod h1:FpkQEhXnPnOthhzymB7CGsFk2G9VLXONKD9G7QGMM+4= diff --git a/scripts/keymigrate/migrate.go b/scripts/keymigrate/migrate.go index 2061223ad..b53c5e98f 100644 --- a/scripts/keymigrate/migrate.go +++ b/scripts/keymigrate/migrate.go @@ -15,8 +15,8 @@ import ( "math/rand" "runtime" "strconv" - "sync" + "github.com/creachadair/taskgroup" "github.com/google/orderedcode" dbm "github.com/tendermint/tm-db" ) @@ -27,7 +27,7 @@ type ( ) func getAllLegacyKeys(db dbm.DB) ([]keyID, error) { - out := []keyID{} + var out []keyID iter, err := db.Iterator(nil, nil) if err != nil { @@ -43,11 +43,8 @@ func getAllLegacyKeys(db dbm.DB) ([]keyID, error) { continue } - // there's inconsistency around tm-db's handling of - // key copies. - nk := make([]byte, len(k)) - copy(nk, k) - out = append(out, nk) + // Make an explicit copy, since not all tm-db backends do. + out = append(out, []byte(string(k))) } if err = iter.Error(); err != nil { @@ -61,17 +58,6 @@ func getAllLegacyKeys(db dbm.DB) ([]keyID, error) { return out, nil } -func makeKeyChan(keys []keyID) <-chan keyID { - out := make(chan keyID, len(keys)) - defer close(out) - - for _, key := range keys { - out <- key - } - - return out -} - func keyIsLegacy(key keyID) bool { for _, prefix := range []keyID{ // core "store" @@ -111,7 +97,7 @@ func keyIsHash(key keyID) bool { return len(key) == 32 && !bytes.Contains(key, []byte("/")) } -func migarateKey(key keyID) (keyID, error) { +func migrateKey(key keyID) (keyID, error) { switch { case bytes.HasPrefix(key, keyID("H:")): val, err := strconv.Atoi(string(key[2:])) @@ -349,53 +335,23 @@ func Migrate(ctx context.Context, db dbm.DB) error { return err } - numWorkers := runtime.NumCPU() - wg := &sync.WaitGroup{} + var errs []string + g, start := taskgroup.New(func(err error) error { + errs = append(errs, err.Error()) + return err + }).Limit(runtime.NumCPU()) - errs := make(chan error, numWorkers) - - keyCh := makeKeyChan(keys) - - // run migrations. - for i := 0; i < numWorkers; i++ { - wg.Add(1) - go func() { - defer wg.Done() - for key := range keyCh { - err := replaceKey(db, key, migarateKey) - if err != nil { - errs <- err - } - - if ctx.Err() != nil { - return - } + for _, key := range keys { + key := key + start(func() error { + if err := ctx.Err(); err != nil { + return err } - }() + return replaceKey(db, key, migrateKey) + }) } - - // collect and process the errors. - errStrs := []string{} - signal := make(chan struct{}) - go func() { - defer close(signal) - for err := range errs { - if err == nil { - continue - } - errStrs = append(errStrs, err.Error()) - } - }() - - // Wait for everything to be done. - wg.Wait() - close(errs) - <-signal - - // check the error results - if len(errs) != 0 { - return fmt.Errorf("encountered errors during migration: %v", errStrs) + if g.Wait() != nil { + return fmt.Errorf("encountered errors during migration: %q", errs) } - return nil } diff --git a/scripts/keymigrate/migrate_test.go b/scripts/keymigrate/migrate_test.go index 21e9592fb..8f6f30808 100644 --- a/scripts/keymigrate/migrate_test.go +++ b/scripts/keymigrate/migrate_test.go @@ -117,7 +117,7 @@ func TestMigration(t *testing.T) { }) t.Run("Conversion", func(t *testing.T) { for kind, le := range legacyPrefixes { - nk, err := migarateKey(le) + nk, err := migrateKey(le) require.NoError(t, err, kind) require.False(t, keyIsLegacy(nk), kind) } @@ -159,7 +159,7 @@ func TestMigration(t *testing.T) { "UserKey3": []byte("foo/bar/baz/1.2/4"), } for kind, key := range table { - out, err := migarateKey(key) + out, err := migrateKey(key) require.Error(t, err, kind) require.Nil(t, out, kind) } @@ -177,7 +177,7 @@ func TestMigration(t *testing.T) { return nil, errors.New("hi") })) }) - t.Run("KeyDisapears", func(t *testing.T) { + t.Run("KeyDisappears", func(t *testing.T) { db := dbm.NewMemDB() key := keyID("hi") require.NoError(t, db.Set(key, []byte("world"))) @@ -215,17 +215,6 @@ func TestMigration(t *testing.T) { require.False(t, keyIsLegacy(key)) } }) - t.Run("ChannelConversion", func(t *testing.T) { - ch := makeKeyChan([]keyID{ - makeKey(t, "abc", int64(2), int64(42)), - makeKey(t, int64(42)), - }) - count := 0 - for range ch { - count++ - } - require.Equal(t, 2, count) - }) t.Run("Migrate", func(t *testing.T) { _, db := getLegacyDatabase(t) diff --git a/scripts/scmigrate/migrate.go b/scripts/scmigrate/migrate.go index e6eee3d95..ba3de2698 100644 --- a/scripts/scmigrate/migrate.go +++ b/scripts/scmigrate/migrate.go @@ -73,8 +73,6 @@ func sortMigrations(scData []toMigrate) { }) } -func getMigrationsToDelete(in []toMigrate) []toMigrate { return in[1:] } - func getAllSeenCommits(ctx context.Context, db dbm.DB) ([]toMigrate, error) { scKeyPrefix := makeKeyFromPrefix(prefixSeenCommit) iter, err := db.Iterator( @@ -117,6 +115,34 @@ func getAllSeenCommits(ctx context.Context, db dbm.DB) ([]toMigrate, error) { return scData, nil } +func renameRecord(ctx context.Context, db dbm.DB, keep toMigrate) error { + wantKey := makeKeyFromPrefix(prefixSeenCommit) + if bytes.Equal(keep.key, wantKey) { + return nil // we already did this conversion + } + + // This record's key has already been converted to the "new" format, we just + // now need to trim off the tail. + val, err := db.Get(keep.key) + if err != nil { + return err + } + + batch := db.NewBatch() + if err := batch.Delete(keep.key); err != nil { + return err + } + if err := batch.Set(wantKey, val); err != nil { + return err + } + werr := batch.Write() + cerr := batch.Close() + if werr != nil { + return werr + } + return cerr +} + func deleteRecords(ctx context.Context, db dbm.DB, scData []toMigrate) error { // delete all the remaining stale values in a single batch batch := db.NewBatch() @@ -141,20 +167,29 @@ func Migrate(ctx context.Context, db dbm.DB) error { scData, err := getAllSeenCommits(ctx, db) if err != nil { return fmt.Errorf("sourcing tasks to migrate: %w", err) + } else if len(scData) == 0 { + return nil // nothing to do } - // sort earliest->latest commits. + // Sort commits in decreasing order of height. sortMigrations(scData) - // trim the one we want to save: - scData = getMigrationsToDelete(scData) + // Keep and rename the newest seen commit, delete the rest. + // In TM < v0.35 we kept a last-seen commit for each height; in v0.35 we + // retain only the latest. + keep, remove := scData[0], scData[1:] - if len(scData) <= 1 { + if err := renameRecord(ctx, db, keep); err != nil { + return fmt.Errorf("renaming seen commit record: %w", err) + } + + if len(remove) == 0 { return nil } - // write the migration (remove ) - if err := deleteRecords(ctx, db, scData); err != nil { + // Remove any older seen commits. Prior to v0.35, we kept these records for + // all heights, but v0.35 keeps only the latest. + if err := deleteRecords(ctx, db, remove); err != nil { return fmt.Errorf("writing data: %w", err) } diff --git a/scripts/scmigrate/migrate_test.go b/scripts/scmigrate/migrate_test.go index abe12584d..900a15f85 100644 --- a/scripts/scmigrate/migrate_test.go +++ b/scripts/scmigrate/migrate_test.go @@ -1,6 +1,7 @@ package scmigrate import ( + "bytes" "context" "math/rand" "testing" @@ -104,15 +105,6 @@ func TestMigrations(t *testing.T) { assertWellOrderedMigrations(t, testData) }) }) - t.Run("GetMigrationsToDelete", func(t *testing.T) { - for i := 1; i < 100; i++ { - data := appendRandomMigrations([]toMigrate{}, i) - toMigrate := getMigrationsToDelete(data) - if len(data) != len(toMigrate)+1 { - t.Fatalf("migration prep did not save one document [original=%d migrations=%d]", len(data), len(toMigrate)) - } - } - }) t.Run("InvalidMigrations", func(t *testing.T) { if _, err := makeToMigrate(nil); err == nil { t.Fatal("should error for nil migrations") @@ -156,18 +148,24 @@ func TestMigrations(t *testing.T) { // safe to rerun t.Run(test, func(t *testing.T) { if err := Migrate(ctx, db); err != nil { - t.Fatal(err) + t.Fatalf("Migration failed: %v", err) } post, err := getAllSeenCommits(ctx, db) if err != nil { - t.Fatal(err) + t.Fatalf("Fetching seen commits: %v", err) } + if len(post) != 1 { - t.Fatal("migration was not successful") + t.Fatalf("Wrong number of commits: got %d, wanted 1", len(post)) } - if post[0].commit.Height != latestHeight { - t.Fatal("migration did not save correct document") + + wantKey := makeKeyFromPrefix(prefixSeenCommit) + if !bytes.Equal(post[0].key, wantKey) { + t.Errorf("Seen commit key: got %x, want %x", post[0].key, wantKey) + } + if got := post[0].commit.Height; got != latestHeight { + t.Fatalf("Wrong commit height after migration: got %d, wanted %d", got, latestHeight) } }) } From 53b7dbe285fdbbd142d4d15337a5e026280653a0 Mon Sep 17 00:00:00 2001 From: "M. J. Fromberger" Date: Thu, 7 Apr 2022 15:30:48 -0700 Subject: [PATCH 2/2] Forward-port changelog for v0.34.19 to master. (#8279) --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b4a022dcc..07d049cf2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -230,6 +230,12 @@ Special thanks to external contributors on this release: @JayT106, - [cmd/tendermint/commands] [\#6623](https://github.com/tendermint/tendermint/pull/6623) replace `$HOME/.some/test/dir` with `t.TempDir` (@tanyabouman) - [statesync] \6807 Implement P2P state provider as an alternative to RPC (@cmwaters) +## v0.34.19 + +### BUG FIXES + +- [cli] [\#8270](https://github.com/tendermint/tendermint/issues/8270) fix reset commands (@alexanderbez). + ## v0.34.18 ### BREAKING CHANGES