[kudu-CR] [RaftPeerPB] introduce attributes and health status

[Mike Percy (Code Review)]

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 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Mon, 20 Nov 2017 06:02:15 +
Gerrit-HasComments: No

[kudu-CR] [RaftPeerPB] introduce attributes and health status

[Mike Percy (Code Review)]

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 
Tested-by: Mike Percy 
---
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 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] [RaftPeerPB] introduce attributes and health status

[Mike Percy (Code Review)]

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 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Mon, 20 Nov 2017 06:02:31 +
Gerrit-HasComments: No

[kudu-CR] [RaftPeerPB] introduce attributes and health status

[Mike Percy (Code Review)]

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 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] [RaftPeerPB] introduce attributes and health status

[Alexey Serbin (Code Review)]

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 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 17 Nov 2017 01:53:19 +
Gerrit-HasComments: Yes

[kudu-CR] [RaftPeerPB] introduce attributes and health status

[Todd Lipcon (Code Review)]

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 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 17 Nov 2017 00:48:28 +
Gerrit-HasComments: Yes

[kudu-CR] [RaftPeerPB] introduce attributes and health status

[Mike Percy (Code Review)]

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 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 17 Nov 2017 00:23:09 +
Gerrit-HasComments: No

[kudu-CR] [RaftPeerPB] introduce attributes and health status

[Mike Percy (Code Review)]

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 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 17 Nov 2017 00:06:05 +
Gerrit-HasComments: Yes

[kudu-CR] [RaftPeerPB] introduce attributes and health status

[Alexey Serbin (Code Review)]

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 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 

[kudu-CR] [RaftPeerPB] introduce attributes and health status

[Alexey Serbin (Code Review)]

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 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 16 Nov 2017 23:43:22 +
Gerrit-HasComments: Yes

[kudu-CR] [RaftPeerPB] introduce attributes and health status

[Mike Percy (Code Review)]

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 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 16 Nov 2017 23:02:21 +
Gerrit-HasComments: Yes

[kudu-CR] [RaftPeerPB] introduce attributes and health status

[Alexey Serbin (Code Review)]

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 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Thu, 16 Nov 2017 22:56:59 +
Gerrit-HasComments: Yes

[kudu-CR] [RaftPeerPB] introduce attributes and health status

[Alexey Serbin (Code Review)]

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 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Mike Percy 
Gerrit-Reviewer: Todd Lipcon