[GitHub] [ignite-3] ibessonov commented on a diff in pull request #1023: IGNITE-17555 Fix AssertionError and NPE in configuration

[GitBox <gi...@apache.org>]

ibessonov commented on code in PR #1023:
URL: https://github.com/apache/ignite-3/pull/1023#discussion_r949878512


##########
modules/runner/src/main/java/org/apache/ignite/internal/configuration/storage/DistributedConfigurationStorage.java:
##########
@@ -247,23 +266,6 @@ public CompletableFuture<Boolean> write(Map<String, ? extends Serializable> newV
 
         operations.add(Operations.put(MASTER_KEY, ByteUtils.longToBytes(curChangeId)));
 
-        // Condition for a valid MetaStorage data update. Several possibilities here:
-        //  - First update ever, MASTER_KEY property must be absent from MetaStorage.
-        //  - Current node has already performed some updates or received them from MetaStorage watch listener. In this
-        //    case "curChangeId" must match the MASTER_KEY revision exactly.
-        //  - Current node has been restarted and received updates from MetaStorage watch listeners after that. Same as
-        //    above, "curChangeId" must match the MASTER_KEY revision exactly.
-        //  - Current node has been restarted and have not received any updates from MetaStorage watch listeners yet.
-        //    In this case "curChangeId" matches APPLIED_REV, which may or may not match the MASTER_KEY revision. Two
-        //    options here:
-        //     - MASTER_KEY is missing in local MetaStorage copy. This means that current node have not performed nor
-        //       observed any configuration changes. Valid condition is "MASTER_KEY does not exist".
-        //     - MASTER_KEY is present in local MetaStorage copy. The MASTER_KEY revision is unknown but is less than or
-        //       equal to APPLIED_REV. Obviously, there have been no updates from the future yet. It's also guaranteed
-        //       that the next received configuration update will have the MASTER_KEY revision strictly greater than
-        //       current APPLIED_REV. This allows to conclude that "MASTER_KEY revision <= curChangeId" is a valid
-        //       condition for update.
-        // Combining all of the above, it's concluded that the following condition must be used:
         SimpleCondition condition = curChangeId == 0L
                 ? Conditions.notExists(MASTER_KEY)
                 : Conditions.revision(MASTER_KEY).le(curChangeId);

Review Comment:
   Should replace that with `eq` probably



-- 
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-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org