Will Berkeley has posted comments on this change. ( http://gerrit.cloudera.org:8080/11115 )
Change subject: [location_awareness] Assign locations to registering tablet servers ...................................................................... Patch Set 4: (5 comments) http://gerrit.cloudera.org:8080/#/c/11115/4/src/kudu/master/master_path_handlers.cc File src/kudu/master/master_path_handlers.cc: http://gerrit.cloudera.org:8080/#/c/11115/4/src/kudu/master/master_path_handlers.cc@117 PS4, Line 117: shared_ptr<TSDescriptor> > nit: go a bit further and use auto here? Feel free to ignore, though. Done http://gerrit.cloudera.org:8080/#/c/11115/4/src/kudu/master/ts_descriptor-test.cc File src/kudu/master/ts_descriptor-test.cc: http://gerrit.cloudera.org:8080/#/c/11115/4/src/kudu/master/ts_descriptor-test.cc@86 PS4, Line 86: const string location = "/foo-bar0/BAAZ.9-quux"; > nit: maybe, add underscore as well Done http://gerrit.cloudera.org:8080/#/c/11115/4/src/kudu/master/ts_descriptor-test.cc@86 PS4, Line 86: const string location = "/foo-bar0/BAAZ.9-quux"; > nit: maybe, add underscore as well Done http://gerrit.cloudera.org:8080/#/c/11115/4/src/kudu/master/ts_descriptor-test.cc@103 PS4, Line 103: "/foo$", // Contains the illegal character '$'. > nit: maybe, add "\" \"" just in case That's the first test case in this vector. http://gerrit.cloudera.org:8080/#/c/11115/4/src/kudu/master/ts_descriptor.cc File src/kudu/master/ts_descriptor.cc: http://gerrit.cloudera.org:8080/#/c/11115/4/src/kudu/master/ts_descriptor.cc@241 PS4, Line 241: LOG(ERROR) << Substitute( Think we should do something more than logging the failed registration attempt? Right now if location assignment isn't working the master logs about it over and over as the tablet server retries registration indefinitely. At least, the verbosity should be reduced since registration will be attempted once per second. -- To view, visit http://gerrit.cloudera.org:8080/11115 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5eb98823ab7b3b8141b8630196c29c1ebf8e6878 Gerrit-Change-Number: 11115 Gerrit-PatchSet: 4 Gerrit-Owner: Will Berkeley <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Fengling Wang <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Will Berkeley <[email protected]> Gerrit-Comment-Date: Thu, 09 Aug 2018 17:36:59 +0000 Gerrit-HasComments: Yes
