Skip to content

Commit

Permalink
fix: snapshotter's failure is not propogated (#16588)
Browse files Browse the repository at this point in the history
(cherry picked from commit aeccbc9)

# Conflicts:
#	store/CHANGELOG.md
  • Loading branch information
yihuang authored and mergify[bot] committed Jun 17, 2023
1 parent f34e99d commit 074b4e3
Show file tree
Hide file tree
Showing 4 changed files with 101 additions and 3 deletions.
8 changes: 5 additions & 3 deletions snapshots/chunk.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}

Expand Down
32 changes: 32 additions & 0 deletions snapshots/helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
11 changes: 11 additions & 0 deletions snapshots/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
}
53 changes: 53 additions & 0 deletions store/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
<!--
Guiding Principles:
Changelogs are for humans, not machines.
There should be an entry for every single version.
The same types of changes should be grouped.
Versions and sections should be linkable.
The latest version comes first.
The release date of each version is displayed.
Mention whether you follow Semantic Versioning.
Usage:
Change log entries are to be added to the Unreleased section under the
appropriate stanza (see below). Each entry should ideally include a tag and
the Github issue reference in the following format:
* (<tag>) [#<issue-number>] Changelog message.
Types of changes (Stanzas):
"Features" for new features.
"Improvements" for changes in existing functionality.
"Deprecated" for soon-to-be removed features.
"Bug Fixes" for any bug fixes.
"API Breaking" for breaking exported APIs used by developers building on SDK.
Ref: https://keepachangelog.com/en/1.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.

0 comments on commit 074b4e3

Please sign in to comment.