tkalkirill commented on code in PR #2844:
URL: https://github.com/apache/ignite-3/pull/2844#discussion_r1394164260
##########
modules/configuration/src/main/java/org/apache/ignite/internal/configuration/ConfigurationChanger.java:
##########
@@ -321,9 +322,9 @@ private void persistDefaults() {
}
/**
- * Sets {@link #initialConfiguration}. This configuration will be used to
initialize the configuration if the storage is empty. If the
- * storage is not empty, this configuration will be ignored. This method
must be called before {@link #start()}. If the method is not
- * called, the initial configuration will be empty.
+ * Sets {@link #initialConfiguration}. This configuration will be used to
initialize the configuration if the revision of the storage is
+ * 0. If the revision of the storage is non-zero, this configuration will
be ignored. his method must be called before {@link #start()}.
Review Comment:
```suggestion
* {@code 0}. If the revision of the storage is non-zero, this
configuration will be ignored. his method must be called before {@link
#start()}.
```
##########
modules/runner/src/main/java/org/apache/ignite/internal/app/IgniteImpl.java:
##########
@@ -1043,18 +1038,27 @@ public void init(
}
/**
- * Initializes the cluster configuration with the specified user-provided
configuration upon cluster initialization.
+ * Checks if the local revision is 0 and initializes the cluster
configuration with the specified user-provided configuration upon
Review Comment:
```suggestion
* Checks if the local revision is {@code 0} and initializes the cluster
configuration with the specified user-provided configuration upon
```
##########
modules/configuration/src/main/java/org/apache/ignite/internal/configuration/ConfigurationChanger.java:
##########
@@ -97,8 +97,8 @@ public abstract class ConfigurationChanger implements
DynamicConfigurationChange
private volatile StorageRoots storageRoots;
/**
- * Initial configuration. This configuration will be used to initialize
the configuration if the storage is empty. If the storage is not
- * empty, this configuration will be ignored.
+ * Initial configuration. This configuration will be used to initialize
the configuration if the revision of the storage is 0. If the
Review Comment:
```suggestion
* Initial configuration. This configuration will be used to initialize
the configuration if the revision of the storage is {@code 0}. If the
```
##########
modules/configuration/src/main/java/org/apache/ignite/internal/configuration/storage/ConfigurationStorage.java:
##########
@@ -79,6 +79,11 @@ public interface ConfigurationStorage extends
ManuallyCloseable {
*/
CompletableFuture<Long> lastRevision();
+ /**
+ * Returns a future that will be completed with the latest revision of the
storage.
+ */
Review Comment:
```suggestion
/** Returns a future that will be completed with the latest revision of
the configuration storage. */
```
##########
modules/runner/src/main/java/org/apache/ignite/internal/app/IgniteImpl.java:
##########
@@ -1043,18 +1038,27 @@ public void init(
}
/**
- * Initializes the cluster configuration with the specified user-provided
configuration upon cluster initialization.
+ * Checks if the local revision is 0 and initializes the cluster
configuration with the specified user-provided configuration upon
+ * cluster initialization. If the local revision is not 0, does nothing.
*/
private CompletableFuture<Void>
initializeClusterConfiguration(ExecutorService startupExecutor) {
- return cmgMgr.initialClusterConfigurationFuture().thenAcceptAsync(cfg
-> {
- if (cfg == null) {
- return;
- }
-
- Config config = ConfigFactory.parseString(cfg);
- ConfigurationSource hoconSource =
HoconConverter.hoconSource(config.root());
-
clusterCfgMgr.configurationRegistry().initializeConfigurationWith(hoconSource);
- }, startupExecutor);
+ return cfgStorage.localRevision()
+ .thenComposeAsync(appliedRevision -> {
+ if (appliedRevision == 0) {
+ return completedFuture(null);
+ } else {
+ return cmgMgr.initialClusterConfigurationFuture()
+ .thenAcceptAsync(cfg -> {
Review Comment:
Just the name `cfg` doesn't say much, maybe `initialConfigHocon`?
##########
modules/runner/src/main/java/org/apache/ignite/internal/app/IgniteImpl.java:
##########
@@ -1043,18 +1038,27 @@ public void init(
}
/**
- * Initializes the cluster configuration with the specified user-provided
configuration upon cluster initialization.
+ * Checks if the local revision is 0 and initializes the cluster
configuration with the specified user-provided configuration upon
+ * cluster initialization. If the local revision is not 0, does nothing.
Review Comment:
```suggestion
* cluster initialization. If the local revision is not {@code 0}, does
nothing.
```
##########
modules/runner/src/main/java/org/apache/ignite/internal/configuration/storage/DistributedConfigurationStorage.java:
##########
@@ -265,22 +257,9 @@ public CompletableFuture<Boolean> write(Map<String, ?
extends Serializable> newV
// - 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 MetaStorage's local revision,
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 MetaStorage's local revision. 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 MetaStorage's local revision. This allows to conclude
that "MASTER_KEY revision <= curChangeId" is a valid
- // condition for update.
- // Joining all of the above, it's concluded that the following
condition must be used:
- Condition condition = curChangeId == 0L
+ Condition condition = changeId == 0
Review Comment:
Is it correct to delete the entire comment that was previously there?
--
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]
