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

kv/kvserver: fix TestSideloadingSideloadedStorage w/ RocksDB #50243

Merged
merged 1 commit into from
Jun 15, 2020

Conversation

jbowens
Copy link
Collaborator

@jbowens jbowens commented Jun 15, 2020

Fix the TestSideloadingSideloadedStorage test when running with
COCKROACH_STORAGE_ENGINE=rocksdb. The test has always depended on the
exact error message surfaced from os.Remove. In #49717,
diskSideloadStorage was modified to use the engine's RemoveDir operation
rather than interacting with the filesystem directly. Since Pebble uses
os.Remove for its implementation and emulates its error messages in its
MemFS implementation, the exact message comparison continued to succeed.

After #49717, when running with RocksDB, the RocksDB env's error message
was surfaced, with its own context prefixing. This updates the test to
case-insensitively check for 'directory not empty' instead.

Fixes #50226.

Release note: None

Fix the TestSideloadingSideloadedStorage test when running with
COCKROACH_STORAGE_ENGINE=rocksdb. The test has always depended on the
exact error message surfaced from os.Remove. In cockroachdb#49717,
diskSideloadStorage was modified to use the engine's RemoveDir operation
rather than interacting with the filesystem directly. Since Pebble uses
os.Remove for its implementation and emulates its error messages in its
MemFS implementation, the exact message comparison continued to succeed.

After cockroachdb#49717, the RocksDB env's error message was surfaced, with its own
context prefixing. This updates the test to case-insensitively check for
'directory not empty' instead.

Fixes cockroachdb#50226.

Release note: None
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@jbowens jbowens changed the title pkg/kv/kvserver: fix TestSideloadingSideloadedStorage w/ RocksDB kv/kvserver: fix TestSideloadingSideloadedStorage w/ RocksDB Jun 15, 2020
Copy link
Collaborator

@petermattis petermattis left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @andreimatei and @petermattis)

@jbowens
Copy link
Collaborator Author

jbowens commented Jun 15, 2020

TFTR!

bors r+

@craig
Copy link
Contributor

craig bot commented Jun 15, 2020

Build succeeded

@craig craig bot merged commit 5cff000 into cockroachdb:master Jun 15, 2020
@jbowens jbowens deleted the jackson/kvrocksdb branch June 15, 2020 22:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

kvserver: TestSideloadingSideloadedStorage fails with RocksDB
3 participants