Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/10581 )
Change subject: KUDU-2191: Add 'kudu_only' flag to AlterTable ...................................................................... Patch Set 3: (4 comments) http://gerrit.cloudera.org:8080/#/c/10581/3/src/kudu/client/client.h File src/kudu/client/client.h: http://gerrit.cloudera.org:8080/#/c/10581/3/src/kudu/client/client.h@1193 PS3, Line 1193: Status AlterTable(bool kudu_only); How about modeling this as a private KuduTableAlterer mutator? Like this: // Whether to alter the table in Kudu as well as in any connected external metadata system, or just in Kudu itself. KuduTableAlterer* kudu_only(bool kudu_only); Then Alter() remains the only way to perform the alter table. http://gerrit.cloudera.org:8080/#/c/10581/3/src/kudu/integration-tests/master_hms-itest.cc File src/kudu/integration-tests/master_hms-itest.cc: http://gerrit.cloudera.org:8080/#/c/10581/3/src/kudu/integration-tests/master_hms-itest.cc@453 PS3, Line 453: TEST_F(MasterHmsTest, TestKuduOnly) { Dan mentioned in the past that tests which start up an HMS are super slow. Maybe you could incorporate this into TestAlterTable? http://gerrit.cloudera.org:8080/#/c/10581/3/src/kudu/master/catalog_manager.cc File src/kudu/master/catalog_manager.cc: http://gerrit.cloudera.org:8080/#/c/10581/3/src/kudu/master/catalog_manager.cc@2158 PS3, Line 2158: (!req.has_kudu_only() || !req.kudu_only())) { The default value of a bool in PB is false, so you don't need to check has_kudu_only. http://gerrit.cloudera.org:8080/#/c/10581/3/src/kudu/master/master.proto File src/kudu/master/master.proto: http://gerrit.cloudera.org:8080/#/c/10581/3/src/kudu/master/master.proto@568 PS3, Line 568: // Whether to alter the corresponding table in the Hive Metastore or not. How about defining this as an enum that describes which external systems should be affected by the alter? Then you could have entries like ALL, NO_HMS, etc. It'll be more descriptive that way. -- To view, visit http://gerrit.cloudera.org:8080/10581 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I703f12b95bb7ca2d65455f0f0602520332b3c678 Gerrit-Change-Number: 10581 Gerrit-PatchSet: 3 Gerrit-Owner: Hao Hao <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: Hao Hao <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Comment-Date: Thu, 07 Jun 2018 19:45:57 +0000 Gerrit-HasComments: Yes
