Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/11115 )
Change subject: [location_awareness] Assign locations to registering tablet servers ...................................................................... Patch Set 8: (2 comments) http://gerrit.cloudera.org:8080/#/c/11115/7/src/kudu/master/master_path_handlers.cc File src/kudu/master/master_path_handlers.cc: http://gerrit.cloudera.org:8080/#/c/11115/7/src/kudu/master/master_path_handlers.cc@101 PS7, Line 101: left->permanent_uuid() < right->permanent_uuid() Is this supposed to work even for nullptrs? Or it should SIGSEGV and that's the expected behavior? http://gerrit.cloudera.org:8080/#/c/11115/7/src/kudu/master/ts_descriptor.cc File src/kudu/master/ts_descriptor.cc: http://gerrit.cloudera.org:8080/#/c/11115/7/src/kudu/master/ts_descriptor.cc@231 PS7, Line 231: string location; : TRACE("Assigning location"); : Status s = GetLocationFromLocationMappingCmd(location_ma Should we set some limit on how long this is going to take? For the extreme case, imagine this takes 3 minutes for the script to return. Is it OK to wait for a long time here in the context of MasterServiceImpl::TSHeartbeat()? Another thought: how is it important to make sure the location is assigned before sending back the OK response? From the code above I see the entry will be updated anyways, but only location will not be set if the location assignment script returns an error. Maybe, we should run the location assignment in the background? -- 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: 8 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: Tue, 04 Sep 2018 18:51:39 +0000 Gerrit-HasComments: Yes
