denis-chudov commented on code in PR #4376:
URL: https://github.com/apache/ignite-3/pull/4376#discussion_r1757133992
##########
modules/replicator/src/main/java/org/apache/ignite/internal/replicator/configuration/ReplicationConfigurationSchema.java:
##########
@@ -40,4 +40,14 @@ public class ReplicationConfigurationSchema {
@Value(hasDefault = true)
@Range(min = 1000)
public long rpcTimeout = TimeUnit.SECONDS.toMillis(60);
+
+ /** The interval in milliseconds that is used in the beginning of lease
granting process. */
+ @Value(hasDefault = true)
+ @Range(min = 0)
Review Comment:
It shouldn't be zero, I'd prefer the default of leaseExpirationInterval
##########
modules/replicator/src/main/java/org/apache/ignite/internal/replicator/configuration/ReplicationConfigurationSchema.java:
##########
@@ -40,4 +40,14 @@ public class ReplicationConfigurationSchema {
@Value(hasDefault = true)
@Range(min = 1000)
public long rpcTimeout = TimeUnit.SECONDS.toMillis(60);
+
+ /** The interval in milliseconds that is used in the beginning of lease
granting process. */
+ @Value(hasDefault = true)
+ @Range(min = 0)
+ public long agreementAcceptanceInterval = 120_000;
Review Comment:
```suggestion
public long leaseAgreementAcceptanceTimeLimit = 120_000;
```
##########
modules/replicator/src/main/java/org/apache/ignite/internal/replicator/configuration/ReplicationConfigurationSchema.java:
##########
@@ -40,4 +40,14 @@ public class ReplicationConfigurationSchema {
@Value(hasDefault = true)
@Range(min = 1000)
public long rpcTimeout = TimeUnit.SECONDS.toMillis(60);
+
+ /** The interval in milliseconds that is used in the beginning of lease
granting process. */
+ @Value(hasDefault = true)
+ @Range(min = 0)
+ public long agreementAcceptanceInterval = 120_000;
+
+ /** Lease holding interval. */
+ @Value(hasDefault = true)
+ @Range(min = 0)
Review Comment:
It shouldn't be zero, at lease it should be greater (preferable several
times) than LeaseUpdater#UPDATE_LEASE_MS (500 ms). I would suggest 2000, but
it's discussible. Also we should set the max value, maybe default of
leaseAgreementAcceptanceTimeLimit?
##########
modules/placement-driver/src/main/java/org/apache/ignite/internal/placementdriver/LeaseUpdater.java:
##########
@@ -514,7 +513,9 @@ private void writeNewLease(
) {
HybridTimestamp startTs = clockService.now();
- var expirationTs = new HybridTimestamp(startTs.getPhysical() +
longLeaseInterval, 0);
+ long interval =
replicationConfiguration.agreementAcceptanceInterval().value();
Review Comment:
I don't know how fast the values can be retrieved from configuration, are
you sure that we shouldn't get this only once per iteration in
#updateLeaseBatchInternal?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
