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/08/10 23:14:55 UTC

[kudu-CR] KUDU-564 (part 1): log a 'diff' when tablet config changes

Hello Mike Percy, Alexey Serbin,

I'd like you to do a code review.  Please visit

    http://gerrit.cloudera.org:8080/3939

to review the following change.

Change subject: KUDU-564 (part 1): log a 'diff' when tablet config changes
......................................................................

KUDU-564 (part 1): log a 'diff' when tablet config changes

This adds some code to 'diff' two consensus configurations, so that log
messages in the master and during config change are much easier to read. For
example, after the update, raft_consensus-itest has logs like:

I0810 16:05:15.715248 21161 raft_consensus_state.cc:609] T 382e9565bbc64aefb973d1f0b21fcc7c P d4ce96ae644c45e1a867f4e14e6f6746 [term 1 NON_PARTICIPANT]: Committing config change with OpId 1.2: config changed from index -1 to 2, VOTER P d4ce96ae644c45e1a867f4e14e6f6746 (127.37.24.4) evicted. New config: { opid_index: 2 peers { permanent_uuid: "544750e465814b6087b2c44a23d240f4" member_type: VOTER last_known_addr { host: "127.37.24.0" port: 39133 } } peers { permanent_uuid: "535909db4a48478288433b7f6999565e" member_type: VOTER last_known_addr { host: "127.37.24.3" port: 35244 } } peers { permanent_uuid: "9b9c692a90854ba7909a47912fc3d0bb" member_type: VOTER last_known_addr { host: "127.37.24.1" port: 39890 } } peers { permanent_uuid: "13754a25859d4fd1a97fcaa0fcb6f849" member_type: VOTER last_known_addr { host: "127.37.24.2" port: 40505 } } }

I left in the dumping of the full 'new config' since having all the information
can still be relevant, but the human readable summary makes this much easier to
interpret.

Change-Id: Icb785093176eea067de039b14f6600084998861a
---
M src/kudu/consensus/quorum_util-test.cc
M src/kudu/consensus/quorum_util.cc
M src/kudu/consensus/quorum_util.h
M src/kudu/consensus/raft_consensus_state.cc
M src/kudu/master/catalog_manager.cc
5 files changed, 182 insertions(+), 7 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/39/3939/1
-- 
To view, visit http://gerrit.cloudera.org:8080/3939
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Icb785093176eea067de039b14f6600084998861a
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>

[kudu-CR] KUDU-564 (part 1): log a 'diff' when tablet config changes

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.

Alexey Serbin has posted comments on this change.

Change subject: KUDU-564 (part 1): log a 'diff' when tablet config changes
......................................................................


Patch Set 1: Code-Review+1

A tiny nit: is it possible to re-format the commit message to preserve the useful information but still fit into 72 chars?

-- 
To view, visit http://gerrit.cloudera.org:8080/3939
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Icb785093176eea067de039b14f6600084998861a
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: No

[kudu-CR] KUDU-564 (part 1): log a 'diff' when tablet config changes

Posted by "Kudu Jenkins (Code Review)" <ge...@cloudera.org>.

Kudu Jenkins has posted comments on this change.

Change subject: KUDU-564 (part 1): log a 'diff' when tablet config changes
......................................................................


Patch Set 1:

Build Started http://104.196.14.100/job/kudu-gerrit/2807/

-- 
To view, visit http://gerrit.cloudera.org:8080/3939
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Icb785093176eea067de039b14f6600084998861a
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: No

[kudu-CR] KUDU-564 (part 1): log a 'diff' when tablet config changes

Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.

Mike Percy has posted comments on this change.

Change subject: KUDU-564 (part 1): log a 'diff' when tablet config changes
......................................................................


Patch Set 1: Code-Review+2

(1 comment)

Nice. Looks good except for one thing I was wondering about

http://gerrit.cloudera.org:8080/#/c/3939/1/src/kudu/consensus/quorum_util.cc
File src/kudu/consensus/quorum_util.cc:

Line 255:                    peer_infos[old_state.leader_uuid()].first.last_known_addr().host(),
not guaranteed to have a leader so i am not sure what happens when last_known_addr is empty. Is it friendly and we just end up with an empty string from host()? Maybe we want a somewhat nicer message in that case.


-- 
To view, visit http://gerrit.cloudera.org:8080/3939
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Icb785093176eea067de039b14f6600084998861a
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: Yes