rpuch commented on code in PR #5093:
URL: https://github.com/apache/ignite-3/pull/5093#discussion_r1926465879
##########
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:
Should we allow these timeouts to be changed at runtime?
##########
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. Javadocs are missing
2. Abbreviations like RO and RW look ugly in user-facing API. Should we name
them `readOnlyXXX` and `readWriteXXX`?
##########
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:
There is already `timeout` which defines default timeouts. Now, `rwTimeout`
and `roTimeout` are added. Do they serve the same purpose? If yes, `timeout`
should probably be removed.
But if it's removed, it's a breaking change (as 3.0 code freeze has already
happened) and it should be handled in a different way, probably by declaring it
as deprecated and, maybe, using the existing value for some time?
--
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
