Skip to content

Commit

Permalink
Merge pull request eBay#36 from ClickHouse-Extras/fix_handle_commit_b…
Browse files Browse the repository at this point in the history
…atch

Fix bug which lead to memory leak

(cherry picked from commit 1707a75)
  • Loading branch information
alesapin committed Jan 22, 2022
1 parent 6f26688 commit f54c04d
Showing 1 changed file with 16 additions and 3 deletions.
19 changes: 16 additions & 3 deletions src/handle_commit.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -300,14 +300,27 @@ void raft_server::commit_app_log(ulong idx_to_commit,
std::list< ptr<commit_ret_elem> > async_elems;
if (need_to_handle_commit_elem) {
std::unique_lock<std::mutex> cre_lock(commit_ret_elems_lock_);
bool match_found = false;
/// Sometimes user can batch requests to RAFT: for example send 30
/// append entries requests in a single batch. For such request batch
/// user will receive a single response: all was successful or all
/// failed. Obviously we don't need to add info about responses
/// (commit_ret_elems) for 29 requests from batch and need to do it only
/// for 30-th request. precommit_index is exact value which identify ID
/// of the last request from the latest batch. So if we commiting this
/// last request and for some reason it was not added into
/// commit_ret_elems in the handle_cli_req method (logical race
/// condition) we have to add it here. Otherwise we don't need to add
/// anything into commit_ret_elems_, because nobody will wait for the
/// responses of the intermediate requests from requests batch.
bool need_to_check_commit_ret = sm_idx == pc_idx;

auto entry = commit_ret_elems_.find(sm_idx);
if (entry != commit_ret_elems_.end()) {
ptr<commit_ret_elem> elem = entry->second;
if (elem->idx_ == sm_idx) {
elem->result_code_ = cmd_result_code::OK;
elem->ret_value_ = ret_value;
match_found = true;
need_to_check_commit_ret = false;
p_dv("notify cb %ld %p", sm_idx, &elem->awaiter_);

switch (ctx_->get_params()->return_method_) {
Expand All @@ -326,7 +339,7 @@ void raft_server::commit_app_log(ulong idx_to_commit,
}
}

if (!match_found) {
if (need_to_check_commit_ret) {
// If not found, commit thread is invoked earlier than user thread.
// Create one here.
ptr<commit_ret_elem> elem = cs_new<commit_ret_elem>();
Expand Down

0 comments on commit f54c04d

Please sign in to comment.