Skip to content

Upgrade RocksDB, chiefly to mitigate iterator bug #108

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.

Sign up for GitHub

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

Open
bitonic wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Upgrade RocksDB, chiefly to mitigate iterator bug #108

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e7cd4a8
Upgrade RocksDB, chiefly to mitigate iterator bug
bitonic Feb 27, 2026
a7b43bf
tests: fix destruction order
mcrnic Mar 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 7 additions & 17 deletions cpp/cdc/CDCDB.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1730,7 +1730,7 @@ struct CDCDBImpl {
{
// Additional safety check: the key is what we expect.
auto foundKey = ExternalValue<DirsToTxnsKey>::FromSlice(it->key());
ALWAYS_ASSERT(!foundKey().isSentinel() && foundKey().txnId() == txnId);
ALWAYS_ASSERT(foundKey().dirId() == dirId && !foundKey().isSentinel() && foundKey().txnId() == txnId);
}
// now that we've done our checks, we can remove the key
ROCKS_DB_CHECKED(dbTxn.Delete(_dirsToTxnsCf, k.toSlice()));
Expand All @@ -1743,23 +1743,13 @@ struct CDCDBImpl {
bool removeSentinel = true;
if (it->Valid()) { // there's something, set the sentinel
auto nextK = ExternalValue<DirsToTxnsKey>::FromSlice(it->key());
// This should never happen, since we specify the upper bound.
// However the RocksDB that we use is buggy, see
// <https://github.com/facebook/rocksdb/commit/543191f2eacadf14e3aa6ff9a08f85a8ad82da95>.
// To be removed on upgrade.
if (nextK().dirId() == dirId) {
removeSentinel = false;
auto sentinelV = CDCTxnIdValue::Static(nextK().txnId());
LOG_DEBUG(_env, "selected %s as next in line after finishing %s", nextK().txnId(), txnId);
mightBeReady.emplace_back(nextK().txnId());
ROCKS_DB_CHECKED(dbTxn.Put(_dirsToTxnsCf, sentinelK.toSlice(), sentinelV.toSlice()));
} else {
RAISE_ALERT_APP_TYPE(_env, XmonAppType::DAYTIME, "Unexpectedly stepped from %s to %s, is RocksDB not respecting our upper bound?", dirId, nextK().dirId());
}
} else {
ALWAYS_ASSERT(nextK().dirId() == dirId); // must be true given the upper bound
auto sentinelV = CDCTxnIdValue::Static(nextK().txnId());
LOG_DEBUG(_env, "selected %s as next in line after finishing %s", nextK().txnId(), txnId);
mightBeReady.emplace_back(nextK().txnId());
ROCKS_DB_CHECKED(dbTxn.Put(_dirsToTxnsCf, sentinelK.toSlice(), sentinelV.toSlice()));
} else { // we were the last ones here, remove sentinel
ROCKS_DB_CHECKED(it->status());
}
if (removeSentinel) { // we were the last ones here, remove sentinel
ROCKS_DB_CHECKED(dbTxn.Delete(_dirsToTxnsCf, sentinelK.toSlice()));
}
}
Expand Down
3 changes: 3 additions & 0 deletions cpp/tests/tests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -413,6 +413,9 @@ struct TempShardDB {
}

~TempShardDB() {
db.reset();
blockServicesCacheDB.reset();
sharedDB.reset();
std::error_code err;
if (std::filesystem::remove_all(std::filesystem::path(dbDir), err) < 0) {
std::cerr << "Could not remove " << dbDir << ": " << err << std::endl;
Expand Down
1 change: 1 addition & 0 deletions cpp/tests/utils/TempBlockServicesCacheDB.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ struct TempBlockServicesCacheDB {
}

~TempBlockServicesCacheDB() {
sharedDB.reset();
std::error_code err;
std::filesystem::remove_all(std::filesystem::path(dbDir), err);
}
Expand Down
3 changes: 3 additions & 0 deletions cpp/tests/utils/TempLogsDB.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,9 @@ struct TempLogsDB {
}

~TempLogsDB() {
db.reset();
sharedDB.reset();

std::error_code err;
if (std::filesystem::remove_all(std::filesystem::path(dbDir), err) < 0) {
std::cerr << "Could not remove " << dbDir << ": " << err << std::endl;
Expand Down
2 changes: 2 additions & 0 deletions cpp/tests/utils/TempRegistryDB.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ struct TempRegistryDB {
}

~TempRegistryDB() {
db.reset();
sharedDB.reset();
std::error_code err;
if (std::filesystem::remove_all(std::filesystem::path(dbDir), err) < 0) {
std::cerr << "Could not remove " << dbDir << ": " << err << std::endl;
Expand Down
4 changes: 2 additions & 2 deletions cpp/thirdparty.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,8 +115,8 @@ separate_arguments(
)
ExternalProject_Add(make_rocksdb
DOWNLOAD_DIR ${THIRDPARTY_DOWNLOAD_DIR}
URL https://github.com/facebook/rocksdb/archive/refs/tags/v7.9.2.tar.gz
URL_HASH SHA256=886378093098a1b2521b824782db7f7dd86224c232cf9652fcaf88222420b292
URL https://github.com/facebook/rocksdb/archive/refs/tags/v10.10.1.tar.gz
URL_HASH SHA256=df2ff348f3fac8578fd4b727eee7267aaf90cd403c99b55e898d1db63fa8cff5
DOWNLOAD_NAME rocksdb.tar.gz
PREFIX thirdparty/rocksdb
UPDATE_COMMAND ""
Expand Down
Loading