Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17356 )
Change subject: KUDU-2612: propagate commit timestamp (Java client) ...................................................................... Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/17356/1/java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransaction.java File java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransaction.java: http://gerrit.cloudera.org:8080/#/c/17356/1/java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransaction.java@557 PS1, Line 557: if (resp.hasCommitTimestamp()) { : client.updateLastPropagatedTimestamp(resp.getCommitTimestamp()); : } > Do we still need this if we're calling it at isCommitComplete()? Yes, this is necessary -- you could remove this and saw that the case of committing a transaction synchronously fails to propagate the commit timestamp. The way how Java client works is a bit complicated in this regard since the call chain is based on callbacks because of the asynchronous nature of the core of the Java client. In other words, isCommitComplete() isn't called as a part of KuduTransaction.commit(true). I didn't find a better way to have this piece of code only in one place. The other alternatives I evaluated are (a) doing that in GetTransactionStateRequest.deserialize() (b) doing that in an extra callback added into the chain for isCommitComplete(). IMO, (a) is the wrong place to do so conceptually, and (b) would make the code much more complex Probably, the source of confusion was in the commit description -- I updated it correspondingly. -- To view, visit http://gerrit.cloudera.org:8080/17356 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4177fe0d137b70bd18dd6c87eb42e8aaf03a00b3 Gerrit-Change-Number: 17356 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 28 Apr 2021 21:59:34 +0000 Gerrit-HasComments: Yes
