You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org> on 2018/01/30 18:46:41 UTC
[kudu-CR] Don't rely on pending config OpId index for peer promotion
David Ribeiro Alves has uploaded this change for review. ( http://gerrit.cloudera.org:8080/9161
Change subject: Don't rely on pending config OpId index for peer promotion
......................................................................
Don't rely on pending config OpId index for peer promotion
As part of the effort to make OpId and timestamp assignment atomic
we can't rely on a pending config having been assigned an OpId anymore.
This because the config is marked as the active config _before_ an OpId
is assigned later on, by the queue. The OpId only actually gets set when
the config is committed. Follow up patches will complete remove this
reliance.
In general this doesn't have much impact except where we're promoting a peer.
In this case we currently pass the current active config's term and index from
the queue back to consensus which then uses this info to perform the promotion
only when the current term and index match. We can't do this in the new
setting where the term and index are not available, but moreover this info
is not strictly needed for promotion at all, so this patch removes this
extra info being passed around.
The reasoning why the extra info is not needed is the following:
- If a peer' uuid is marked for promotion in the current committed
config, as verified directly through cmeta, and is considered up-to-date
enough to be promoted, it doesn't matter whether the active config in the
queue is the same as the committed config. The peer still can and should
be promoted.
- Essentially, omitting this extra info just means that we have to re-check
that the member type and attributes are still the expected ones, which is
not costly.
Change-Id: I3a033ce16f626458efdb59b1b4dd8450e8591761
---
M src/kudu/consensus/consensus_queue.cc
M src/kudu/consensus/consensus_queue.h
M src/kudu/consensus/raft_consensus.cc
M src/kudu/consensus/raft_consensus.h
4 files changed, 34 insertions(+), 54 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/61/9161/1
--
To view, visit http://gerrit.cloudera.org:8080/9161
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I3a033ce16f626458efdb59b1b4dd8450e8591761
Gerrit-Change-Number: 9161
Gerrit-PatchSet: 1
Gerrit-Owner: David Ribeiro Alves <da...@gmail.com>
[kudu-CR] Don't rely on pending config OpId index for peer promotion
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/9161 )
Change subject: Don't rely on pending config OpId index for peer promotion
......................................................................
Patch Set 3:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/9161/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/9161/3//COMMIT_MSG@13
PS3, Line 13: remove
nit: removing?
http://gerrit.cloudera.org:8080/#/c/9161/3/src/kudu/consensus/raft_consensus.cc
File src/kudu/consensus/raft_consensus.cc:
http://gerrit.cloudera.org:8080/#/c/9161/3/src/kudu/consensus/raft_consensus.cc@864
PS3, Line 864: req.set_cas_config_opid_index(current_committed_config_index);
Wouldn't it break the CAS semantics? As I understand, the CAS semantics assumes the configuration change should be done as exactly with the configuration it was planned. If it happens that the current committed configuration differs, how is it CAS semantics? Probably, I miss something, but this piece looks suspicious to me.
--
To view, visit http://gerrit.cloudera.org:8080/9161
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3a033ce16f626458efdb59b1b4dd8450e8591761
Gerrit-Change-Number: 9161
Gerrit-PatchSet: 3
Gerrit-Owner: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Fri, 02 Feb 2018 00:18:36 +0000
Gerrit-HasComments: Yes
[kudu-CR] Don't rely on pending config OpId index for peer promotion
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/9161 )
Change subject: Don't rely on pending config OpId index for peer promotion
......................................................................
Don't rely on pending config OpId index for peer promotion
As part of the effort to make OpId and timestamp assignment atomic
we can't rely on a pending config having been assigned an OpId anymore.
This because the config is marked as the active config _before_ an OpId
is assigned later on, by the queue. The OpId only actually gets set when
the config is committed. Follow up patches will complete remove this
reliance.
In general this doesn't have much impact except where we're promoting a peer.
In this case we currently pass the current active config's term and index from
the queue back to consensus which then uses this info to perform the promotion
only when the current term and index match. We can't do this in the new
setting where the term and index are not available, but moreover this info
is not strictly needed for promotion at all, so this patch removes this
extra info being passed around.
The reasoning why the extra info is not needed is the following:
- If a peer' uuid is marked for promotion in the current committed
config, as verified directly through cmeta, and is considered up-to-date
enough to be promoted, it doesn't matter whether the active config in the
queue is the same as the committed config. The peer still can and should
be promoted.
- Essentially, omitting this extra info just means that we have to re-check
that the member type and attributes are still the expected ones, which is
not costly.
Change-Id: I3a033ce16f626458efdb59b1b4dd8450e8591761
Reviewed-on: http://gerrit.cloudera.org:8080/9161
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin <as...@cloudera.com>
Reviewed-by: Mike Percy <mp...@apache.org>
---
M src/kudu/consensus/consensus_queue.cc
M src/kudu/consensus/consensus_queue.h
M src/kudu/consensus/raft_consensus.cc
M src/kudu/consensus/raft_consensus.h
4 files changed, 34 insertions(+), 54 deletions(-)
Approvals:
Kudu Jenkins: Verified
Alexey Serbin: Looks good to me, but someone else must approve
Mike Percy: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/9161
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I3a033ce16f626458efdb59b1b4dd8450e8591761
Gerrit-Change-Number: 9161
Gerrit-PatchSet: 4
Gerrit-Owner: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] Don't rely on pending config OpId index for peer promotion
Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/9161
to look at the new patch set (#3).
Change subject: Don't rely on pending config OpId index for peer promotion
......................................................................
Don't rely on pending config OpId index for peer promotion
As part of the effort to make OpId and timestamp assignment atomic
we can't rely on a pending config having been assigned an OpId anymore.
This because the config is marked as the active config _before_ an OpId
is assigned later on, by the queue. The OpId only actually gets set when
the config is committed. Follow up patches will complete remove this
reliance.
In general this doesn't have much impact except where we're promoting a peer.
In this case we currently pass the current active config's term and index from
the queue back to consensus which then uses this info to perform the promotion
only when the current term and index match. We can't do this in the new
setting where the term and index are not available, but moreover this info
is not strictly needed for promotion at all, so this patch removes this
extra info being passed around.
The reasoning why the extra info is not needed is the following:
- If a peer' uuid is marked for promotion in the current committed
config, as verified directly through cmeta, and is considered up-to-date
enough to be promoted, it doesn't matter whether the active config in the
queue is the same as the committed config. The peer still can and should
be promoted.
- Essentially, omitting this extra info just means that we have to re-check
that the member type and attributes are still the expected ones, which is
not costly.
Change-Id: I3a033ce16f626458efdb59b1b4dd8450e8591761
---
M src/kudu/consensus/consensus_queue.cc
M src/kudu/consensus/consensus_queue.h
M src/kudu/consensus/raft_consensus.cc
M src/kudu/consensus/raft_consensus.h
4 files changed, 34 insertions(+), 54 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/61/9161/3
--
To view, visit http://gerrit.cloudera.org:8080/9161
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3a033ce16f626458efdb59b1b4dd8450e8591761
Gerrit-Change-Number: 9161
Gerrit-PatchSet: 3
Gerrit-Owner: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] Don't rely on pending config OpId index for peer promotion
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/9161 )
Change subject: Don't rely on pending config OpId index for peer promotion
......................................................................
Patch Set 3: Code-Review+1
(1 comment)
http://gerrit.cloudera.org:8080/#/c/9161/3/src/kudu/consensus/raft_consensus.cc
File src/kudu/consensus/raft_consensus.cc:
http://gerrit.cloudera.org:8080/#/c/9161/3/src/kudu/consensus/raft_consensus.cc@864
PS3, Line 864: req.set_cas_config_opid_index(current_committed_config_index);
> We want to retain it so when the master says to add a new node we ignore th
Ah, than makes sense. I thought there is a separate code path for that. Having just one is better, of course.
--
To view, visit http://gerrit.cloudera.org:8080/9161
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3a033ce16f626458efdb59b1b4dd8450e8591761
Gerrit-Change-Number: 9161
Gerrit-PatchSet: 3
Gerrit-Owner: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Fri, 02 Feb 2018 18:41:57 +0000
Gerrit-HasComments: Yes
[kudu-CR] Don't rely on pending config OpId index for peer promotion
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/9161 )
Change subject: Don't rely on pending config OpId index for peer promotion
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/9161/3/src/kudu/consensus/raft_consensus.cc
File src/kudu/consensus/raft_consensus.cc:
http://gerrit.cloudera.org:8080/#/c/9161/3/src/kudu/consensus/raft_consensus.cc@864
PS3, Line 864: req.set_cas_config_opid_index(current_committed_config_index);
> Maybe, then we can safely stop filling cas_config_opid_index at all? I mea
'when' --> 'why then', sorry for the typos
--
To view, visit http://gerrit.cloudera.org:8080/9161
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3a033ce16f626458efdb59b1b4dd8450e8591761
Gerrit-Change-Number: 9161
Gerrit-PatchSet: 3
Gerrit-Owner: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Fri, 02 Feb 2018 01:09:04 +0000
Gerrit-HasComments: Yes
[kudu-CR] Don't rely on pending config OpId index for peer promotion
Posted by "David Ribeiro Alves (Code Review)" <ge...@cloudera.org>.
David Ribeiro Alves has posted comments on this change. ( http://gerrit.cloudera.org:8080/9161 )
Change subject: Don't rely on pending config OpId index for peer promotion
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/9161/3/src/kudu/consensus/consensus_queue.cc
File src/kudu/consensus/consensus_queue.cc:
http://gerrit.cloudera.org:8080/#/c/9161/3/src/kudu/consensus/consensus_queue.cc@a982
PS3, Line 982:
> I'm not 100% following the commit message and how it corresponds to this an
true, but what I guess I was saying is. do we care?
there are at least a couple of cases where it might be interesting to not pass a committed config index:
1 - If the config is committed by the time the notification is executed, but is not yet here
2 - if the config changed since between we check it here and in consensus, but the peer is still up for promotion.
in the worst case we issue some extra notifications.
that being said I can easily add the check for "pendingness". That would eliminate possibility 1. actually passing the committed index I don't think would buy us anything though.
--
To view, visit http://gerrit.cloudera.org:8080/9161
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3a033ce16f626458efdb59b1b4dd8450e8591761
Gerrit-Change-Number: 9161
Gerrit-PatchSet: 3
Gerrit-Owner: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Wed, 31 Jan 2018 02:42:28 +0000
Gerrit-HasComments: Yes
[kudu-CR] Don't rely on pending config OpId index for peer promotion
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/9161 )
Change subject: Don't rely on pending config OpId index for peer promotion
......................................................................
Patch Set 3: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/9161
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3a033ce16f626458efdb59b1b4dd8450e8591761
Gerrit-Change-Number: 9161
Gerrit-PatchSet: 3
Gerrit-Owner: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Mon, 05 Feb 2018 19:49:29 +0000
Gerrit-HasComments: No
[kudu-CR] Don't rely on pending config OpId index for peer promotion
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/9161 )
Change subject: Don't rely on pending config OpId index for peer promotion
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/9161/3/src/kudu/consensus/consensus_queue.cc
File src/kudu/consensus/consensus_queue.cc:
http://gerrit.cloudera.org:8080/#/c/9161/3/src/kudu/consensus/consensus_queue.cc@a982
PS3, Line 982:
I'm not 100% following the commit message and how it corresponds to this and the above removed comment.
Wouldn't it be sufficient to just pass the _committed_ config's opid index here instead of the active config?
I see we don't have access to the committed config in the context of the queue state, but it seems like it would also be reasonable to say that, if the active_config has no opid_index (i.e. it is a pending config) then we know that the attempt to promote is going to fail anyway. In that case we can just skip the notification.
--
To view, visit http://gerrit.cloudera.org:8080/9161
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3a033ce16f626458efdb59b1b4dd8450e8591761
Gerrit-Change-Number: 9161
Gerrit-PatchSet: 3
Gerrit-Owner: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Wed, 31 Jan 2018 01:19:16 +0000
Gerrit-HasComments: Yes
[kudu-CR] Don't rely on pending config OpId index for peer promotion
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/9161 )
Change subject: Don't rely on pending config OpId index for peer promotion
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/9161/3/src/kudu/consensus/raft_consensus.cc
File src/kudu/consensus/raft_consensus.cc:
http://gerrit.cloudera.org:8080/#/c/9161/3/src/kudu/consensus/raft_consensus.cc@864
PS3, Line 864: req.set_cas_config_opid_index(current_committed_config_index);
> 'when' --> 'why then', sorry for the typos
We want to retain it so when the master says to add a new node we ignore the master when its information is out of date.
--
To view, visit http://gerrit.cloudera.org:8080/9161
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3a033ce16f626458efdb59b1b4dd8450e8591761
Gerrit-Change-Number: 9161
Gerrit-PatchSet: 3
Gerrit-Owner: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Fri, 02 Feb 2018 02:04:21 +0000
Gerrit-HasComments: Yes
[kudu-CR] Don't rely on pending config OpId index for peer promotion
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/9161 )
Change subject: Don't rely on pending config OpId index for peer promotion
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/9161/3/src/kudu/consensus/raft_consensus.cc
File src/kudu/consensus/raft_consensus.cc:
http://gerrit.cloudera.org:8080/#/c/9161/3/src/kudu/consensus/raft_consensus.cc@864
PS3, Line 864: req.set_cas_config_opid_index(current_committed_config_index);
> Wouldn't it break the CAS semantics? As I understand, the CAS semantics as
This seems OK to me because we checked the config index of the committed config under the consensus lock. If that changes between the time we drop the lock and reacquire it in ChangeConfig() then we'll log a warning ("Unable to promote non-voter") and retry again later.
--
To view, visit http://gerrit.cloudera.org:8080/9161
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3a033ce16f626458efdb59b1b4dd8450e8591761
Gerrit-Change-Number: 9161
Gerrit-PatchSet: 3
Gerrit-Owner: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Fri, 02 Feb 2018 00:38:27 +0000
Gerrit-HasComments: Yes
[kudu-CR] Don't rely on pending config OpId index for peer promotion
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/9161 )
Change subject: Don't rely on pending config OpId index for peer promotion
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/9161/3/src/kudu/consensus/raft_consensus.cc
File src/kudu/consensus/raft_consensus.cc:
http://gerrit.cloudera.org:8080/#/c/9161/3/src/kudu/consensus/raft_consensus.cc@864
PS3, Line 864: req.set_cas_config_opid_index(current_committed_config_index);
> This seems OK to me because we checked the config index of the committed co
Maybe, then we can safely stop filling cas_config_opid_index at all? I mean once we assume promoting a non-voter is safe regardless of current term/configuration index, when just not drop it at this stage as well?
--
To view, visit http://gerrit.cloudera.org:8080/9161
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3a033ce16f626458efdb59b1b4dd8450e8591761
Gerrit-Change-Number: 9161
Gerrit-PatchSet: 3
Gerrit-Owner: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Fri, 02 Feb 2018 01:07:00 +0000
Gerrit-HasComments: Yes
[kudu-CR] Don't rely on pending config OpId index for peer promotion
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/9161 )
Change subject: Don't rely on pending config OpId index for peer promotion
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/9161/3/src/kudu/consensus/consensus_queue.cc
File src/kudu/consensus/consensus_queue.cc:
http://gerrit.cloudera.org:8080/#/c/9161/3/src/kudu/consensus/consensus_queue.cc@a982
PS3, Line 982:
> That's fair. It's possible would have lost leadership by the time the notif
I suppose we can get away with not passing it. In general, it's easier to reason about changes to the config in a "current state" vacuum, and if any state has changed then we can simply force a retry. This was written to be consistent with that philosophy. That said, I don't think the proposed change will have any negative effects.
--
To view, visit http://gerrit.cloudera.org:8080/9161
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3a033ce16f626458efdb59b1b4dd8450e8591761
Gerrit-Change-Number: 9161
Gerrit-PatchSet: 3
Gerrit-Owner: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Fri, 02 Feb 2018 00:19:54 +0000
Gerrit-HasComments: Yes
[kudu-CR] Don't rely on pending config OpId index for peer promotion
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/9161 )
Change subject: Don't rely on pending config OpId index for peer promotion
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/9161/3/src/kudu/consensus/consensus_queue.cc
File src/kudu/consensus/consensus_queue.cc:
http://gerrit.cloudera.org:8080/#/c/9161/3/src/kudu/consensus/consensus_queue.cc@a982
PS3, Line 982:
> true, but what I guess I was saying is. do we care?
That's fair. It's possible would have lost leadership by the time the notification executes, though, right? I guess we only woudl end up with a WARN in that case.
Curious for Mike's take on why the original "binding" of the notification to its original term was considered important.
--
To view, visit http://gerrit.cloudera.org:8080/9161
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3a033ce16f626458efdb59b1b4dd8450e8591761
Gerrit-Change-Number: 9161
Gerrit-PatchSet: 3
Gerrit-Owner: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: David Ribeiro Alves <da...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Wed, 31 Jan 2018 18:34:04 +0000
Gerrit-HasComments: Yes