ascherbakoff commented on code in PR #5093:
URL: https://github.com/apache/ignite-3/pull/5093#discussion_r1926502174


##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/storage/InternalTableImpl.java:
##########
@@ -198,8 +198,9 @@ public class InternalTableImpl implements InternalTable {
     /** Map update guarded by {@link #updatePartitionMapsMux}. */
     private volatile Int2ObjectMap<PendingComparableValuesTracker<Long, Void>> 
storageIndexTrackerByPartitionId = emptyMap();
 
-    /** Implicit transaction timeout. */
-    private final long implicitTransactionTimeout;
+    private final long roTransactionTimeout;
+
+    private final long rwTransactionTimeout;

Review Comment:
   +1. we should



##########
modules/transactions/src/main/java/org/apache/ignite/internal/tx/configuration/TransactionConfigurationSchema.java:
##########
@@ -41,10 +41,29 @@ public class TransactionConfigurationSchema {
     @Value(hasDefault = true)
     public final long timeout = 10_000;
 
-    /** Timeout for implicit transactions (milliseconds). */
     @Range(min = 1)
     @Value(hasDefault = true)
-    public final long implicitTransactionTimeout = 3_000;
+    public final long minRoTimeout = 1;
+
+    @Range(min = 1)
+    @Value(hasDefault = true)
+    public final long maxRoTimeout = Long.MAX_VALUE;

Review Comment:
   What is the purpose of 
   minRoTimeout maxRoTimeout minRwTimeout maxRwTimeout ?
   It doesn't make sense to me.



##########
modules/transactions/src/main/java/org/apache/ignite/internal/tx/configuration/TransactionConfigurationSchema.java:
##########
@@ -41,10 +41,29 @@ public class TransactionConfigurationSchema {
     @Value(hasDefault = true)
     public final long timeout = 10_000;
 
-    /** Timeout for implicit transactions (milliseconds). */
     @Range(min = 1)
     @Value(hasDefault = true)
-    public final long implicitTransactionTimeout = 3_000;
+    public final long minRoTimeout = 1;
+
+    @Range(min = 1)
+    @Value(hasDefault = true)
+    public final long maxRoTimeout = Long.MAX_VALUE;
+
+    @Range(min = 1)
+    @Value(hasDefault = true)
+    public final long minRwTimeout = 1;
+
+    @Range(min = 1)
+    @Value(hasDefault = true)
+    public final long maxRwTimeout = Long.MAX_VALUE;
+
+    @Range(min = 1)
+    @Value(hasDefault = true)
+    public final long roTimeout = 10_000;

Review Comment:
   How these defaults are calculated?
   They seem too low.
   I suggest 10 minutes for RO txns, and 30 seconds for RW txns



##########
modules/transactions/src/main/java/org/apache/ignite/internal/tx/configuration/TransactionConfigurationSchema.java:
##########
@@ -41,10 +41,29 @@ public class TransactionConfigurationSchema {
     @Value(hasDefault = true)
     public final long timeout = 10_000;
 
-    /** Timeout for implicit transactions (milliseconds). */
     @Range(min = 1)
     @Value(hasDefault = true)
-    public final long implicitTransactionTimeout = 3_000;
+    public final long minRoTimeout = 1;

Review Comment:
   +1. we should



##########
modules/transactions/src/main/java/org/apache/ignite/internal/tx/configuration/TransactionConfigurationSchema.java:
##########
@@ -41,10 +41,29 @@ public class TransactionConfigurationSchema {
     @Value(hasDefault = true)
     public final long timeout = 10_000;
 
-    /** Timeout for implicit transactions (milliseconds). */
     @Range(min = 1)
     @Value(hasDefault = true)
-    public final long implicitTransactionTimeout = 3_000;
+    public final long minRoTimeout = 1;
+
+    @Range(min = 1)
+    @Value(hasDefault = true)
+    public final long maxRoTimeout = Long.MAX_VALUE;
+
+    @Range(min = 1)
+    @Value(hasDefault = true)
+    public final long minRwTimeout = 1;
+
+    @Range(min = 1)
+    @Value(hasDefault = true)
+    public final long maxRwTimeout = Long.MAX_VALUE;
+
+    @Range(min = 1)
+    @Value(hasDefault = true)
+    public final long roTimeout = 10_000;
+
+    @Range(min = 1)
+    @Value(hasDefault = true)
+    public final long rwTimeout = 3_000;

Review Comment:
   I believe we still can do breaking changes in configuration APIs.



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