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 2017/11/16 01:16:45 UTC
[kudu-CR] [RaftPeerPB] introduce replica health status
Alexey Serbin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/8561
Change subject: [RaftPeerPB] introduce replica health status
......................................................................
[RaftPeerPB] introduce replica health status
Introduced replica health status. The health status is a non-persistent
optional field in RaftPeerPB which is set by leader replica.
This commit introduces a single change in the RaftPeerPB protobuf
message. It provides necessary interface to start working on the new
scheme of automatic replacement of tablet replicas, so it would be
possible to work in parallel in accordance with the v1 implementation
plan.
Change-Id: Id82f838fd3612ab9bc3f91cac7a840cb9f36ff4c
---
M src/kudu/consensus/metadata.proto
1 file changed, 12 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/61/8561/1
--
To view, visit http://gerrit.cloudera.org:8080/8561
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Id82f838fd3612ab9bc3f91cac7a840cb9f36ff4c
Gerrit-Change-Number: 8561
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
[kudu-CR] [RaftPeerPB] introduce attributes and health status
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/8561 )
Change subject: [RaftPeerPB] introduce attributes and health status
......................................................................
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/8561/3/src/kudu/consensus/metadata.proto
File src/kudu/consensus/metadata.proto:
http://gerrit.cloudera.org:8080/#/c/8561/3/src/kudu/consensus/metadata.proto@42
PS3, Line 42: HealthStatus
> From what I see in this file, enums do not have that fancy PB suffix:
That's bad, right?
--
To view, visit http://gerrit.cloudera.org:8080/8561
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id82f838fd3612ab9bc3f91cac7a840cb9f36ff4c
Gerrit-Change-Number: 8561
Gerrit-PatchSet: 4
Gerrit-Owner: Alexey Serbin <as...@cloudera.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: Fri, 17 Nov 2017 00:06:05 +0000
Gerrit-HasComments: Yes
[kudu-CR] [RaftPeerPB] introduce new status fields
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/8561 )
Change subject: [RaftPeerPB] introduce new status fields
......................................................................
Patch Set 2:
Just put up a gerrit for the TabletReplicaTest failures (which I also hit today, coincidentally): http://gerrit.cloudera.org:8080/8564
--
To view, visit http://gerrit.cloudera.org:8080/8561
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id82f838fd3612ab9bc3f91cac7a840cb9f36ff4c
Gerrit-Change-Number: 8561
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Thu, 16 Nov 2017 04:47:43 +0000
Gerrit-HasComments: No
[kudu-CR] [RaftPeerPB] introduce attributes and health status
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Mike Percy, Todd Lipcon,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/8561
to look at the new patch set (#3).
Change subject: [RaftPeerPB] introduce attributes and health status
......................................................................
[RaftPeerPB] introduce attributes and health status
This standalone commit introduces changes to provide necessary interface
for the new 3-4-3 scheme of automated replica replacement.
Attributes and health report information has been added into the
RaftPeerPB message (which represents a tablet replica).
The 'health_report' is a non-persistent optional field which is populated
by leader replica. It reflects the health of the replica as seen by
the leader.
The 'attrs' field is used as an umbrella for various per-replica
attributes. The attributes are persisted along with other fields
for RaftPeerPB structure. Right away, the 'promote' and 'replace'
attributes are introduced: they are necessary for the new scheme
of automated replica replacement:
* 'promote' is applicable to non-voter replicas only. When set
to 'true', it makes the leader replica promoting the non-voter
replica once it catches up with the leader's WAL.
* 'replace' is applicable to voter replicas only. When set, it means
'replace the replica regardless of its health status'.
This attribute is needed to provide the capability to manually move
a replica from its current location using the kudu CLI tool
(i.e. 'kudu tablet change_config move_replica ...').
Change-Id: Id82f838fd3612ab9bc3f91cac7a840cb9f36ff4c
---
M src/kudu/consensus/metadata.proto
1 file changed, 34 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/61/8561/3
--
To view, visit http://gerrit.cloudera.org:8080/8561
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id82f838fd3612ab9bc3f91cac7a840cb9f36ff4c
Gerrit-Change-Number: 8561
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] [RaftPeerPB] introduce attributes and health status
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Mike Percy, Kudu Jenkins, Todd Lipcon,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/8561
to look at the new patch set (#4).
Change subject: [RaftPeerPB] introduce attributes and health status
......................................................................
[RaftPeerPB] introduce attributes and health status
This standalone commit introduces changes to provide necessary interface
for the new 3-4-3 scheme of automated replica replacement.
Attributes and health report information has been added into the
RaftPeerPB message (which represents a tablet replica).
The 'health_report' is a non-persistent optional field which is populated
by leader replica. It reflects the health of the replica as seen by
the leader.
The 'attrs' field is used as an umbrella for various per-replica
attributes. The attributes are persisted along with other fields
for RaftPeerPB structure. Right away, the 'promote' and 'replace'
attributes are introduced: they are necessary for the new scheme
of automated replica replacement:
* 'promote' is applicable to non-voter replicas only. When set
to 'true', it makes the leader replica promoting the non-voter
replica once it catches up with the leader's WAL.
* 'replace' is applicable to voter replicas only. When set, it means
'replace the replica regardless of its health status'.
This attribute is needed to provide the capability to manually move
a replica from its current location using the kudu CLI tool
(i.e. 'kudu tablet change_config move_replica ...').
Change-Id: Id82f838fd3612ab9bc3f91cac7a840cb9f36ff4c
---
M src/kudu/consensus/metadata.proto
1 file changed, 34 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/61/8561/4
--
To view, visit http://gerrit.cloudera.org:8080/8561
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id82f838fd3612ab9bc3f91cac7a840cb9f36ff4c
Gerrit-Change-Number: 8561
Gerrit-PatchSet: 4
Gerrit-Owner: Alexey Serbin <as...@cloudera.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>
[kudu-CR] [RaftPeerPB] introduce attributes and health status
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/8561 )
Change subject: [RaftPeerPB] introduce attributes and health status
......................................................................
Patch Set 4: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/8561
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id82f838fd3612ab9bc3f91cac7a840cb9f36ff4c
Gerrit-Change-Number: 8561
Gerrit-PatchSet: 4
Gerrit-Owner: Alexey Serbin <as...@cloudera.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: Mon, 20 Nov 2017 06:02:15 +0000
Gerrit-HasComments: No
[kudu-CR] [RaftPeerPB] introduce attributes and health status
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/8561 )
Change subject: [RaftPeerPB] introduce attributes and health status
......................................................................
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/8561/2/src/kudu/consensus/metadata.proto
File src/kudu/consensus/metadata.proto:
http://gerrit.cloudera.org:8080/#/c/8561/2/src/kudu/consensus/metadata.proto@76
PS2, Line 76: // that leaves the configuration.
I'm still unsure on this design point, particularly with whether the master needs to persist the info. Can you outline somewhere in one of the various docs where the master keeps this state and how it handles refreshing upon failover in a scalable way?
--
To view, visit http://gerrit.cloudera.org:8080/8561
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id82f838fd3612ab9bc3f91cac7a840cb9f36ff4c
Gerrit-Change-Number: 8561
Gerrit-PatchSet: 4
Gerrit-Owner: Alexey Serbin <as...@cloudera.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: Fri, 17 Nov 2017 00:48:28 +0000
Gerrit-HasComments: Yes
[kudu-CR] [RaftPeerPB] introduce attributes and health status
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/8561 )
Change subject: [RaftPeerPB] introduce attributes and health status
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/8561/2/src/kudu/consensus/metadata.proto
File src/kudu/consensus/metadata.proto:
http://gerrit.cloudera.org:8080/#/c/8561/2/src/kudu/consensus/metadata.proto@76
PS2, Line 76: // only field, it should not be persisted or read from the persistent storage.
> I'm still unsure on this design point, particularly with whether the master
I added the following paragraph into the optionE document:
...
When the leader master changes, the new leader master requests non-incremental tablet report from all tablet leaders. So, even if some change happened to replicas while there were no leader master around, those changes will be addressed by the newly elected leader master once it receives non-incremental tablet reports.
...
Do you have some particular scenario in mind which requires persisting replica's state at the master's side?
--
To view, visit http://gerrit.cloudera.org:8080/8561
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id82f838fd3612ab9bc3f91cac7a840cb9f36ff4c
Gerrit-Change-Number: 8561
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.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: Fri, 17 Nov 2017 01:53:19 +0000
Gerrit-HasComments: Yes
[kudu-CR] [RaftPeerPB] introduce attributes and health status
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/8561 )
Change subject: [RaftPeerPB] introduce attributes and health status
......................................................................
[RaftPeerPB] introduce attributes and health status
This standalone commit introduces changes to provide necessary interface
for the new 3-4-3 scheme of automated replica replacement.
Attributes and health report information has been added into the
RaftPeerPB message (which represents a tablet replica).
The 'health_report' is a non-persistent optional field which is populated
by leader replica. It reflects the health of the replica as seen by
the leader.
The 'attrs' field is used as an umbrella for various per-replica
attributes. The attributes are persisted along with other fields
for RaftPeerPB structure. Right away, the 'promote' and 'replace'
attributes are introduced: they are necessary for the new scheme
of automated replica replacement:
* 'promote' is applicable to non-voter replicas only. When set
to 'true', it makes the leader replica promoting the non-voter
replica once it catches up with the leader's WAL.
* 'replace' is applicable to voter replicas only. When set, it means
'replace the replica regardless of its health status'.
This attribute is needed to provide the capability to manually move
a replica from its current location using the kudu CLI tool
(i.e. 'kudu tablet change_config move_replica ...').
Change-Id: Id82f838fd3612ab9bc3f91cac7a840cb9f36ff4c
Reviewed-on: http://gerrit.cloudera.org:8080/8561
Reviewed-by: Mike Percy <mp...@apache.org>
Tested-by: Mike Percy <mp...@apache.org>
---
M src/kudu/consensus/metadata.proto
1 file changed, 34 insertions(+), 0 deletions(-)
Approvals:
Mike Percy: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/8561
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Id82f838fd3612ab9bc3f91cac7a840cb9f36ff4c
Gerrit-Change-Number: 8561
Gerrit-PatchSet: 5
Gerrit-Owner: Alexey Serbin <as...@cloudera.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>
[kudu-CR] [RaftPeerPB] introduce attributes and health status
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/8561 )
Change subject: [RaftPeerPB] introduce attributes and health status
......................................................................
Patch Set 4:
Based on Slack convo w/ Todd and Alexey I think this is good to go.
--
To view, visit http://gerrit.cloudera.org:8080/8561
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id82f838fd3612ab9bc3f91cac7a840cb9f36ff4c
Gerrit-Change-Number: 8561
Gerrit-PatchSet: 4
Gerrit-Owner: Alexey Serbin <as...@cloudera.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: Mon, 20 Nov 2017 06:02:31 +0000
Gerrit-HasComments: No
[kudu-CR] [RaftPeerPB] introduce attributes and health status
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/8561 )
Change subject: [RaftPeerPB] introduce attributes and health status
......................................................................
Patch Set 4: Code-Review+2
I'm ok either way, naming is a pretty subjective thing.
--
To view, visit http://gerrit.cloudera.org:8080/8561
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id82f838fd3612ab9bc3f91cac7a840cb9f36ff4c
Gerrit-Change-Number: 8561
Gerrit-PatchSet: 4
Gerrit-Owner: Alexey Serbin <as...@cloudera.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: Fri, 17 Nov 2017 00:23:09 +0000
Gerrit-HasComments: No
[kudu-CR] [RaftPeerPB] introduce attributes and health status
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/8561 )
Change subject: [RaftPeerPB] introduce attributes and health status
......................................................................
Patch Set 2:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/8561/2/src/kudu/consensus/metadata.proto
File src/kudu/consensus/metadata.proto:
http://gerrit.cloudera.org:8080/#/c/8561/2/src/kudu/consensus/metadata.proto@61
PS2, Line 61: enum HealthStatus {
: UNKNOWN_HEALTH_STATUS = 999;
: HEALTHY = 0; // Replica is functioning properly.
: FAILED = 1; // Replica has failed and needs replacement.
: };
> Since we may want to extend the number of health flags in the future what d
Good idea! Done.
http://gerrit.cloudera.org:8080/#/c/8561/2/src/kudu/consensus/metadata.proto@82
PS2, Line 82: do_promote
> nit: I'd personally prefer not to have the "do_" prefix here, but if you fe
Done
--
To view, visit http://gerrit.cloudera.org:8080/8561
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id82f838fd3612ab9bc3f91cac7a840cb9f36ff4c
Gerrit-Change-Number: 8561
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.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: Thu, 16 Nov 2017 22:56:59 +0000
Gerrit-HasComments: Yes
[kudu-CR] [RaftPeerPB] introduce replica health status
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/8561 )
Change subject: [RaftPeerPB] introduce replica health status
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/8561/1/src/kudu/consensus/metadata.proto
File src/kudu/consensus/metadata.proto:
http://gerrit.cloudera.org:8080/#/c/8561/1/src/kudu/consensus/metadata.proto@75
PS1, Line 75: This is run-time
: // only field, it should not be persisted or read from the persistent storage.
is this true even on the master? or would we expect the master to persist it? if the master doesn't persist it somehow, and the master fails over, how does the new master not get super confused about what's going on with the "extra" replicas?
--
To view, visit http://gerrit.cloudera.org:8080/8561
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id82f838fd3612ab9bc3f91cac7a840cb9f36ff4c
Gerrit-Change-Number: 8561
Gerrit-PatchSet: 1
Gerrit-Owner: 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: Thu, 16 Nov 2017 01:30:32 +0000
Gerrit-HasComments: Yes
[kudu-CR] [RaftPeerPB] introduce attributes and health status
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/8561 )
Change subject: [RaftPeerPB] introduce attributes and health status
......................................................................
Patch Set 3:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/8561/3/src/kudu/consensus/metadata.proto
File src/kudu/consensus/metadata.proto:
http://gerrit.cloudera.org:8080/#/c/8561/3/src/kudu/consensus/metadata.proto@42
PS3, Line 42: HealthStatus
> nit: I think this should be named HealthStatusPB to indicate that it's defi
From what I see in this file, enums do not have that fancy PB suffix:
dhcp-10-16-1-175:kudu[3-4-3]$ find . -name '*.proto' | xargs grep -E 'enum [A-Z].*[^PB] '| wc -l
42
Enums with PB suffix are rather exceptions:
dhcp-10-16-1-175:kudu[3-4-3]$ find . -name '*.proto' | xargs grep -E 'enum [A-Z].*PB ' | wc -l
4
http://gerrit.cloudera.org:8080/#/c/8561/3/src/kudu/consensus/metadata.proto@48
PS3, Line 48:
> nit: trailing space
Done
http://gerrit.cloudera.org:8080/#/c/8561/3/src/kudu/consensus/metadata.proto@97
PS3, Line 97:
> insert: a
Done
--
To view, visit http://gerrit.cloudera.org:8080/8561
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id82f838fd3612ab9bc3f91cac7a840cb9f36ff4c
Gerrit-Change-Number: 8561
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin <as...@cloudera.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: Thu, 16 Nov 2017 23:43:22 +0000
Gerrit-HasComments: Yes
[kudu-CR] [RaftPeerPB] introduce attributes and health status
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has removed a vote on this change.
Change subject: [RaftPeerPB] introduce attributes and health status
......................................................................
Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/8561
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: Id82f838fd3612ab9bc3f91cac7a840cb9f36ff4c
Gerrit-Change-Number: 8561
Gerrit-PatchSet: 4
Gerrit-Owner: Alexey Serbin <as...@cloudera.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>
[kudu-CR] [RaftPeerPB] introduce replica health status
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/8561 )
Change subject: [RaftPeerPB] introduce replica health status
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/8561/1/src/kudu/consensus/metadata.proto
File src/kudu/consensus/metadata.proto:
http://gerrit.cloudera.org:8080/#/c/8561/1/src/kudu/consensus/metadata.proto@75
PS1, Line 75: This is run-time
: // only field, it should not be persisted or read from the persistent storage.
> is this true even on the master? or would we expect the master to persist i
Ah, I see -- it seems I missed the PROMOTE and REPLACE attributes. I'll add them for the complete picture, sure.
By my understanding, master does not need to persist the health status. Overall, it's not necessary to know the previous state of the health status of a replica, it's enough to see what current status is. Do you have some use case in mind when master would get super-confused with 'extra' replicas?
If the leader master changes, master is about to receive non-incremental report on all tablets in the system.
--
To view, visit http://gerrit.cloudera.org:8080/8561
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id82f838fd3612ab9bc3f91cac7a840cb9f36ff4c
Gerrit-Change-Number: 8561
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.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: Thu, 16 Nov 2017 01:45:57 +0000
Gerrit-HasComments: Yes
[kudu-CR] [RaftPeerPB] introduce new status fields
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/8561 )
Change subject: [RaftPeerPB] introduce new status fields
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/8561/1/src/kudu/consensus/metadata.proto
File src/kudu/consensus/metadata.proto:
http://gerrit.cloudera.org:8080/#/c/8561/1/src/kudu/consensus/metadata.proto@75
PS1, Line 75: This is run-time
: // only field, it should not be persisted or read from the persistent storage.
> Ah, I see -- it seems I missed the PROMOTE and REPLACE attributes. I'll ad
I agree with Alexey. Since the master is event-driven, the master should wait until it gets a new tablet report before acting. Master failover is supposed to trigger a full tablet report from all replicas.
--
To view, visit http://gerrit.cloudera.org:8080/8561
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id82f838fd3612ab9bc3f91cac7a840cb9f36ff4c
Gerrit-Change-Number: 8561
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Thu, 16 Nov 2017 22:01:01 +0000
Gerrit-HasComments: Yes
[kudu-CR] [RaftPeerPB] introduce new status fields
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/8561 )
Change subject: [RaftPeerPB] introduce new status fields
......................................................................
Patch Set 2: Verified+1
unrelated flakes in:
Parameters/TestRowSetTreePerformance.TestPerformance/19
UpdateScanDeltaCompactionTest.TestAll
TabletReplicaTest.TestDMSAnchorPreventsLogGC
TabletReplicaTest.TestActiveTransactionPreventsLogGC
--
To view, visit http://gerrit.cloudera.org:8080/8561
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id82f838fd3612ab9bc3f91cac7a840cb9f36ff4c
Gerrit-Change-Number: 8561
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Thu, 16 Nov 2017 03:28:25 +0000
Gerrit-HasComments: No
[kudu-CR] [RaftPeerPB] introduce attributes and health status
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/8561 )
Change subject: [RaftPeerPB] introduce attributes and health status
......................................................................
Patch Set 3:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/8561/3/src/kudu/consensus/metadata.proto
File src/kudu/consensus/metadata.proto:
http://gerrit.cloudera.org:8080/#/c/8561/3/src/kudu/consensus/metadata.proto@42
PS3, Line 42: HealthStatus
nit: I think this should be named HealthStatusPB to indicate that it's defined in a protobuf file. Other examples of enums that left this out are a mistake
http://gerrit.cloudera.org:8080/#/c/8561/3/src/kudu/consensus/metadata.proto@48
PS3, Line 48:
nit: trailing space
http://gerrit.cloudera.org:8080/#/c/8561/3/src/kudu/consensus/metadata.proto@97
PS3, Line 97:
insert: a
--
To view, visit http://gerrit.cloudera.org:8080/8561
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id82f838fd3612ab9bc3f91cac7a840cb9f36ff4c
Gerrit-Change-Number: 8561
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin <as...@cloudera.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: Thu, 16 Nov 2017 23:02:21 +0000
Gerrit-HasComments: Yes
[kudu-CR] [RaftPeerPB] introduce new status fields
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has removed Kudu Jenkins from this change. ( http://gerrit.cloudera.org:8080/8561 )
Change subject: [RaftPeerPB] introduce new status fields
......................................................................
Removed reviewer Kudu Jenkins with the following votes:
* Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/8561
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteReviewer
Gerrit-Change-Id: Id82f838fd3612ab9bc3f91cac7a840cb9f36ff4c
Gerrit-Change-Number: 8561
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
[kudu-CR] [RaftPeerPB] introduce new status fields
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Mike Percy, Kudu Jenkins, Todd Lipcon,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/8561
to look at the new patch set (#2).
Change subject: [RaftPeerPB] introduce new status fields
......................................................................
[RaftPeerPB] introduce new status fields
Introduced the following fields for a tablet replica (RaftPeerPB):
* health_status
* do_promote
* do_replace
The 'health_status' is a non-persistent optional field which is set by
leader replica. It reflects the health of the replica as seen by
the leader.
The 'do_promote' is an optional field which is set by master. It's
applicable to non-voter replicas only. If set to 'true', it makes
the leader replica promoting the replica once it catches up with the
leader's WAL.
The 'do_replace' is an optional field which means 'replace the replica
regardless of its health status'. It's a field to provide the
capability to manually move replica from its current location using the
kudu CLI tool (i.e. 'kudu tablet change_config move_replica').
This commit introduces a single change in the RaftPeerPB protobuf
message. It provides necessary interface to start working on the new
scheme of automatic replacement of tablet replicas, so it would be
possible to work in parallel in accordance with the v1 implementation
plan.
Change-Id: Id82f838fd3612ab9bc3f91cac7a840cb9f36ff4c
---
M src/kudu/consensus/metadata.proto
1 file changed, 21 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/61/8561/2
--
To view, visit http://gerrit.cloudera.org:8080/8561
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id82f838fd3612ab9bc3f91cac7a840cb9f36ff4c
Gerrit-Change-Number: 8561
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.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>
[kudu-CR] [RaftPeerPB] introduce new status fields
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/8561 )
Change subject: [RaftPeerPB] introduce new status fields
......................................................................
Patch Set 2:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/8561/2/src/kudu/consensus/metadata.proto
File src/kudu/consensus/metadata.proto:
http://gerrit.cloudera.org:8080/#/c/8561/2/src/kudu/consensus/metadata.proto@61
PS2, Line 61: enum HealthStatus {
: UNKNOWN_HEALTH_STATUS = 999;
: HEALTHY = 0; // Replica is functioning properly.
: FAILED = 1; // Replica has failed and needs replacement.
: };
Since we may want to extend the number of health flags in the future what do you think about pulling this enum out of RaftPeerPB and put it in a separate top-level message such as HealthStatusPB? Something like
message HealthReportPB {
enum HealthStatusPB {
UNKNOWN = 999;
FAILED = 0;
HEALTHY = 1;
}
optional HealthStatusPB overall_health = 0;
}
message RaftPeerPB {
...
optional HealthReportPB health_report;
}
http://gerrit.cloudera.org:8080/#/c/8561/2/src/kudu/consensus/metadata.proto@82
PS2, Line 82: do_promote
nit: I'd personally prefer not to have the "do_" prefix here, but if you feel it's very helpful for understanding I can get behind it.
Also, regarding the promote / replace attributes, perhaps we should also nest them in a message so that extending it later is more encapsulated? They are logically related to each other.
I was thinking something like:
message RaftPeerAttrsPB {
optional bool promote;
optional bool replace;
}
message RaftPeerPB {
...
optional RaftPeerAttrsPB attrs;
}
--
To view, visit http://gerrit.cloudera.org:8080/8561
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id82f838fd3612ab9bc3f91cac7a840cb9f36ff4c
Gerrit-Change-Number: 8561
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Thu, 16 Nov 2017 20:53:51 +0000
Gerrit-HasComments: Yes