Skip to content

Commit

Permalink
Introduce rdb_snapshot_unique_ptr & make rdb_get_rocksdb_db return re…
Browse files Browse the repository at this point in the history
…ference

This is a second batch of cleanups from the repeatable read snapshot work.

- Make rdb_get_rocksdb_db return reference instead of pointer, update callers
  and callees. Inline this function in ha_rocksdb_proto.h, move rdb declaration
  there to support inlining in detail namespace as documentation that it should
  not be accessed directly. Update ha_rocksdb.cc itself to use
  rdb_get_rocksdb_db instead of rdb. Remove many redudant rdb != nullptr checks
  and asserts.
- Introduce rdb_snapshot_unique_ptr with a custom deleter and a factory function
  get_rdb_snapshot to manager global RocksDB snapshots
- Make Rdb_explicit_snapshot create the snapshot itself, by calling
  get_rdb_snapshot, instead of receiving one through parameters, simplify its
  signature.
- Replace rocksdb::ManagedSnapshot uses with rdb_snapshot_unique_ptr ones.
- Remove rarely-used explicit transaction snapshot assignment code from
  acquire_snapshot and move it to the few callers where it may happen.
  Add new method Rdb_transaction::has_explicit_or_read_only_snapshot to support
  this.
- For Rdb_transaction, make m_insert_count, m_update_count, m_delete_count,
  m_auto_incr_map, & m_rollback_only fields private instead of protected. Move
  their reset at the end of committed or rolledback transactions to on_finish
  method in the base class. Remove redundant m_writes_at_last_savepoint reset in
  set_initial_savepoint.
- Add several asserts.
  • Loading branch information
laurynas-biveinis committed Dec 17, 2024
1 parent 69710da commit 5c5c2ae
Show file tree
Hide file tree
Showing 16 changed files with 370 additions and 419 deletions.
14 changes: 7 additions & 7 deletions storage/rocksdb/clone/donor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -614,8 +614,8 @@ donor::donor(const myrocks::clone::locator &l, const uchar *&loc,

donor::~donor() {
if (m_rdb_file_deletes_disabled) {
auto *const rdb = myrocks::rdb_get_rocksdb_db();
const auto result = rdb->EnableFileDeletions();
auto &rdb = myrocks::rdb_get_rocksdb_db();
const auto result = rdb.EnableFileDeletions();
if (!result.ok()) {
myrocks::rdb_log_status_error(result,
"RocksDB file deletion re-enable failed");
Expand Down Expand Up @@ -700,9 +700,9 @@ int donor::next_checkpoint_locked(bool final, std::size_t &total_new_size) {
auto err = m_checkpoint.cleanup();
if (err != 0) return save_and_return_error(err, "RocksDB checkpoint error");

auto *const rdb = final ? myrocks::rdb_get_rocksdb_db() : nullptr;
if (rdb != nullptr) {
const auto dfd_result = rdb->DisableFileDeletions();
if (final) {
const auto dfd_result =
myrocks::rdb_get_rocksdb_db().DisableFileDeletions();
m_rdb_file_deletes_disabled = dfd_result.ok();
if (!m_rdb_file_deletes_disabled) {
myrocks::rdb_log_status_error(dfd_result,
Expand All @@ -714,7 +714,7 @@ int donor::next_checkpoint_locked(bool final, std::size_t &total_new_size) {

err = m_checkpoint.init();
if (err != 0) {
if (rdb) rdb->EnableFileDeletions();
if (final) myrocks::rdb_get_rocksdb_db().EnableFileDeletions();
return save_and_return_error(err, "RocksDB checkpoint error");
}

Expand All @@ -726,7 +726,7 @@ int donor::next_checkpoint_locked(bool final, std::size_t &total_new_size) {
if (err != 0) {
// Ignore the return value because we are already returning an error
(void)m_checkpoint.cleanup();
if (rdb) rdb->EnableFileDeletions();
if (final) myrocks::rdb_get_rocksdb_db().EnableFileDeletions();
return err;
}

Expand Down
Loading

0 comments on commit 5c5c2ae

Please sign in to comment.