You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Todd Lipcon (Code Review)" <ge...@cloudera.org> on 2016/09/08 03:39:04 UTC
[kudu-CR] KUDU-1135 (part 1): avoid flushing cmeta to disk twice when voting
Hello David Ribeiro Alves, Mike Percy,
I'd like you to do a code review. Please visit
http://gerrit.cloudera.org:8080/4333
to review the following change.
Change subject: KUDU-1135 (part 1): avoid flushing cmeta to disk twice when voting
......................................................................
KUDU-1135 (part 1): avoid flushing cmeta to disk twice when voting
This adds a flag to the term advancement calls which prevents it from
flushing to disk. We use this flag during voting when we know we are
about to flush our vote to disk immediately following the term
advancement.
This is part 1: another patch will do the same optimization for the case
of starting an election.
Change-Id: Iecc55bc9e96dcdc918ede1190b7cbac719f95506
---
M src/kudu/consensus/consensus_meta.cc
M src/kudu/consensus/consensus_meta.h
M src/kudu/consensus/raft_consensus.cc
M src/kudu/consensus/raft_consensus.h
M src/kudu/consensus/raft_consensus_quorum-test.cc
M src/kudu/consensus/raft_consensus_state.cc
M src/kudu/consensus/raft_consensus_state.h
7 files changed, 80 insertions(+), 14 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/33/4333/1
--
To view, visit http://gerrit.cloudera.org:8080/4333
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Iecc55bc9e96dcdc918ede1190b7cbac719f95506
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
[kudu-CR] KUDU-1135 (part 1): avoid flushing cmeta to disk twice when voting
Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change.
Change subject: KUDU-1135 (part 1): avoid flushing cmeta to disk twice when voting
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/4333/1/src/kudu/consensus/raft_consensus.cc
File src/kudu/consensus/raft_consensus.cc:
Line 1362: // our vote.
> I personally prefer wrapping comments at 80 chars unless there is a reason
I guess it's all subjective, but usually since I already resized my windows/terms to the possible size of the code then having shorter comments provides me with no benefit, while having less lines allows me to see more code without scrolling.
--
To view, visit http://gerrit.cloudera.org:8080/4333
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Iecc55bc9e96dcdc918ede1190b7cbac719f95506
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1135 (part 1): avoid flushing cmeta to disk twice when voting
Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change.
Change subject: KUDU-1135 (part 1): avoid flushing cmeta to disk twice when voting
......................................................................
Patch Set 1: Verified+1
unrelated flake
--
To view, visit http://gerrit.cloudera.org:8080/4333
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Iecc55bc9e96dcdc918ede1190b7cbac719f95506
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: No
[kudu-CR] KUDU-1135 (part 1): avoid flushing cmeta to disk twice when voting
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has submitted this change and it was merged.
Change subject: KUDU-1135 (part 1): avoid flushing cmeta to disk twice when voting
......................................................................
KUDU-1135 (part 1): avoid flushing cmeta to disk twice when voting
This adds a flag to the term advancement calls which prevents it from
flushing to disk. We use this flag during voting when we know we are
about to flush our vote to disk immediately following the term
advancement.
This is part 1: another patch will do the same optimization for the case
of starting an election.
Change-Id: Iecc55bc9e96dcdc918ede1190b7cbac719f95506
Reviewed-on: http://gerrit.cloudera.org:8080/4333
Reviewed-by: Mike Percy <mp...@apache.org>
Reviewed-by: David Ribeiro Alves <dr...@apache.org>
Tested-by: David Ribeiro Alves <dr...@apache.org>
---
M src/kudu/consensus/consensus_meta.cc
M src/kudu/consensus/consensus_meta.h
M src/kudu/consensus/raft_consensus.cc
M src/kudu/consensus/raft_consensus.h
M src/kudu/consensus/raft_consensus_quorum-test.cc
M src/kudu/consensus/raft_consensus_state.cc
M src/kudu/consensus/raft_consensus_state.h
7 files changed, 80 insertions(+), 14 deletions(-)
Approvals:
David Ribeiro Alves: Looks good to me, approved; Verified
Mike Percy: Looks good to me, but someone else must approve
--
To view, visit http://gerrit.cloudera.org:8080/4333
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Iecc55bc9e96dcdc918ede1190b7cbac719f95506
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] KUDU-1135 (part 1): avoid flushing cmeta to disk twice when voting
Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change.
Change subject: KUDU-1135 (part 1): avoid flushing cmeta to disk twice when voting
......................................................................
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/4333/1/src/kudu/consensus/raft_consensus.cc
File src/kudu/consensus/raft_consensus.cc:
Line 1362: // our vote.
nit: no need to wrap
http://gerrit.cloudera.org:8080/#/c/4333/1/src/kudu/consensus/raft_consensus_state.h
File src/kudu/consensus/raft_consensus_state.h:
Line 198: enum FlushToDisk {
maybe rename this argument to FlushMetaToDisk to make sure it's relevant for meta only
--
To view, visit http://gerrit.cloudera.org:8080/4333
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Iecc55bc9e96dcdc918ede1190b7cbac719f95506
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1135 (part 1): avoid flushing cmeta to disk twice when voting
Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.
Kudu Jenkins has posted comments on this change.
Change subject: KUDU-1135 (part 1): avoid flushing cmeta to disk twice when voting
......................................................................
Patch Set 1:
Build Started http://104.196.14.100/job/kudu-gerrit/3278/
--
To view, visit http://gerrit.cloudera.org:8080/4333
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Iecc55bc9e96dcdc918ede1190b7cbac719f95506
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: No
[kudu-CR] KUDU-1135 (part 1): avoid flushing cmeta to disk twice when voting
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change.
Change subject: KUDU-1135 (part 1): avoid flushing cmeta to disk twice when voting
......................................................................
Patch Set 1: Code-Review+1
(1 comment)
lgtm
http://gerrit.cloudera.org:8080/#/c/4333/1/src/kudu/consensus/raft_consensus.cc
File src/kudu/consensus/raft_consensus.cc:
Line 1362: // our vote.
> nit: no need to wrap
I personally prefer wrapping comments at 80 chars unless there is a reason not to, and that's what I usually do. It's slightly easier to read. But 80 chars is often too short for code...
--
To view, visit http://gerrit.cloudera.org:8080/4333
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Iecc55bc9e96dcdc918ede1190b7cbac719f95506
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1135 (part 1): avoid flushing cmeta to disk twice when voting
Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change.
Change subject: KUDU-1135 (part 1): avoid flushing cmeta to disk twice when voting
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/4333
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Iecc55bc9e96dcdc918ede1190b7cbac719f95506
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: David Ribeiro Alves <dr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: No