Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/17674 )
Change subject: KUDU-3304: [Alter] Support to alter table's replication factor ...................................................................... Patch Set 7: (1 comment) http://gerrit.cloudera.org:8080/#/c/17674/4/src/kudu/client/client.h File src/kudu/client/client.h: http://gerrit.cloudera.org:8080/#/c/17674/4/src/kudu/client/client.h@1721 PS4, Line 1721: : /// Set the table's replication factor. : /// : /// @note The replication factor should be a odd number and range in : /// [1, --max_num_replicas] : /// : /// @param [in] replication_factor : /// The table's new replication factor. > Right -- in some aspects it's close to CreateTable() and DDL operations per Alexey and I discussed this offline at length. Introducing this method as a part of public API has deeper repercussions that are worth pondering. We strive to have our programatic public interface be as stable as possible, so ironing out these details up front would help alleviate much of the concern (or see below for an alternate path). - As pointed out, the semantics of altering the replication factor are a bit fuzzy, and if thinking about replication factor as a first class citizen of metadata that can be altered, there are things we should probably iron out before pushing this into the public client API. Namely, how long should the wait() method actually wait (majority replicated? fully replicated?)? And how should this metadata be synchronized with the HMS, if using Impala and/or Hive? These are certainly questions we can answer now (just spitballing, but we could update IsAlterTableDone() to check replica count), but my point is that these are questions that should be thought through before committing to public API. - Given the expectations of altering the replication factor may be different from other kinds of alter DDL, should this be a part of the KuduTableAlterer API at all, or should we introduce some other API that encompasses only changing replication factor? That way, we could build something more tailored replication factor (e.g. wait for full replication, majority replication, etc.). And beyond that, there may be other use cases to consider, like writing a single-replica table and then increasing the number of replicas once fully compacted as a means to potentially faster replication. Public API aside, if the main goal is a means to bump the replication factor via tooling, an alternate route would be to not expose this as C++ client API just yet, and instead rely on friendship and internal methods to alter the replication factor. We do something similar to set table types for the transactions system table, since table types are also not exposed to public API yet. See [1] for an example. Going this route would still allow us to introduce tooling still, but not lock us into a client API. Then, as we fix any rough edges with tooling, we can learn and deliberate on what the semantics for the public API ought to be, without releasing anything too unstable. [1] https://github.com/apache/kudu/blob/add7cb99fc8d6ab586e27d0ecd7d1698f0f45ae3/src/kudu/transactions/txn_system_client.cc#L130 -- To view, visit http://gerrit.cloudera.org:8080/17674 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3aa2d5b12c508ba761fa9410ad1a465cf31fb7d7 Gerrit-Change-Number: 17674 Gerrit-PatchSet: 7 Gerrit-Owner: Yingchun Lai <[email protected]> Gerrit-Reviewer: Abhishek Chennaka <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yingchun Lai <[email protected]> Gerrit-Comment-Date: Fri, 30 Jul 2021 23:58:37 +0000 Gerrit-HasComments: Yes
