Hello Alexey Serbin, Grant Henke,

I'd like you to do a code review. Please visit

    http://gerrit.cloudera.org:8080/16691

to review the following change.


Change subject: tablet: support merge compacting a MRS with invisible rows
......................................................................

tablet: support merge compacting a MRS with invisible rows

In testing an upcoming patch, I hit a check failure when merging
multiple MemRowSets in which some rows are effectively invisible (i.e.
inserted and deleted in the same write request).

F1030 18:30:04.936852 96696 compaction.cc:413] Check failed: !empty() 0 of 0
*** Check failure stack trace: ***
*** Aborted at 1604107804 (unix time) try "date -d @1604107804" if you are 
using GNU date ***
PC: @     0x7f7ca8b311d7 __GI_raise
*** SIGABRT (@0x1117000179b8) received by PID 96696 (TID 0x7f7cb5fcb9c0) from 
PID 96696; stack trace: ***
    @     0x7f7cb204e370 (unknown)
    @     0x7f7ca8b311d7 __GI_raise
    @     0x7f7ca8b328c8 __GI_abort
    @     0x7f7cab6b97b9 google::logging_fail()
    @     0x7f7cab6baf8d google::LogMessage::Fail()
    @     0x7f7cab6bcee3 google::LogMessage::SendToLog()
    @     0x7f7cab6baae9 google::LogMessage::Flush()
    @     0x7f7cab6bd86f google::LogMessageFatal::~LogMessageFatal()
    @     0x7f7cb4e751b1 kudu::tablet::(anonymous 
namespace)::MergeCompactionInput::MergeState::Dominates()
    @     0x7f7cb4e76dbf kudu::tablet::(anonymous 
namespace)::MergeCompactionInput::TryInsertIntoDominanceList()
    @     0x7f7cb4e76bea kudu::tablet::(anonymous 
namespace)::MergeCompactionInput::ProcessEmptyInputs()
    @     0x7f7cb4e7565f kudu::tablet::(anonymous 
namespace)::MergeCompactionInput::Init()
    @     0x7f7cb4e7b0e5 kudu::tablet::FlushCompactionInput()
    @           0x48020d kudu::tablet::TestCompaction::DoFlushAndReopen()
    @           0x46bcfc 
kudu::tablet::TestCompaction_TestMergeMRSWithInvisibleRows_Test::TestBody()
    @     0x7f7cb531fb39 
testing::internal::HandleExceptionsInMethodIfSupported<>()
    @     0x7f7cb531104f testing::Test::Run()
    @     0x7f7cb531110d testing::TestInfo::Run()
    @     0x7f7cb5311225 testing::TestCase::Run()
    @     0x7f7cb53114e8 testing::internal::UnitTestImpl::RunAllTests()
    @     0x7f7cb5311789 testing::UnitTest::Run()
    @     0x7f7cb59cacbd RUN_ALL_TESTS()
    @     0x7f7cb59c8c91 main
    @     0x7f7ca8b1db35 __libc_start_main
    @           0x461509 (unknown)
Aborted

The issue is that when we iterate through the merge compaction states,
we would previously only cull the input merge state (i.e. the state
representing a rowset being merged) if it had no blocks to read. Based
on this, compaction code assumed that no inputs could possible be empty.
However, it did not take into account the case where an input has
blocks, but the rowset is effectively empty because rows were inserted
and deleted in the same op. In such cases,
MemRowSetCompactionInput::PrepareBlock() leaves the input in an empty
state if there are no further blocks, hence the crash.

This updates the behavior to check for both blocks _and_ emptiness
following the block preparation.

NOTE: this code path isn't exercised today, but it will be in an
upcoming patch that will merge multiple MRSs from multiple transactions.

Change-Id: I9016f2cd3714a4ac7b5fb1abd80b9e6bd86d5ac5
---
M src/kudu/tablet/compaction-test.cc
M src/kudu/tablet/compaction.cc
2 files changed, 73 insertions(+), 18 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/91/16691/1
--
To view, visit http://gerrit.cloudera.org:8080/16691
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I9016f2cd3714a4ac7b5fb1abd80b9e6bd86d5ac5
Gerrit-Change-Number: 16691
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Wong <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Grant Henke <[email protected]>

Reply via email to