denis-chudov commented on code in PR #5685:
URL: https://github.com/apache/ignite-3/pull/5685#discussion_r2079452292
##########
modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/AlterZoneCommand.java:
##########
@@ -136,23 +148,51 @@ private CatalogZoneDescriptor
fromParamsAndPreviousValue(CatalogZoneDescriptor p
CatalogStorageProfilesDescriptor storageProfiles =
storageProfileParams != null
? fromParams(storageProfileParams) :
previous.storageProfiles();
+ int replicas = requireNonNullElse(this.replicas, previous.replicas());
+ int quorumSize = adjustQuorumSize(replicas, previous.quorumSize());
+
return new CatalogZoneDescriptor(
previous.id(),
previous.name(),
- Objects.requireNonNullElse(partitions, previous.partitions()),
- Objects.requireNonNullElse(replicas, previous.replicas()),
- Objects.requireNonNullElse(autoAdjust,
previous.dataNodesAutoAdjust()),
- Objects.requireNonNullElse(scaleUp,
previous.dataNodesAutoAdjustScaleUp()),
- Objects.requireNonNullElse(scaleDown,
previous.dataNodesAutoAdjustScaleDown()),
- Objects.requireNonNullElse(filter, previous.filter()),
+ requireNonNullElse(partitions, previous.partitions()),
+ replicas,
+ quorumSize,
+ requireNonNullElse(autoAdjust, previous.dataNodesAutoAdjust()),
+ requireNonNullElse(scaleUp,
previous.dataNodesAutoAdjustScaleUp()),
+ requireNonNullElse(scaleDown,
previous.dataNodesAutoAdjustScaleDown()),
+ requireNonNullElse(filter, previous.filter()),
storageProfiles,
previous.consistencyMode()
);
}
+ /**
+ * Adjusts quorum size so it is always consistent with the replicas count.
+ *
+ * @param replicas Desired replicas count.
+ * @param previousQuorumSize Previous quorum size.
+ * @return Adjusted quorum size.
+ */
+ private int adjustQuorumSize(int replicas, int previousQuorumSize) {
+ int quorumSize = requireNonNullElse(this.quorumSize,
previousQuorumSize);
+
+ int minQuorum = min(replicas, 2);
+ int maxQuorum = max(minQuorum, (int) (round(replicas / 2.0)));
+
+ if (quorumSize > maxQuorum) {
+ LOG.info("Quorum size adjusted from {} to {}.", quorumSize,
maxQuorum);
Review Comment:
```suggestion
LOG.info("Quorum size adjusted from {} to {} because is exceeds
the maximum quorum value.", quorumSize, maxQuorum);
```
##########
modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/AlterZoneCommand.java:
##########
@@ -136,23 +148,51 @@ private CatalogZoneDescriptor
fromParamsAndPreviousValue(CatalogZoneDescriptor p
CatalogStorageProfilesDescriptor storageProfiles =
storageProfileParams != null
? fromParams(storageProfileParams) :
previous.storageProfiles();
+ int replicas = requireNonNullElse(this.replicas, previous.replicas());
+ int quorumSize = adjustQuorumSize(replicas, previous.quorumSize());
+
return new CatalogZoneDescriptor(
previous.id(),
previous.name(),
- Objects.requireNonNullElse(partitions, previous.partitions()),
- Objects.requireNonNullElse(replicas, previous.replicas()),
- Objects.requireNonNullElse(autoAdjust,
previous.dataNodesAutoAdjust()),
- Objects.requireNonNullElse(scaleUp,
previous.dataNodesAutoAdjustScaleUp()),
- Objects.requireNonNullElse(scaleDown,
previous.dataNodesAutoAdjustScaleDown()),
- Objects.requireNonNullElse(filter, previous.filter()),
+ requireNonNullElse(partitions, previous.partitions()),
+ replicas,
+ quorumSize,
+ requireNonNullElse(autoAdjust, previous.dataNodesAutoAdjust()),
+ requireNonNullElse(scaleUp,
previous.dataNodesAutoAdjustScaleUp()),
+ requireNonNullElse(scaleDown,
previous.dataNodesAutoAdjustScaleDown()),
+ requireNonNullElse(filter, previous.filter()),
storageProfiles,
previous.consistencyMode()
);
}
+ /**
+ * Adjusts quorum size so it is always consistent with the replicas count.
+ *
+ * @param replicas Desired replicas count.
+ * @param previousQuorumSize Previous quorum size.
+ * @return Adjusted quorum size.
+ */
+ private int adjustQuorumSize(int replicas, int previousQuorumSize) {
+ int quorumSize = requireNonNullElse(this.quorumSize,
previousQuorumSize);
Review Comment:
Am I right that `this.quorumSize` is not null only if it is explicitly set
by user?
I think it would be good idea to leave the adjustment for the case of
`this.quorumSize == null` and throw exception on validation otherwise
##########
modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/AlterZoneCommand.java:
##########
@@ -136,23 +148,51 @@ private CatalogZoneDescriptor
fromParamsAndPreviousValue(CatalogZoneDescriptor p
CatalogStorageProfilesDescriptor storageProfiles =
storageProfileParams != null
? fromParams(storageProfileParams) :
previous.storageProfiles();
+ int replicas = requireNonNullElse(this.replicas, previous.replicas());
+ int quorumSize = adjustQuorumSize(replicas, previous.quorumSize());
+
return new CatalogZoneDescriptor(
previous.id(),
previous.name(),
- Objects.requireNonNullElse(partitions, previous.partitions()),
- Objects.requireNonNullElse(replicas, previous.replicas()),
- Objects.requireNonNullElse(autoAdjust,
previous.dataNodesAutoAdjust()),
- Objects.requireNonNullElse(scaleUp,
previous.dataNodesAutoAdjustScaleUp()),
- Objects.requireNonNullElse(scaleDown,
previous.dataNodesAutoAdjustScaleDown()),
- Objects.requireNonNullElse(filter, previous.filter()),
+ requireNonNullElse(partitions, previous.partitions()),
+ replicas,
+ quorumSize,
+ requireNonNullElse(autoAdjust, previous.dataNodesAutoAdjust()),
+ requireNonNullElse(scaleUp,
previous.dataNodesAutoAdjustScaleUp()),
+ requireNonNullElse(scaleDown,
previous.dataNodesAutoAdjustScaleDown()),
+ requireNonNullElse(filter, previous.filter()),
storageProfiles,
previous.consistencyMode()
);
}
+ /**
+ * Adjusts quorum size so it is always consistent with the replicas count.
+ *
+ * @param replicas Desired replicas count.
+ * @param previousQuorumSize Previous quorum size.
+ * @return Adjusted quorum size.
+ */
+ private int adjustQuorumSize(int replicas, int previousQuorumSize) {
+ int quorumSize = requireNonNullElse(this.quorumSize,
previousQuorumSize);
+
+ int minQuorum = min(replicas, 2);
+ int maxQuorum = max(minQuorum, (int) (round(replicas / 2.0)));
+
+ if (quorumSize > maxQuorum) {
+ LOG.info("Quorum size adjusted from {} to {}.", quorumSize,
maxQuorum);
+ return maxQuorum;
+ } else if (quorumSize < minQuorum) {
+ LOG.info("Quorum size adjusted from {} to {}.", quorumSize,
minQuorum);
Review Comment:
```suggestion
LOG.info("Quorum size adjusted from {} to {} because it is less
than the minimum quorum value.", quorumSize, minQuorum);
```
--
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
