Abhishek Chennaka has posted comments on this change. ( http://gerrit.cloudera.org:8080/18515 )
Change subject: [master] KUDU-2671: Range specific hashing during table alter op. ...................................................................... Patch Set 6: (5 comments) http://gerrit.cloudera.org:8080/#/c/18515/6//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/18515/6//COMMIT_MSG@9 PS6, Line 9: This commit has the changes needed on the master side to : support this functionality > I mean does PS6 contains everything needed in that regard or you are going >From my understanding so far these are the needed changes in catalog_manager.cc Once we have the client in place, we can then perform end-to-end tests under various scenarios to re-confirm that. http://gerrit.cloudera.org:8080/#/c/18515/6/src/kudu/master/catalog_manager.cc File src/kudu/master/catalog_manager.cc: http://gerrit.cloudera.org:8080/#/c/18515/6/src/kudu/master/catalog_manager.cc@2625 PS6, Line 2625: custom_hash_schema_size > Whoops, sorry. I take it back -- custom_hash_schema is repeated field, not Yes, updating the master.proto to store the information about presence of custom hash schema makes sense in a follow-up changelist. http://gerrit.cloudera.org:8080/#/c/18515/6/src/kudu/master/master-test.cc File src/kudu/master/master-test.cc: http://gerrit.cloudera.org:8080/#/c/18515/6/src/kudu/master/master-test.cc@313 PS6, Line 313: RETURN_NOT_OK(proxy_->CreateTable(req, resp, &controller)); > This fails due to 'resp' being nullptr in MasterTest.TestResetBlockCacheMet Using the pass by reference as was done previously in the next version of the patch to fix this. http://gerrit.cloudera.org:8080/#/c/18515/6/src/kudu/master/master-test.cc@313 PS6, Line 313: RETURN_NOT_OK(proxy_->CreateTable(req, resp, &controller)); > One more thing to take care of. I'm not sure that's something related (did I tried to investigate but realized I could use some guidance. Submitting a next patch for now (and hoping this is unrelated) http://gerrit.cloudera.org:8080/#/c/18515/6/src/kudu/master/master-test.cc@1017 PS6, Line 1017: // Check the number of tablets in the table : EasyCurl c; : faststring buf; : ASSERT_OK(c.FetchURL(Substitute("http://$0/tables";, mini_master_->bound_http_addr().ToString()), : &buf)); : string raw = buf.ToString(); : ASSERT_STR_CONTAINS(raw, "\"tablet_count\":\"2\""); > Ah, my bad -- GetTabletLocations isn't available since tablet servers are n Thanks Alexey for the pointers. -- To view, visit http://gerrit.cloudera.org:8080/18515 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iea9e3317d172c9ae76662c44b21fca9a4819930a Gerrit-Change-Number: 18515 Gerrit-PatchSet: 6 Gerrit-Owner: Abhishek Chennaka <[email protected]> Gerrit-Reviewer: Abhishek Chennaka <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy <[email protected]> Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Thu, 09 Jun 2022 04:36:50 +0000 Gerrit-HasComments: Yes
