You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Alexey Serbin (Code Review)" <ge...@cloudera.org> on 2018/07/20 01:49:12 UTC
[kudu-CR] WIP: KUDU-2509 fix use-after-free in case of WAL replay error
Alexey Serbin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/10997
Change subject: WIP: KUDU-2509 fix use-after-free in case of WAL replay error
......................................................................
WIP: KUDU-2509 fix use-after-free in case of WAL replay error
Fixed use-after-free mistake in case of an failure to apply commit
or replicate message from WAL while bootstrapping a tablet. The
use-after-free condition happened in some rare cases.
In addition, reduced number of memory allocations while reading WAL
entries from disk during tablet bootstrap. Also did other unsorted
minor changes in tablet_bootstrap.cc and around like replacing
gscoped_ptr with unique_ptr, etc.
WIP: need to add good repro scenario for the bug, which does not seem to
be very easy.
Change-Id: I11373b1cc34d9e2e0181bee2d3841b49022218ed
---
M src/kudu/consensus/log_util.cc
M src/kudu/consensus/log_util.h
M src/kudu/integration-tests/log_verifier.cc
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tools/tool_action_common.cc
M src/kudu/tools/tool_action_local_replica.cc
6 files changed, 144 insertions(+), 133 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/97/10997/1
--
To view, visit http://gerrit.cloudera.org:8080/10997
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I11373b1cc34d9e2e0181bee2d3841b49022218ed
Gerrit-Change-Number: 10997
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
[kudu-CR] KUDU-2509 fix use-after-free in case of WAL replay error
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Mike Percy, Kudu Jenkins, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/10997
to look at the new patch set (#4).
Change subject: KUDU-2509 fix use-after-free in case of WAL replay error
......................................................................
KUDU-2509 fix use-after-free in case of WAL replay error
Fixed use-after-free mistake in case of an failure to apply a pending
commit message from the WAL while bootstrapping a tablet.
Also, a repro scenario to expose the use-after-free condition is added.
Prior to the fix, the repro scenario would crash with SIGSEGV on Linux
or with SIGBUS on OS X (at least for DEBUG builds).
Change-Id: I11373b1cc34d9e2e0181bee2d3841b49022218ed
---
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_bootstrap.cc
2 files changed, 81 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/97/10997/4
--
To view, visit http://gerrit.cloudera.org:8080/10997
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I11373b1cc34d9e2e0181bee2d3841b49022218ed
Gerrit-Change-Number: 10997
Gerrit-PatchSet: 4
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
[kudu-CR] WIP: KUDU-2509 fix use-after-free in case of WAL replay error
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/10997 )
Change subject: WIP: KUDU-2509 fix use-after-free in case of WAL replay error
......................................................................
Patch Set 2:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/10997/2//COMMIT_MSG
Commit Message:
PS2:
> Could you prepare a second version of this patch that's much more targeted,
That's a very good idea. Done.
http://gerrit.cloudera.org:8080/#/c/10997/2/src/kudu/tablet/tablet_bootstrap.cc
File src/kudu/tablet/tablet_bootstrap.cc:
http://gerrit.cloudera.org:8080/#/c/10997/2/src/kudu/tablet/tablet_bootstrap.cc@629
PS2, Line 629: tablet_.swap(tablet);
> Nit: I tend to prefer this style:
I think using std::move() instead of swap() is a better choice when the target object is not yet constructed. Maybe, sometimes it's important to destroy the lhs prior to the assignment, so swap() would not be an option. And if we want to invalidate the rhs, std::move() is also a better choice.
From the other side, sometimes the move assignment operator is implemented using swap() (and that seems to be true for scoped_refptr()). However, that's not always the case, and probably in such cases using std::move() is better.
I also found this piece: https://stackoverflow.com/questions/6687388/why-do-some-people-use-swap-for-move-assignments
OK, I'll replace it with std::move() in the dedicated changelist (this one will be re-targeted for back-porting into other branches).
http://gerrit.cloudera.org:8080/#/c/10997/2/src/kudu/tablet/tablet_bootstrap.cc@891
PS2, Line 891: entry_ptr->release();
> Can you change OpIndexToEntryMap to store unique_ptr<LogEntryPB> for the va
I looked at that briefly, and that seems doable, yes. I just didn't want to broaden this changelist too much, however now when we have a localized fix targeted for backporting, I think I'll do that as well.
--
To view, visit http://gerrit.cloudera.org:8080/10997
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I11373b1cc34d9e2e0181bee2d3841b49022218ed
Gerrit-Change-Number: 10997
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Fri, 20 Jul 2018 05:09:22 +0000
Gerrit-HasComments: Yes
[kudu-CR] WIP: KUDU-2509 fix use-after-free in case of WAL replay error
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Mike Percy, Kudu Jenkins, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/10997
to look at the new patch set (#3).
Change subject: WIP: KUDU-2509 fix use-after-free in case of WAL replay error
......................................................................
WIP: KUDU-2509 fix use-after-free in case of WAL replay error
Fixed use-after-free mistake in case of an failure to apply commit
or replicate message from WAL while bootstrapping a tablet. The
use-after-free condition happened in some rare cases.
WIP: need to add a repro scenario for the bug, which does not seem to
be easy.
Change-Id: I11373b1cc34d9e2e0181bee2d3841b49022218ed
---
M src/kudu/tablet/tablet_bootstrap.cc
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/97/10997/3
--
To view, visit http://gerrit.cloudera.org:8080/10997
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I11373b1cc34d9e2e0181bee2d3841b49022218ed
Gerrit-Change-Number: 10997
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
[kudu-CR] KUDU-2509 fix use-after-free in case of WAL replay error
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/10997 )
Change subject: KUDU-2509 fix use-after-free in case of WAL replay error
......................................................................
Patch Set 4:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/10997/4//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/10997/4//COMMIT_MSG@9
PS4, Line 9: an
> Nit: a
Done
http://gerrit.cloudera.org:8080/#/c/10997/4/src/kudu/tablet/tablet_bootstrap-test.cc
File src/kudu/tablet/tablet_bootstrap-test.cc:
http://gerrit.cloudera.org:8080/#/c/10997/4/src/kudu/tablet/tablet_bootstrap-test.cc@699
PS4, Line 699: true
> This is the default value, no? Also, don't we need a NO_FATALS() wrapper ar
It's a good call. Indeed, that's necessary if we want to spot an error: I used just bland copy-paste technique in this place. I'll add an additional patch to correct that in other test scenarios in this test.
http://gerrit.cloudera.org:8080/#/c/10997/4/src/kudu/tablet/tablet_bootstrap-test.cc@707
PS4, Line 707: const auto schema = Schema({ ColumnSchema("key", INT32),
> Can you refactor LogTestBase a bit so you don't need to redescribe schema_
I think it's not re-factoring LogTestBase just for this tiny thing. However, I updated the code of the test so now it's easy to follow the idea behind.
http://gerrit.cloudera.org:8080/#/c/10997/4/src/kudu/tablet/tablet_bootstrap-test.cc@728
PS4, Line 728: TxResultPB* result = mutate_commit->mutable_result();
: OperationResultPB* mutate = result->add_ops();
: MemStoreTargetPB* target = mutate->add_mutated_stores();
: target->set_mrs_id(1);
> Maybe combine into one line?
Done
http://gerrit.cloudera.org:8080/#/c/10997/4/src/kudu/tablet/tablet_bootstrap-test.cc@738
PS4, Line 738: result = insert_commit->mutable_result();
: OperationResultPB* insert = result->add_ops();
: target = insert->add_mutated_stores();
: target->set_mrs_id(1);
> Combine this too?
Done
--
To view, visit http://gerrit.cloudera.org:8080/10997
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I11373b1cc34d9e2e0181bee2d3841b49022218ed
Gerrit-Change-Number: 10997
Gerrit-PatchSet: 4
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Tue, 24 Jul 2018 00:51:29 +0000
Gerrit-HasComments: Yes
[kudu-CR] WIP: KUDU-2509 fix use-after-free in case of WAL replay error
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/10997 )
Change subject: WIP: KUDU-2509 fix use-after-free in case of WAL replay error
......................................................................
Patch Set 2:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/10997/2//COMMIT_MSG
Commit Message:
PS2:
Could you prepare a second version of this patch that's much more targeted, one we could more easily backport to older releases?
http://gerrit.cloudera.org:8080/#/c/10997/2/src/kudu/consensus/log_util.cc
File src/kudu/consensus/log_util.cc:
http://gerrit.cloudera.org:8080/#/c/10997/2/src/kudu/consensus/log_util.cc@186
PS2, Line 186: pending_entries_.front().swap(*entry);
Nit: *entry = std::move(pending_entries_.front())
http://gerrit.cloudera.org:8080/#/c/10997/2/src/kudu/tablet/tablet_bootstrap.cc
File src/kudu/tablet/tablet_bootstrap.cc:
http://gerrit.cloudera.org:8080/#/c/10997/2/src/kudu/tablet/tablet_bootstrap.cc@629
PS2, Line 629: tablet_.swap(tablet);
Nit: I tend to prefer this style:
tablet_ = std::move(tablet);
Thoughts?
http://gerrit.cloudera.org:8080/#/c/10997/2/src/kudu/tablet/tablet_bootstrap.cc@891
PS2, Line 891: entry_ptr->release();
Can you change OpIndexToEntryMap to store unique_ptr<LogEntryPB> for the value type? Seems like that would simplify some of this too, though maybe it's not possible (i.e. LogEntryPB is actually shared ownership), or maybe it's too big of a change.
--
To view, visit http://gerrit.cloudera.org:8080/10997
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I11373b1cc34d9e2e0181bee2d3841b49022218ed
Gerrit-Change-Number: 10997
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Fri, 20 Jul 2018 02:52:41 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2509 fix use-after-free in case of WAL replay error
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/10997 )
Change subject: KUDU-2509 fix use-after-free in case of WAL replay error
......................................................................
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/10997/4/src/kudu/tablet/tablet_bootstrap-test.cc
File src/kudu/tablet/tablet_bootstrap-test.cc:
http://gerrit.cloudera.org:8080/#/c/10997/4/src/kudu/tablet/tablet_bootstrap-test.cc@707
PS4, Line 707: const auto schema = Schema({ ColumnSchema("key", INT32),
> I think it's not re-factoring LogTestBase just for this tiny thing. Howeve
s/it's not re-factoring/it's not worth re-factoring/
--
To view, visit http://gerrit.cloudera.org:8080/10997
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I11373b1cc34d9e2e0181bee2d3841b49022218ed
Gerrit-Change-Number: 10997
Gerrit-PatchSet: 4
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Tue, 24 Jul 2018 00:52:24 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2509 fix use-after-free in case of WAL replay error
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Mike Percy, Kudu Jenkins, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/10997
to look at the new patch set (#5).
Change subject: KUDU-2509 fix use-after-free in case of WAL replay error
......................................................................
KUDU-2509 fix use-after-free in case of WAL replay error
Fixed use-after-free mistake in case of a failure to apply a pending
commit message from the WAL while bootstrapping a tablet.
Also, a repro scenario to expose the use-after-free condition is added.
Prior to the fix, the repro scenario would crash with SIGSEGV on Linux
or with SIGBUS on OS X (at least for DEBUG builds).
Change-Id: I11373b1cc34d9e2e0181bee2d3841b49022218ed
---
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_bootstrap.cc
2 files changed, 74 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/97/10997/5
--
To view, visit http://gerrit.cloudera.org:8080/10997
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I11373b1cc34d9e2e0181bee2d3841b49022218ed
Gerrit-Change-Number: 10997
Gerrit-PatchSet: 5
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
[kudu-CR] KUDU-2509 fix use-after-free in case of WAL replay error
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has removed Kudu Jenkins from this change. ( http://gerrit.cloudera.org:8080/10997 )
Change subject: KUDU-2509 fix use-after-free in case of WAL replay error
......................................................................
Removed reviewer Kudu Jenkins with the following votes:
* Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/10997
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteReviewer
Gerrit-Change-Id: I11373b1cc34d9e2e0181bee2d3841b49022218ed
Gerrit-Change-Number: 10997
Gerrit-PatchSet: 5
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
[kudu-CR] WIP: KUDU-2509 fix use-after-free in case of WAL replay error
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/10997 )
Change subject: WIP: KUDU-2509 fix use-after-free in case of WAL replay error
......................................................................
Patch Set 3: Code-Review+2
But if you can conceive of a reasonable test, please add that too.
--
To view, visit http://gerrit.cloudera.org:8080/10997
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I11373b1cc34d9e2e0181bee2d3841b49022218ed
Gerrit-Change-Number: 10997
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Fri, 20 Jul 2018 04:30:27 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-2509 fix use-after-free in case of WAL replay error
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/10997 )
Change subject: KUDU-2509 fix use-after-free in case of WAL replay error
......................................................................
Patch Set 4:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/10997/4//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/10997/4//COMMIT_MSG@9
PS4, Line 9: an
Nit: a
http://gerrit.cloudera.org:8080/#/c/10997/4/src/kudu/tablet/tablet_bootstrap-test.cc
File src/kudu/tablet/tablet_bootstrap-test.cc:
http://gerrit.cloudera.org:8080/#/c/10997/4/src/kudu/tablet/tablet_bootstrap-test.cc@699
PS4, Line 699: true
This is the default value, no? Also, don't we need a NO_FATALS() wrapper around these calls?
http://gerrit.cloudera.org:8080/#/c/10997/4/src/kudu/tablet/tablet_bootstrap-test.cc@707
PS4, Line 707: const auto schema = Schema({ ColumnSchema("key", INT32),
Can you refactor LogTestBase a bit so you don't need to redescribe schema_ in its entirety? Maybe a method that returns the column schemas that go into schema_, then you could add just the one column to it here? Also it'd document the intent.
http://gerrit.cloudera.org:8080/#/c/10997/4/src/kudu/tablet/tablet_bootstrap-test.cc@728
PS4, Line 728: TxResultPB* result = mutate_commit->mutable_result();
: OperationResultPB* mutate = result->add_ops();
: MemStoreTargetPB* target = mutate->add_mutated_stores();
: target->set_mrs_id(1);
Maybe combine into one line?
mutate_commit()->mutable_result()->add_ops()->add_mutated_stores()->set_mrs_id(1);
http://gerrit.cloudera.org:8080/#/c/10997/4/src/kudu/tablet/tablet_bootstrap-test.cc@738
PS4, Line 738: result = insert_commit->mutable_result();
: OperationResultPB* insert = result->add_ops();
: target = insert->add_mutated_stores();
: target->set_mrs_id(1);
Combine this too?
--
To view, visit http://gerrit.cloudera.org:8080/10997
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I11373b1cc34d9e2e0181bee2d3841b49022218ed
Gerrit-Change-Number: 10997
Gerrit-PatchSet: 4
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Mon, 23 Jul 2018 22:31:03 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2509 fix use-after-free in case of WAL replay error
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10997 )
Change subject: KUDU-2509 fix use-after-free in case of WAL replay error
......................................................................
KUDU-2509 fix use-after-free in case of WAL replay error
Fixed use-after-free mistake in case of a failure to apply a pending
commit message from the WAL while bootstrapping a tablet.
Also, a repro scenario to expose the use-after-free condition is added.
Prior to the fix, the repro scenario would crash with SIGSEGV on Linux
or with SIGBUS on OS X (at least for DEBUG builds).
Change-Id: I11373b1cc34d9e2e0181bee2d3841b49022218ed
Reviewed-on: http://gerrit.cloudera.org:8080/10997
Tested-by: Alexey Serbin <as...@cloudera.com>
Reviewed-by: Adar Dembo <ad...@cloudera.com>
---
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tablet/tablet_bootstrap.cc
2 files changed, 74 insertions(+), 3 deletions(-)
Approvals:
Alexey Serbin: Verified
Adar Dembo: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/10997
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I11373b1cc34d9e2e0181bee2d3841b49022218ed
Gerrit-Change-Number: 10997
Gerrit-PatchSet: 6
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
[kudu-CR] WIP: KUDU-2509 fix use-after-free in case of WAL replay error
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Mike Percy, Kudu Jenkins, Adar Dembo,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/10997
to look at the new patch set (#2).
Change subject: WIP: KUDU-2509 fix use-after-free in case of WAL replay error
......................................................................
WIP: KUDU-2509 fix use-after-free in case of WAL replay error
Fixed use-after-free mistake in case of an failure to apply commit
or replicate message from WAL while bootstrapping a tablet. The
use-after-free condition happened in some rare cases.
In addition, reduced number of memory allocations while reading WAL
entries from disk during tablet bootstrap. Also did other unsorted
minor changes in tablet_bootstrap.cc and around like replacing
gscoped_ptr with unique_ptr, etc.
WIP: need to add good repro scenario for the bug, which does not seem to
be very easy.
Change-Id: I11373b1cc34d9e2e0181bee2d3841b49022218ed
---
M src/kudu/consensus/log_util.cc
M src/kudu/consensus/log_util.h
M src/kudu/integration-tests/log_verifier.cc
M src/kudu/tablet/tablet_bootstrap.cc
M src/kudu/tools/tool_action_common.cc
M src/kudu/tools/tool_action_local_replica.cc
6 files changed, 142 insertions(+), 134 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/97/10997/2
--
To view, visit http://gerrit.cloudera.org:8080/10997
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I11373b1cc34d9e2e0181bee2d3841b49022218ed
Gerrit-Change-Number: 10997
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
[kudu-CR] KUDU-2509 fix use-after-free in case of WAL replay error
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/10997 )
Change subject: KUDU-2509 fix use-after-free in case of WAL replay error
......................................................................
Patch Set 5: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/10997
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I11373b1cc34d9e2e0181bee2d3841b49022218ed
Gerrit-Change-Number: 10997
Gerrit-PatchSet: 5
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Tue, 24 Jul 2018 04:35:28 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-2509 fix use-after-free in case of WAL replay error
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/10997 )
Change subject: KUDU-2509 fix use-after-free in case of WAL replay error
......................................................................
Patch Set 5: Verified+1
Unrelated flakes in TSAN build:
StopTablets/StopTabletITest.TestStoppedTabletsScansGetRedirected/0
org.apache.kudu.backup.TestKuduBackup.classMethod
--
To view, visit http://gerrit.cloudera.org:8080/10997
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I11373b1cc34d9e2e0181bee2d3841b49022218ed
Gerrit-Change-Number: 10997
Gerrit-PatchSet: 5
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Comment-Date: Tue, 24 Jul 2018 02:54:50 +0000
Gerrit-HasComments: No