Skip to content

fix writing empty batch in multi-batch mode#312

Merged
tabokie merged 1 commit intotikv:6.29.tikvfrom
tabokie:fix-empty-wb
Aug 8, 2022
Merged

fix writing empty batch in multi-batch mode#312
tabokie merged 1 commit intotikv:6.29.tikvfrom
tabokie:fix-empty-wb

Conversation

@tabokie
Copy link
Member

@tabokie tabokie commented Aug 8, 2022

In this line, memtable_write_cnt is incremented regardless of whether the write batch is empty.

memtable_write_cnt++;

It's then added to the global pending_memtable_writes_ counter:

pending_memtable_writes_ += memtable_write_cnt;

However, when it's empty, the request won't be enqueued to the commit queue, and the counter will never be decremented properly here:

void DBImpl::MultiBatchWriteCommit(CommitRequest* request) {
write_thread_.ExitWaitSequenceCommit(request, &versions_->last_sequence_);
size_t pending_cnt = pending_memtable_writes_.fetch_sub(1) - 1;

As a result, a later write will potentially block on WaitForPendingWrites forever.

Signed-off-by: tabokie xy.tao@outlook.com

Signed-off-by: tabokie <xy.tao@outlook.com>
Copy link
Member

@Connor1996 Connor1996 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about

if (count == 0) continue;

@tabokie
Copy link
Member Author

tabokie commented Aug 8, 2022

@Connor1996 Ideally we should only handle memtable logic in this block. Adding a continue would also skip the total_bytes_size calculation below (even though this calculation is unnecessary either, but the decision should not be made here).

Copy link
Member

@Connor1996 Connor1996 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@ethercflow ethercflow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@tabokie tabokie requested a review from BusyJay August 8, 2022 09:11
@tabokie tabokie merged commit f4cba2f into tikv:6.29.tikv Aug 8, 2022
@tabokie tabokie deleted the fix-empty-wb branch August 8, 2022 11:35
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.

4 participants