Copilot commented on code in PR #6971:
URL: https://github.com/apache/ignite-3/pull/6971#discussion_r2537985341
##########
modules/runner/src/main/java/org/apache/ignite/internal/app/NodePropertiesImpl.java:
##########
@@ -67,16 +70,26 @@ private void detectAndSaveColocationStatusIfNeeded() {
VaultEntry entry = vaultManager.get(ZONE_BASED_REPLICATION_KEY);
if (entry != null) {
colocationEnabled = entry.value()[0] == 1;
-
+ if (!colocationEnabled) {
+ throw new
IgniteException(UNSUPPORTED_TABLE_BASED_REPLICATION_ERR, "Table based
replication is no longer supported."
+ + " Downgrade back to 3.1 and copy your data to a
cluster of desired version.");
+ }
logComment = "from Vault";
} else {
boolean freshNode = vaultManager.name() == null;
if (freshNode) {
colocationEnabled = IgniteSystemProperties.colocationEnabled();
+ // TODO https://issues.apache.org/jira/browse/IGNITE-22522
Remove.
+ // It's a temporary code that will be removed when !colocation
mode will be fully dropped. That's the reason why instead of
+ // introducing new error code, existing somewhat related is
used.
+ if (!colocationEnabled) {
+ throw new IgniteException(ILLEGAL_ARGUMENT_ERR, "Table
based replication is no longer supported, consider restarting"
+ + " the node in zone based replication mode.");
+ }
logComment = "from system properties on a fresh node";
} else {
- colocationEnabled = false;
- logComment = "node of an older version was run without zone
based replication";
+ throw new
IgniteException(UNSUPPORTED_TABLE_BASED_REPLICATION_ERR, "Table based
replication is no longer supported."
+ + " Downgrade back to 3.1 and copy your data to a
cluster of desired version.");
}
saveToVault(colocationEnabled);
Review Comment:
After the new error-throwing logic is added, the code at lines 91-93 will
now always throw an exception for used nodes without a vault entry. This means
`colocationEnabled` will never be set and `saveToVault(colocationEnabled)` at
line 95 will never be reached. This appears to be intentional (preventing
non-colocation mode), but the unreachable `saveToVault` call should be removed
to avoid confusion.
##########
modules/runner/src/main/java/org/apache/ignite/internal/app/NodePropertiesImpl.java:
##########
@@ -67,16 +70,26 @@ private void detectAndSaveColocationStatusIfNeeded() {
VaultEntry entry = vaultManager.get(ZONE_BASED_REPLICATION_KEY);
if (entry != null) {
colocationEnabled = entry.value()[0] == 1;
-
+ if (!colocationEnabled) {
+ throw new
IgniteException(UNSUPPORTED_TABLE_BASED_REPLICATION_ERR, "Table based
replication is no longer supported."
+ + " Downgrade back to 3.1 and copy your data to a
cluster of desired version.");
+ }
logComment = "from Vault";
} else {
boolean freshNode = vaultManager.name() == null;
if (freshNode) {
colocationEnabled = IgniteSystemProperties.colocationEnabled();
+ // TODO https://issues.apache.org/jira/browse/IGNITE-22522
Remove.
+ // It's a temporary code that will be removed when !colocation
mode will be fully dropped. That's the reason why instead of
+ // introducing new error code, existing somewhat related is
used.
+ if (!colocationEnabled) {
+ throw new IgniteException(ILLEGAL_ARGUMENT_ERR, "Table
based replication is no longer supported, consider restarting"
Review Comment:
[nitpick] The comment indicates that `ILLEGAL_ARGUMENT_ERR` is used
temporarily because the code will be removed when non-colocation mode is fully
dropped. However, this creates inconsistency in error handling:
- Lines 74-76: Uses `UNSUPPORTED_TABLE_BASED_REPLICATION_ERR` for used nodes
with vault entry
- Lines 86-88: Uses `ILLEGAL_ARGUMENT_ERR` for fresh nodes
- Lines 91-93: Uses `UNSUPPORTED_TABLE_BASED_REPLICATION_ERR` for used nodes
without vault entry
For consistency and clarity, all three cases should use
`UNSUPPORTED_TABLE_BASED_REPLICATION_ERR` since they represent the same
fundamental issue: table-based replication is no longer supported. The fresh
node case (lines 85-88) should be updated to use the proper error code.
```suggestion
throw new
IgniteException(UNSUPPORTED_TABLE_BASED_REPLICATION_ERR, "Table based
replication is no longer supported, consider restarting"
```
--
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]
