Skip to content

fix(tests): don't clean up RocksDB directory before store shutdown#2107

Open
kkovaacs wants to merge 1 commit into
nextfrom
krisztian/fix-rocksdb-panic-on-test-failures
Open

fix(tests): don't clean up RocksDB directory before store shutdown#2107
kkovaacs wants to merge 1 commit into
nextfrom
krisztian/fix-rocksdb-panic-on-test-failures

Conversation

@kkovaacs
Copy link
Copy Markdown
Contributor

Due to missing store shutdown on panic test failures could cause the temporary directory to be cleaned up while the store was still running.

This would then cause a panic when the LargeSmt RocksDB storage backend was being dropped because we're doing an explicit flush there.

This change fixes this by introducing a TestStore wrapper around the store and the temporary directory that makes sure that the store runtime is stopped before deleting the temporary directory.

Due to missing store shutdown on panic test failures could cause
the temporary directory to be cleaned up while the store was still
running.

This would then cause a panic when the LargeSmt RocksDB storage backend
was being dropped because we're doing an explicit flush there.

This change fixes this by introducing a `TestStore` wrapper around the
store and the temporary directory that makes sure that the store runtime
is stopped before deleting the temporary directory.
@kkovaacs kkovaacs added the no changelog This PR does not require an entry in the `CHANGELOG.md` file label May 20, 2026
@kkovaacs kkovaacs marked this pull request as ready for review May 20, 2026 13:05
Copy link
Copy Markdown
Collaborator

@Mirko-von-Leipzig Mirko-von-Leipzig left a comment

Choose a reason for hiding this comment

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

Thank you!

I guess ideally we would have a proper shutdown channel to the server(s)?

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

Labels

no changelog This PR does not require an entry in the `CHANGELOG.md` file

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants