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/10/23 23:43:22 UTC

[kudu-CR] tablet: fix handling of ghost rows when merging MRSs

Andrew Wong has uploaded this change for review. ( http://gerrit.cloudera.org:8080/16643


Change subject: tablet: fix handling of ghost rows when merging MRSs
......................................................................

tablet: fix handling of ghost rows when merging MRSs

We would previously designate a nullptr to keep the memory used by an
MRS's compaction input RowBlock during compaction. We had a memory
buffer available, but we would previously not point the RowBlock at it
correctly, even though the rest of the compaction code used the correct
memory buffer.

This is inconsequential today because we only try to dereference the
memory buffer from the RowBlock when trying to merge histories of rows
that are strewn across multiple rowsets (e.g. from deletes followed by
inserts of the same rows). Today, we never merge multiple MRSs, we only
merge multiple DRSs, or flush a single MRS. However, I have a patch
coming up that will exercise this.

F1023 16:38:14.955049 40525 compaction.cc:672] Check failed: arena != nullptr Arena can't be null
*** Check failure stack trace: ***
*** Aborted at 1603496294 (unix time) try "date -d @1603496294" if you are using GNU date ***
PC: @     0x7f86791fc1d7 __GI_raise
*** SIGABRT (@0x111700009e4d) received by PID 40525 (TID 0x7f86866719c0) from PID 40525; stack trace: ***
    @     0x7f8682718370 (unknown)
    @     0x7f86791fc1d7 __GI_raise
    @     0x7f86791fd8c8 __GI_abort
    @     0x7f867bd847b9 google::logging_fail()
    @     0x7f867bd85f8d google::LogMessage::Fail()
    @     0x7f867bd87ee3 google::LogMessage::SendToLog()
    @     0x7f867bd85ae9 google::LogMessage::Flush()
    @     0x7f867bd8886f google::LogMessageFatal::~LogMessageFatal()
    @     0x7f8685523a59 kudu::tablet::(anonymous namespace)::MergeCompactionInput::SetPreviousGhost()
    @     0x7f8685522b45 kudu::tablet::(anonymous namespace)::MergeCompactionInput::PrepareBlock()
    @     0x7f8685527c08 kudu::tablet::FlushCompactionInput()
    @           0x47f047 kudu::tablet::TestCompaction::DoFlushAndReopen()
    @           0x46a954 kudu::tablet::TestCompaction_TestMergeMRS_Test::TestBody()
    @     0x7f86859c6b39 testing::internal::HandleExceptionsInMethodIfSupported<>()
    @     0x7f86859b804f testing::Test::Run()
    @     0x7f86859b810d testing::TestInfo::Run()
    @     0x7f86859b8225 testing::TestCase::Run()
    @     0x7f86859b84e8 testing::internal::UnitTestImpl::RunAllTests()
    @     0x7f86859b8789 testing::UnitTest::Run()
    @     0x7f8686070cbd RUN_ALL_TESTS()
    @     0x7f868606ec91 main
    @     0x7f86791e8b35 __libc_start_main
    @           0x460df9 (unknown)
Aborted

Change-Id: I539b9f67ccb2e05cac7eccf75fce89f0359c8ca9
---
M src/kudu/common/rowblock.h
M src/kudu/tablet/compaction-test.cc
M src/kudu/tablet/compaction.cc
3 files changed, 48 insertions(+), 30 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I539b9f67ccb2e05cac7eccf75fce89f0359c8ca9
Gerrit-Change-Number: 16643
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>

[kudu-CR] tablet: fix handling of ghost rows when merging MRSs

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16643 )

Change subject: tablet: fix handling of ghost rows when merging MRSs
......................................................................


Patch Set 1: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/16643
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I539b9f67ccb2e05cac7eccf75fce89f0359c8ca9
Gerrit-Change-Number: 16643
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Sat, 24 Oct 2020 01:29:39 +0000
Gerrit-HasComments: No

[kudu-CR] tablet: fix handling of ghost rows when merging MRSs

Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/16643 )

Change subject: tablet: fix handling of ghost rows when merging MRSs
......................................................................

tablet: fix handling of ghost rows when merging MRSs

We would previously designate a nullptr to keep the memory used by an
MRS's compaction input RowBlock during compaction. We had a memory
buffer available, but we would previously not point the RowBlock at it
correctly, even though the rest of the compaction code used the correct
memory buffer.

This is inconsequential today because we only try to dereference the
memory buffer from the RowBlock when trying to merge histories of rows
that are strewn across multiple rowsets (e.g. from deletes followed by
inserts of the same rows). Today, we never merge multiple MRSs, we only
merge multiple DRSs, or flush a single MRS. However, I have a patch
coming up that will exercise this.

F1023 16:38:14.955049 40525 compaction.cc:672] Check failed: arena != nullptr Arena can't be null
*** Check failure stack trace: ***
*** Aborted at 1603496294 (unix time) try "date -d @1603496294" if you are using GNU date ***
PC: @     0x7f86791fc1d7 __GI_raise
*** SIGABRT (@0x111700009e4d) received by PID 40525 (TID 0x7f86866719c0) from PID 40525; stack trace: ***
    @     0x7f8682718370 (unknown)
    @     0x7f86791fc1d7 __GI_raise
    @     0x7f86791fd8c8 __GI_abort
    @     0x7f867bd847b9 google::logging_fail()
    @     0x7f867bd85f8d google::LogMessage::Fail()
    @     0x7f867bd87ee3 google::LogMessage::SendToLog()
    @     0x7f867bd85ae9 google::LogMessage::Flush()
    @     0x7f867bd8886f google::LogMessageFatal::~LogMessageFatal()
    @     0x7f8685523a59 kudu::tablet::(anonymous namespace)::MergeCompactionInput::SetPreviousGhost()
    @     0x7f8685522b45 kudu::tablet::(anonymous namespace)::MergeCompactionInput::PrepareBlock()
    @     0x7f8685527c08 kudu::tablet::FlushCompactionInput()
    @           0x47f047 kudu::tablet::TestCompaction::DoFlushAndReopen()
    @           0x46a954 kudu::tablet::TestCompaction_TestMergeMRS_Test::TestBody()
    @     0x7f86859c6b39 testing::internal::HandleExceptionsInMethodIfSupported<>()
    @     0x7f86859b804f testing::Test::Run()
    @     0x7f86859b810d testing::TestInfo::Run()
    @     0x7f86859b8225 testing::TestCase::Run()
    @     0x7f86859b84e8 testing::internal::UnitTestImpl::RunAllTests()
    @     0x7f86859b8789 testing::UnitTest::Run()
    @     0x7f8686070cbd RUN_ALL_TESTS()
    @     0x7f868606ec91 main
    @     0x7f86791e8b35 __libc_start_main
    @           0x460df9 (unknown)
Aborted

Change-Id: I539b9f67ccb2e05cac7eccf75fce89f0359c8ca9
Reviewed-on: http://gerrit.cloudera.org:8080/16643
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin <as...@cloudera.com>
---
M src/kudu/common/rowblock.h
M src/kudu/tablet/compaction-test.cc
M src/kudu/tablet/compaction.cc
3 files changed, 48 insertions(+), 30 deletions(-)

Approvals:
  Kudu Jenkins: Verified
  Alexey Serbin: Looks good to me, approved

-- 
To view, visit http://gerrit.cloudera.org:8080/16643
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I539b9f67ccb2e05cac7eccf75fce89f0359c8ca9
Gerrit-Change-Number: 16643
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: Kudu Jenkins (120)