Andrew Wong 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


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'?


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-dev 
Java-user Joe, maybe "ping supports ignore operations" or something?


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 in 
order to use sendRpcToTablet?


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 error 
out when decoding and fail immediately.



--
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: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 06 Nov 2020 07:43:28 +0000
Gerrit-HasComments: Yes

Reply via email to