From 074b4e3e31264f839f0356485b78dedaeed8437c Mon Sep 17 00:00:00 2001 From: yihuang Date: Sat, 17 Jun 2023 22:50:36 +0800 Subject: [PATCH 1/3] fix: snapshotter's failure is not propogated (#16588) (cherry picked from commit aeccbc910c637852a18796477d1a7da886ada794) # Conflicts: # store/CHANGELOG.md --- snapshots/chunk.go | 8 +++--- snapshots/helpers_test.go | 32 +++++++++++++++++++++++ snapshots/manager_test.go | 11 ++++++++ store/CHANGELOG.md | 53 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 101 insertions(+), 3 deletions(-) create mode 100644 store/CHANGELOG.md diff --git a/snapshots/chunk.go b/snapshots/chunk.go index 74503f058027..af05303b4536 100644 --- a/snapshots/chunk.go +++ b/snapshots/chunk.go @@ -58,11 +58,13 @@ func (w *ChunkWriter) Close() error { // CloseWithError closes the writer and sends an error to the reader. func (w *ChunkWriter) CloseWithError(err error) { if !w.closed { + if w.pipe == nil { + // create a dummy pipe just to propagate the error to the reader, it always returns nil + _ = w.chunk() + } w.closed = true close(w.ch) - if w.pipe != nil { - _ = w.pipe.CloseWithError(err) // CloseWithError always returns nil - } + _ = w.pipe.CloseWithError(err) // CloseWithError always returns nil } } diff --git a/snapshots/helpers_test.go b/snapshots/helpers_test.go index 24051a17a927..04448bc0e02a 100644 --- a/snapshots/helpers_test.go +++ b/snapshots/helpers_test.go @@ -158,6 +158,38 @@ func (m *mockSnapshotter) SetSnapshotInterval(snapshotInterval uint64) { m.snapshotInterval = snapshotInterval } +type mockErrorSnapshotter struct{} + +var _ snapshottypes.Snapshotter = (*mockErrorSnapshotter)(nil) + +func (m *mockErrorSnapshotter) Snapshot(height uint64, protoWriter protoio.Writer) error { + return errors.New("mock snapshot error") +} + +func (m *mockErrorSnapshotter) Restore( + height uint64, format uint32, protoReader protoio.Reader, +) (snapshottypes.SnapshotItem, error) { + return snapshottypes.SnapshotItem{}, errors.New("mock restore error") +} + +func (m *mockErrorSnapshotter) SnapshotFormat() uint32 { + return snapshottypes.CurrentFormat +} + +func (m *mockErrorSnapshotter) SupportedFormats() []uint32 { + return []uint32{snapshottypes.CurrentFormat} +} + +func (m *mockErrorSnapshotter) PruneSnapshotHeight(height int64) { +} + +func (m *mockErrorSnapshotter) GetSnapshotInterval() uint64 { + return 0 +} + +func (m *mockErrorSnapshotter) SetSnapshotInterval(snapshotInterval uint64) { +} + // setupBusyManager creates a manager with an empty store that is busy creating a snapshot at height 1. // The snapshot will complete when the returned closer is called. func setupBusyManager(t *testing.T) *snapshots.Manager { diff --git a/snapshots/manager_test.go b/snapshots/manager_test.go index d379f09cb9f1..86572c74028b 100644 --- a/snapshots/manager_test.go +++ b/snapshots/manager_test.go @@ -4,6 +4,7 @@ import ( "errors" "testing" + db "github.com/cosmos/cosmos-db" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/tendermint/tendermint/libs/log" @@ -235,3 +236,13 @@ func TestManager_Restore(t *testing.T) { }) require.NoError(t, err) } + +func TestManager_TakeError(t *testing.T) { + snapshotter := &mockErrorSnapshotter{} + store, err := snapshots.NewStore(db.NewMemDB(), GetTempDir(t)) + require.NoError(t, err) + manager := snapshots.NewManager(store, opts, snapshotter, nil, log.NewNopLogger()) + + _, err = manager.Create(1) + require.Error(t, err) +} diff --git a/store/CHANGELOG.md b/store/CHANGELOG.md new file mode 100644 index 000000000000..f46df2cbae22 --- /dev/null +++ b/store/CHANGELOG.md @@ -0,0 +1,53 @@ + + +# Changelog + +## [Unreleased] + +### Features + +* [#15568](https://github.com/cosmos/cosmos-sdk/pull/15568) Migrate the `iavl` to the new key format. + * Remove `DeleteVersion`, `DeleteVersions`, `LazyLoadVersionForOverwriting` from `iavl` tree API. + * Add `DeleteVersionsTo` and `SaveChangeSet`, since it will keep versions sequentially like `fromVersion` to `toVersion`. + * Refactor the pruning manager to use `DeleteVersionsTo`. +* [#15712](https://github.com/cosmos/cosmos-sdk/pull/15712) Add `WorkingHash` function to the store interface to get the current app hash before commit. + +* [#14645](https://github.com/cosmos/cosmos-sdk/pull/14645) Add limit to the length of key and value. +* [#15683](https://github.com/cosmos/cosmos-sdk/pull/15683) `rootmulti.Store.CacheMultiStoreWithVersion` now can handle loading archival states that don't persist any of the module stores the current state has. +* [#16060](https://github.com/cosmos/cosmos-sdk/pull/16060) Support saving restoring snapshot locally. + +### API Breaking Changes + +* [#16321](https://github.com/cosmos/cosmos-sdk/pull/16321) QueryInterface defines its own request and response types instead of relying on comet/abci & returns an error + +### Bug Fixes + +* [#16588](https://github.com/cosmos/cosmos-sdk/pull/16588) Propogate the Snapshotter's failure to the caller, (it will create a empty snapshot silently before). + +## [v0.1.0-alpha.1](https://github.com/cosmos/cosmos-sdk/releases/tag/store%2Fv0.1.0-alpha.1) - 2023-03-17 + +### Features + +* [#14746](https://github.com/cosmos/cosmos-sdk/pull/14746) The `store` module is extracted to have a separate go.mod file which allows it be a standalone module. +* [#14410](https://github.com/cosmos/cosmos-sdk/pull/14410) `rootmulti.Store.loadVersion` has validation to check if all the module stores' height is correct, it will error if any module store has incorrect height. From 02f7f61f386c767b198fdf412198e54ddc443efa Mon Sep 17 00:00:00 2001 From: HuangYi Date: Mon, 19 Jun 2023 09:00:18 +0800 Subject: [PATCH 2/3] changelog --- CHANGELOG.md | 1 + store/CHANGELOG.md | 53 ---------------------------------------------- 2 files changed, 1 insertion(+), 53 deletions(-) delete mode 100644 store/CHANGELOG.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 122dd9a9fbe8..9a7388571089 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,6 +44,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Bug Fixes * (x/auth) [#16554](https://github.com/cosmos/cosmos-sdk/pull/16554) `ModuleAccount.Validate` now reports a nil `.BaseAccount` instead of panicking. +* [#16588](https://github.com/cosmos/cosmos-sdk/pull/16588) Propogate the Snapshotter's failure to the caller, (it will create a empty snapshot silently before). ## [v0.46.13](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.46.13) - 2023-06-08 diff --git a/store/CHANGELOG.md b/store/CHANGELOG.md deleted file mode 100644 index f46df2cbae22..000000000000 --- a/store/CHANGELOG.md +++ /dev/null @@ -1,53 +0,0 @@ - - -# Changelog - -## [Unreleased] - -### Features - -* [#15568](https://github.com/cosmos/cosmos-sdk/pull/15568) Migrate the `iavl` to the new key format. - * Remove `DeleteVersion`, `DeleteVersions`, `LazyLoadVersionForOverwriting` from `iavl` tree API. - * Add `DeleteVersionsTo` and `SaveChangeSet`, since it will keep versions sequentially like `fromVersion` to `toVersion`. - * Refactor the pruning manager to use `DeleteVersionsTo`. -* [#15712](https://github.com/cosmos/cosmos-sdk/pull/15712) Add `WorkingHash` function to the store interface to get the current app hash before commit. - -* [#14645](https://github.com/cosmos/cosmos-sdk/pull/14645) Add limit to the length of key and value. -* [#15683](https://github.com/cosmos/cosmos-sdk/pull/15683) `rootmulti.Store.CacheMultiStoreWithVersion` now can handle loading archival states that don't persist any of the module stores the current state has. -* [#16060](https://github.com/cosmos/cosmos-sdk/pull/16060) Support saving restoring snapshot locally. - -### API Breaking Changes - -* [#16321](https://github.com/cosmos/cosmos-sdk/pull/16321) QueryInterface defines its own request and response types instead of relying on comet/abci & returns an error - -### Bug Fixes - -* [#16588](https://github.com/cosmos/cosmos-sdk/pull/16588) Propogate the Snapshotter's failure to the caller, (it will create a empty snapshot silently before). - -## [v0.1.0-alpha.1](https://github.com/cosmos/cosmos-sdk/releases/tag/store%2Fv0.1.0-alpha.1) - 2023-03-17 - -### Features - -* [#14746](https://github.com/cosmos/cosmos-sdk/pull/14746) The `store` module is extracted to have a separate go.mod file which allows it be a standalone module. -* [#14410](https://github.com/cosmos/cosmos-sdk/pull/14410) `rootmulti.Store.loadVersion` has validation to check if all the module stores' height is correct, it will error if any module store has incorrect height. From 673ee3a545189f8408f58fd7d338256491e98189 Mon Sep 17 00:00:00 2001 From: HuangYi Date: Mon, 19 Jun 2023 09:20:23 +0800 Subject: [PATCH 3/3] fix test --- snapshots/manager_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/snapshots/manager_test.go b/snapshots/manager_test.go index 86572c74028b..d41819e3f136 100644 --- a/snapshots/manager_test.go +++ b/snapshots/manager_test.go @@ -4,13 +4,14 @@ import ( "errors" "testing" - db "github.com/cosmos/cosmos-db" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/tendermint/tendermint/libs/log" + dbm "github.com/tendermint/tm-db" "github.com/cosmos/cosmos-sdk/snapshots" "github.com/cosmos/cosmos-sdk/snapshots/types" + "github.com/cosmos/cosmos-sdk/testutil" ) var opts = types.NewSnapshotOptions(1500, 2) @@ -239,7 +240,7 @@ func TestManager_Restore(t *testing.T) { func TestManager_TakeError(t *testing.T) { snapshotter := &mockErrorSnapshotter{} - store, err := snapshots.NewStore(db.NewMemDB(), GetTempDir(t)) + store, err := snapshots.NewStore(dbm.NewMemDB(), testutil.GetTempDir(t)) require.NoError(t, err) manager := snapshots.NewManager(store, opts, snapshotter, nil, log.NewNopLogger())