Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/16698 )
Change subject: KUDU-1563. Add a feature flag for IGNORE operations ...................................................................... Patch Set 2: (5 comments) http://gerrit.cloudera.org:8080/#/c/16698/2/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java File java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java: http://gerrit.cloudera.org:8080/#/c/16698/2/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java@2666 PS2, Line 2666: else { > nit: don't need to 'else' after a return Done http://gerrit.cloudera.org:8080/#/c/16698/2/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java@2667 PS2, Line 2667: new IllegalStateException((Exception) resp); > nit: why not just throw 'resp'? I need to throw an unchecked exception. http://gerrit.cloudera.org:8080/#/c/16698/2/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java@2675 PS2, Line 2675: feature > nit: it'll be obvious to us because there's only one so far, but for non-de Done http://gerrit.cloudera.org:8080/#/c/16698/2/java/kudu-client/src/main/java/org/apache/kudu/client/PingRequest.java File java/kudu-client/src/main/java/org/apache/kudu/client/PingRequest.java: http://gerrit.cloudera.org:8080/#/c/16698/2/java/kudu-client/src/main/java/org/apache/kudu/client/PingRequest.java@46 PS2, Line 46: static PingRequest makeMasterPingRequest(KuduTable masterTable, Timer timer, long timeoutMillis) { : return new PingRequest(masterTable, MASTER_SERVICE_NAME, timer, timeoutMillis); > Just checking based on the other files -- were these constructors necessary Yes. Without passing the masterTable, sendRpcToTablet doesn't know how to route the request to the correct master server. http://gerrit.cloudera.org:8080/#/c/16698/2/src/kudu/master/master_service.cc File src/kudu/master/master_service.cc: http://gerrit.cloudera.org:8080/#/c/16698/2/src/kudu/master/master_service.cc@107 PS2, Line 107: TAG_FLAG(master_support_ignore_operations, unsafe); > nit: probably doesn't need to be unsafe -- pretty sure the writes will erro Done -- To view, visit http://gerrit.cloudera.org:8080/16698 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I329bd8bde73d247240ae597b677e2cc20a92343a Gerrit-Change-Number: 16698 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Attila Bukor <[email protected]> Gerrit-Reviewer: Grant Henke <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Fri, 06 Nov 2020 13:47:40 +0000 Gerrit-HasComments: Yes
