Yao Xu has posted comments on this change. ( http://gerrit.cloudera.org:8080/12468 )
Change subject: KUDU-2514 Part 1: Support extra config for table. ...................................................................... Patch Set 8: (2 comments) Thanks adar. I will implement the C++ API and kudu tools in Part 2. Before refactoring the code, I still have some questions I would like to ask? http://gerrit.cloudera.org:8080/#/c/12468/8/java/kudu-client/src/main/java/org/apache/kudu/client/TableExtraConfig.java File java/kudu-client/src/main/java/org/apache/kudu/client/TableExtraConfig.java: http://gerrit.cloudera.org:8080/#/c/12468/8/java/kudu-client/src/main/java/org/apache/kudu/client/TableExtraConfig.java@39 PS8, Line 39: private final static Set<String> supportedKeys = new HashSet<>(Arrays.asList( > I don't think the client should be in the business of deciding > which keys are supported and which aren't. For one, it means an old > client can never be used to read newer extra-config entries from > the server. For two, on CreateTable/AlterTable, the server is going > to validate these anyway, so the server should get to decide what's > OK and what's not. I have a question, if we decide which key support is on the server side, then our RPC will contain a large number of string type kv pairs, which may make the CreateTable/AlterTable RPC to be too large. Is there no problem? If we can accept it, I think we can define a KVConfigPB as a parameter for RPC. http://gerrit.cloudera.org:8080/#/c/12468/8/java/kudu-client/src/main/java/org/apache/kudu/client/TableExtraConfig.java@40 PS8, Line 40: TABLE_HISTORY_MAX_AGE_SEC > This set should be empty in this patch, right? The main reason here is to make the test framework work. Maybe I can find other ways to test it, for example setting the master support configuration by gflag. -- To view, visit http://gerrit.cloudera.org:8080/12468 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0514507dca95602a97e954d1db464b907e073aae Gerrit-Change-Number: 12468 Gerrit-PatchSet: 8 Gerrit-Owner: Yao Xu <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Reviewer: Yao Xu <[email protected]> Gerrit-Comment-Date: Fri, 17 May 2019 11:47:18 +0000 Gerrit-HasComments: Yes
