You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Andrew Wong (Code Review)" <ge...@cloudera.org> on 2020/11/03 00:03:00 UTC
[kudu-CR] tablet: support merge compacting a MRS with invisible rows
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 <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
[kudu-CR] tablet: support merge compacting a MRS with invisible rows
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/16691 )
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
Reviewed-on: http://gerrit.cloudera.org:8080/16691
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin <as...@cloudera.com>
---
M src/kudu/tablet/compaction-test.cc
M src/kudu/tablet/compaction.cc
2 files changed, 73 insertions(+), 18 deletions(-)
Approvals:
Kudu Jenkins: Verified
Alexey Serbin: Looks good to me, approved
--
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: merged
Gerrit-Change-Id: I9016f2cd3714a4ac7b5fb1abd80b9e6bd86d5ac5
Gerrit-Change-Number: 16691
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] tablet: support merge compacting a MRS with invisible rows
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16691 )
Change subject: tablet: support merge compacting a MRS with invisible rows
......................................................................
Patch Set 1: Code-Review+2
--
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: comment
Gerrit-Change-Id: I9016f2cd3714a4ac7b5fb1abd80b9e6bd86d5ac5
Gerrit-Change-Number: 16691
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 03 Nov 2020 01:05:13 +0000
Gerrit-HasComments: No