Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/11716 )
Change subject: catalog_manager: adjust maximum tablet count at table creation time ...................................................................... Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/11716/1/src/kudu/master/catalog_manager.cc File src/kudu/master/catalog_manager.cc: http://gerrit.cloudera.org:8080/#/c/11716/1/src/kudu/master/catalog_manager.cc@1491 PS1, Line 1491: max_tablets nit: maybe, rename 'max_tablets' --> 'max_replicas_total' for readability? http://gerrit.cloudera.org:8080/#/c/11716/1/src/kudu/master/catalog_manager.cc@1499 PS1, Line 1499: max_tablets)), : resp, MasterErrorPB::TOO_MANY_TABLETS); nit: why not to join these two lines? http://gerrit.cloudera.org:8080/#/c/11716/1/src/kudu/master/catalog_manager.cc@1503 PS1, Line 1503: // Verify that the number of replicas isn't larger than the number of live tablet : // servers. : if (FLAGS_catalog_manager_check_ts_count_for_create_table && num_replicas > num_live_tservers) { : // Note: this error message is matched against in master-stress-test. : return SetupError(Status::InvalidArgument(Substitute( : "not enough live tablet servers to create a table with the requested replication " : "factor $0; $1 tablet servers are alive", req.num_replicas(), num_live_tservers)), : resp, MasterErrorPB::REPLICATION_FACTOR_TOO_HIGH); : } While you are at it, one usability nit. Would it make sense to move this check on the number of tablet servers alive _before_ the check for the requested number of tablets (post-replication)? I think it would be better from the usability standpoint. Imagine the following situation: an attempt to create tablet with RF=3, --max_create_tablets_per_ts=60, num_partitions=50, and just 2 out of 3 tablet servers are alive. In that case the --max_create_tablets_per_ts=60 check will trigger first, and that would be surprising for users who have just created a bunch of such tables one hour ago when all 3 tablet servers were up and running, wouldn't it? The message of the earlier check would not even mention the fact of not all tablet servers are running, so it might be really confusing. -- To view, visit http://gerrit.cloudera.org:8080/11716 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I00d91baddb1591476a7be27cba043e6354558208 Gerrit-Change-Number: 11716 Gerrit-PatchSet: 1 Gerrit-Owner: Adar Dembo <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: Jean-Daniel Cryans <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Will Berkeley <[email protected]> Gerrit-Comment-Date: Thu, 18 Oct 2018 00:50:57 +0000 Gerrit-HasComments: Yes
