Skip to content

Commit

Permalink
address a race described in project-tsurugi/tsurugi-issues#1034 (comm…
Browse files Browse the repository at this point in the history
  • Loading branch information
t-horikawa committed Nov 28, 2024
1 parent f8ad37d commit 77552d6
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 8 deletions.
5 changes: 3 additions & 2 deletions include/limestone/api/datastore.h
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ class datastore {
protected: // for tests
auto& log_channels_for_tests() const noexcept { return log_channels_; }
auto epoch_id_informed_for_tests() const noexcept { return epoch_id_informed_.load(); }
auto epoch_id_recorded_for_tests() const noexcept { return epoch_id_recorded_.load(); }
auto epoch_id_recorded_for_tests() const noexcept { return epoch_id_to_be_recorded_.load(); }
auto epoch_id_switched_for_tests() const noexcept { return epoch_id_switched_.load(); }
auto& files_for_tests() const noexcept { return files_; }
void rotate_epoch_file_for_tests() { rotate_epoch_file(); }
Expand All @@ -263,7 +263,8 @@ class datastore {

std::atomic_uint64_t epoch_id_informed_{};

std::atomic_uint64_t epoch_id_recorded_{};
std::atomic_uint64_t epoch_id_to_be_recorded_{};
std::atomic_uint64_t epoch_id_record_finished_{};

std::unique_ptr<backup> backup_{};

Expand Down
18 changes: 12 additions & 6 deletions src/limestone/datastore.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,9 @@ void datastore::switch_epoch(epoch_id_type new_epoch_id) {
}

epoch_id_switched_.store(neid);
update_min_epoch_id(true);
if (state_ != state::not_ready) {
update_min_epoch_id(true);
}
} catch (...) {
HANDLE_EXCEPTION_AND_ABORT();
}
Expand All @@ -171,7 +173,7 @@ void datastore::switch_epoch(epoch_id_type new_epoch_id) {
void datastore::update_min_epoch_id(bool from_switch_epoch) { // NOLINT(readability-function-cognitive-complexity)
auto upper_limit = epoch_id_switched_.load();
if (upper_limit == 0) {
return; // If epoch_id_switched_ is zero, it means no epoch has been switched, so updating epoch_id_recorded_ and epoch_id_informed_ is unnecessary.
return; // If epoch_id_switched_ is zero, it means no epoch has been switched, so updating epoch_id_to_be_recorded_ and epoch_id_informed_ is unnecessary.
}
upper_limit--;
epoch_id_type max_finished_epoch = 0;
Expand All @@ -194,21 +196,21 @@ void datastore::update_min_epoch_id(bool from_switch_epoch) { // NOLINT(readabi
if (from_switch_epoch && (to_be_epoch > static_cast<std::uint64_t>(max_finished_epoch))) {
to_be_epoch = static_cast<std::uint64_t>(max_finished_epoch);
}
auto old_epoch_id = epoch_id_recorded_.load();
auto old_epoch_id = epoch_id_to_be_recorded_.load();
while (true) {
if (old_epoch_id >= to_be_epoch) {
break;
}
if (epoch_id_recorded_.compare_exchange_strong(old_epoch_id, to_be_epoch)) {
if (epoch_id_to_be_recorded_.compare_exchange_strong(old_epoch_id, to_be_epoch)) {
std::lock_guard<std::mutex> lock(mtx_epoch_file_);
if (to_be_epoch < epoch_id_recorded_.load()) {
if (to_be_epoch < epoch_id_to_be_recorded_.load()) {
break;
}
FILE* strm = fopen(epoch_file_path_.c_str(), "a"); // NOLINT(*-owning-memory)
if (!strm) {
LOG_AND_THROW_IO_EXCEPTION("fopen failed", errno);
}
log_entry::durable_epoch(strm, static_cast<epoch_id_type>(epoch_id_recorded_.load()));
log_entry::durable_epoch(strm, static_cast<epoch_id_type>(epoch_id_to_be_recorded_.load()));
if (fflush(strm) != 0) {
LOG_AND_THROW_IO_EXCEPTION("fflush failed", errno);
}
Expand All @@ -218,9 +220,13 @@ void datastore::update_min_epoch_id(bool from_switch_epoch) { // NOLINT(readabi
if (fclose(strm) != 0) { // NOLINT(*-owning-memory)
LOG_AND_THROW_IO_EXCEPTION("fclose failed", errno);
}
epoch_id_record_finished_.store(epoch_id_to_be_recorded_.load());
break;
}
}
if (to_be_epoch > epoch_id_record_finished_.load()) {
return;
}

// update informed_epoch_
to_be_epoch = upper_limit;
Expand Down

0 comments on commit 77552d6

Please sign in to comment.