Skip to content

kv/kvserver: fix TestSideloadingSideloadedStorage w/ RocksDB#50243

Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom
jbowens:jackson/kvrocksdb
Jun 15, 2020
Merged

kv/kvserver: fix TestSideloadingSideloadedStorage w/ RocksDB#50243
craig[bot] merged 1 commit intocockroachdb:masterfrom
jbowens:jackson/kvrocksdb

Conversation

@jbowens
Copy link
Copy Markdown
Contributor

@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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Contributor Author

jbowens commented Jun 15, 2020

TFTR!

bors r+

@craig
Copy link
Copy Markdown
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