sanpwc commented on code in PR #2475:
URL: https://github.com/apache/ignite-3/pull/2475#discussion_r1302845006


##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/replicator/PartitionReplicaListener.java:
##########
@@ -2382,18 +2422,18 @@ private CompletableFuture<Boolean> resolveTxState(
             UUID txId,
             HybridTimestamp timestamp
     ) {
-        requireNonNull(timestamp, "timestamp");
+        boolean readLatest = timestamp == null;
 
         return placementDriver.sendMetaRequest(commitGrpId, 
FACTORY.txStateReplicaRequest()
                         .groupId(commitGrpId)
-                        .readTimestampLong(timestamp.longValue())
+                        .readTimestampLong((readLatest ? 
HybridTimestamp.MIN_VALUE : timestamp).longValue())

Review Comment:
   > I am sure it is correct, because I tested it.
   What exactly did you test? Where are the tests?
   
   I mean that RW transaction has both startTimestamp and corresponding 
operations timestamps. Why we use MIN_VALUE for RW instead of actual RW related 
timestamps?
   
   > because it gives a guarantee that this request don't wait for safe time.
   Could you please share a code, where we wait for a safeTime for the 
writeIntent resolution?



##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/replicator/PartitionReplicaListener.java:
##########
@@ -2382,18 +2422,18 @@ private CompletableFuture<Boolean> resolveTxState(
             UUID txId,
             HybridTimestamp timestamp
     ) {
-        requireNonNull(timestamp, "timestamp");
+        boolean readLatest = timestamp == null;
 
         return placementDriver.sendMetaRequest(commitGrpId, 
FACTORY.txStateReplicaRequest()
                         .groupId(commitGrpId)
-                        .readTimestampLong(timestamp.longValue())
+                        .readTimestampLong((readLatest ? 
HybridTimestamp.MIN_VALUE : timestamp).longValue())

Review Comment:
   > I am sure it is correct, because I tested it.
   What exactly did you test? Where are the tests?
   
   I mean that RW transaction has both startTimestamp and corresponding 
operations timestamps. Why we use MIN_VALUE for RW instead of actual RW related 
timestamps?
   
   > because it gives a guarantee that this request don't wait for safe time.
   
   Could you please share a code, where we wait for a safeTime for the 
writeIntent resolution?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscr...@ignite.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to