Adar Dembo has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8090 )

Change subject: KUDU-1125: issue one catalog write per tablet report
......................................................................


Patch Set 8:

(8 comments)

http://gerrit.cloudera.org:8080/#/c/8090/6/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:

http://gerrit.cloudera.org:8080/#/c/8090/6/src/kudu/master/catalog_manager.cc@3113
PS6, Line 3113:
> TODO(todd) per git blame
Done


http://gerrit.cloudera.org:8080/#/c/8090/7/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:

http://gerrit.cloudera.org:8080/#/c/8090/7/src/kudu/master/catalog_manager.cc@3152
PS7, Line 3152: acquire
> acquire
Done


http://gerrit.cloudera.org:8080/#/c/8090/7/src/kudu/master/catalog_manager.cc@3173
PS7, Line 3173:         // reasonable to ignore it and wait for an operator fix 
the situation.
> I had a similar thought but seems out of scope for this commit. Perhaps add
So, I added this behavior in commit 42b65b0. The idea was to prevent the master 
from automatically deleting truly unknown tablets, but for the gflag to serve 
as a "safety valve" of sorts in case the admin knew that these tablets were no 
longer useful and wanted the master to delete them.

That said, our tooling has improved since then, and it's easier now to manually 
remove unwanted tablets. Plus I doubt anyone has actually used this gflag. So 
I'll remove it.


http://gerrit.cloudera.org:8080/#/c/8090/7/src/kudu/master/catalog_manager.cc@3236
PS7, Line 3236: index) {
> I think this tertiary if condition needs to be: (report.has_consensus_state
Done


http://gerrit.cloudera.org:8080/#/c/8090/7/src/kudu/master/catalog_manager.cc@3291
PS7, Line 3291:
> Interesting question. I think it technically should, because the purpose of
I'll make the change so this doesn't confuse anyone else in the future.


http://gerrit.cloudera.org:8080/#/c/8090/7/src/kudu/master/catalog_manager.cc@3319
PS7, Line 3319:  than
> cstate; a config doesn't really have a term
Done


http://gerrit.cloudera.org:8080/#/c/8090/7/src/kudu/master/catalog_manager.cc@3320
PS7, Line 3320: vious
> cstate
Done


http://gerrit.cloudera.org:8080/#/c/8090/7/src/kudu/master/catalog_manager.cc@3452
PS7, Line 3452:
> in
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie6f5cf0e4b1cd1160b3b310d89c6dbf3dd62e43b
Gerrit-Change-Number: 8090
Gerrit-PatchSet: 8
Gerrit-Owner: Adar Dembo <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Dan Burkert <[email protected]>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <[email protected]>
Gerrit-Reviewer: Todd Lipcon <[email protected]>
Gerrit-Comment-Date: Fri, 06 Oct 2017 01:42:08 +0000
Gerrit-HasComments: Yes

Reply via email to