Andrew Wong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8229 )

Change subject: Add log parser script
......................................................................


Patch Set 1:

(3 comments)

Overall looks pretty good.

http://gerrit.cloudera.org:8080/#/c/8229/1/src/kudu/scripts/kudu-log-parser.pl
File src/kudu/scripts/kudu-log-parser.pl:

http://gerrit.cloudera.org:8080/#/c/8229/1/src/kudu/scripts/kudu-log-parser.pl@5
PS1, Line 5: # Pass it a list of logs from a whole cluster.
For the perl-uninitiated, maybe a sample shell line?


http://gerrit.cloudera.org:8080/#/c/8229/1/src/kudu/scripts/kudu-log-parser.pl@39
PS1, Line 39: (?:leader )?((?:pre-)?
I think this matches "leader pre-election", "leader election", and 
"pre-election". Can all of these happen? Would "((?:leader |pre-)election)" be 
good here?

Also would "forced leader election" be useful here (looking at 
raft_consensus.cc:367)?


http://gerrit.cloudera.org:8080/#/c/8229/1/src/kudu/scripts/kudu-log-parser.pl@236
PS1, Line 236: entry
nit: I'm having some trouble parsing out what an "entry" is vs what a "record" 
is vs what a "rec" is. Are there less similar names we could use here?

I think "entry" is synonymous with "record", so maybe pick one and use it 
throughout?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3bd8bb5d9a3c598ade7bc1bbc8f5a9e24ca618af
Gerrit-Change-Number: 8229
Gerrit-PatchSet: 1
Gerrit-Owner: Mike Percy <[email protected]>
Gerrit-Reviewer: Andrew Wong <[email protected]>
Gerrit-Reviewer: David Ribeiro Alves <[email protected]>
Gerrit-Comment-Date: Tue, 31 Oct 2017 16:21:28 +0000
Gerrit-HasComments: Yes

Reply via email to