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 8: (3 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' It should be impossible for that to happen. I will enforce it with a DCHECK. http://gerrit.cloudera.org:8080/#/c/11115/8/src/kudu/master/ts_descriptor-test.cc File src/kudu/master/ts_descriptor-test.cc: PS8: > Do you think it's worth adding an integration test to verify that a tablet Done 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 extrem In order to implement a timeout, we ought to run the location assignment in the background. This would introduce a lot more complexity into the process, especially if we respond early and "provisionally register" the tablet server (implying a need to possibly roll back a registration). But I think the location mapping command shouldn't be doing anything that has a realistic possibility of taking a while, like DNS or network requests. As long as users have tools to see if registration takes a long time, I think that's sufficient, because the solution to "sometimes by location assignment takes 2 minutes" is "you are doing it wrong- change the script". -- 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 <wdberke...@gmail.com> Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Fengling Wang <fw...@cloudera.com> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Will Berkeley <wdberke...@gmail.com> Gerrit-Comment-Date: Tue, 04 Sep 2018 20:48:31 +0000 Gerrit-HasComments: Yes
