This is an automated email from the ASF dual-hosted git repository.
ibessonov pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/ignite-3.git
The following commit(s) were added to refs/heads/main by this push:
new 9934564b0e IGNITE-18516 Allowed safe configuration read from
configuration update closures & other enhancements (#1540)
9934564b0e is described below
commit 9934564b0ee86b896a912f226a3b9409d422b2eb
Author: Ivan Bessonov <[email protected]>
AuthorDate: Wed Jan 18 17:10:21 2023 +0300
IGNITE-18516 Allowed safe configuration read from configuration update
closures & other enhancements (#1540)
---
.../processor/ConfigurationProcessor.java | 9 ++
modules/configuration/README.md | 9 +-
.../configuration/ConfigurationChanger.java | 92 +++++++------
.../configuration/ConfigurationRegistry.java | 36 ++++++
.../internal/configuration/SuperRootChange.java | 36 ++++++
.../configuration/asm/InnerNodeAsmGenerator.java | 144 +++++++++++++--------
.../configuration/ConfigurationChangerTest.java | 20 +++
.../configuration/ConfigurationRegistryTest.java | 49 +++++++
.../asm/ConfigurationAsmGeneratorTest.java | 29 ++++-
.../distributionzones/DistributionZoneManager.java | 1 -
.../DistributedConfigurationCatchUpTest.java | 22 ++--
.../sql/engine/exec/ddl/DdlCommandHandler.java | 1 -
12 files changed, 339 insertions(+), 109 deletions(-)
diff --git
a/modules/configuration-annotation-processor/src/main/java/org/apache/ignite/internal/configuration/processor/ConfigurationProcessor.java
b/modules/configuration-annotation-processor/src/main/java/org/apache/ignite/internal/configuration/processor/ConfigurationProcessor.java
index cc501368e3..45d529f3f3 100644
---
a/modules/configuration-annotation-processor/src/main/java/org/apache/ignite/internal/configuration/processor/ConfigurationProcessor.java
+++
b/modules/configuration-annotation-processor/src/main/java/org/apache/ignite/internal/configuration/processor/ConfigurationProcessor.java
@@ -518,6 +518,15 @@ public class ConfigurationProcessor extends
AbstractProcessor {
}
changeClsBuilder.addMethod(changeMtdBuilder.build());
+
+ // Create "FooChange changeFoo()" method with no parameters, if
it's a config value or named list value.
+ if (valAnnotation == null) {
+ MethodSpec.Builder shortChangeMtdBuilder =
MethodSpec.methodBuilder(changeMtdName)
+ .addModifiers(PUBLIC, ABSTRACT)
+ .returns(changeFieldType);
+
+ changeClsBuilder.addMethod(shortChangeMtdBuilder.build());
+ }
}
if (isPolymorphicConfig) {
diff --git a/modules/configuration/README.md b/modules/configuration/README.md
index 3803736bfa..6eceb06ab1 100644
--- a/modules/configuration/README.md
+++ b/modules/configuration/README.md
@@ -284,10 +284,13 @@ For the example above, the following interfaces would be
generated:
```java
public interface ParentChange extends ParentView {
ParentChange changeElements(Consumer<NamedListChange<NamedElementChange>>
elements);
+ NamedListChange<NamedElementChange> changeElements();
ParentChange changeChild(Consumer<ChildChange> child);
+ ChildChange changeChild();
ParentChange changePolymorphicChild(Consumer<PolymorphicChange>
polymorphicChild);
+ PolymorphicChange changePolymorphicChild();
}
public interface ChildChange extends ChildView {
@@ -314,9 +317,13 @@ parentCfg.change(parent ->
)
).get();
+parentCfg.change(parent ->
+ parent.changeChild().changeStr("newStr2")
+).get();
+
ChildConfiguration childCfg = parentCfg.child();
-childCfg.changeStr("newStr2").get();
+childCfg.changeStr("newStr3").get();
```
Example of changing the type of a polymorphic configuration:
diff --git
a/modules/configuration/src/main/java/org/apache/ignite/internal/configuration/ConfigurationChanger.java
b/modules/configuration/src/main/java/org/apache/ignite/internal/configuration/ConfigurationChanger.java
index a86447cba4..e018ee70d8 100644
---
a/modules/configuration/src/main/java/org/apache/ignite/internal/configuration/ConfigurationChanger.java
+++
b/modules/configuration/src/main/java/org/apache/ignite/internal/configuration/ConfigurationChanger.java
@@ -53,6 +53,8 @@ import java.util.concurrent.ForkJoinPool;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
import java.util.concurrent.atomic.AtomicLong;
+import java.util.concurrent.locks.ReadWriteLock;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
import java.util.function.Function;
import org.apache.ignite.configuration.ConfigurationChangeException;
import org.apache.ignite.configuration.RootKey;
@@ -102,6 +104,9 @@ public abstract class ConfigurationChanger implements
DynamicConfigurationChange
/** Configuration listener notification counter, must be incremented
before each use of {@link #notificator}. */
private final AtomicLong notificationListenerCnt = new AtomicLong();
+ /** Lock for reading/updating the {@link #storageRoots}. Fair, to give a
higher priority to external updates. */
+ private final ReadWriteLock rwLock = new ReentrantReadWriteLock(true);
+
/**
* Closure interface to be used by the configuration changer. An instance
of this closure is passed into the constructor and invoked
* every time when there's an update from any of the storages.
@@ -482,23 +487,16 @@ public abstract class ConfigurationChanger implements
DynamicConfigurationChange
* @throws ComponentNotStartedException if changer is not started.
*/
private CompletableFuture<Void> changeInternally(ConfigurationSource src) {
- StorageRoots localRoots = storageRoots;
-
- if (localRoots == null) {
+ if (storageRoots == null) {
throw new ComponentNotStartedException();
}
return storage.lastRevision()
- .thenCompose(storageRevision -> {
+ .thenComposeAsync(storageRevision -> {
assert storageRevision != null;
- if (localRoots.version < storageRevision) {
- // Need to wait for the configuration updates from the
storage, then try to update again (loop).
- return localRoots.changeFuture.thenCompose(v ->
changeInternally(src));
- } else {
- return changeInternally0(localRoots, src);
- }
- })
+ return changeInternally0(src, storageRevision);
+ }, pool)
.exceptionally(throwable -> {
Throwable cause = throwable.getCause();
@@ -514,41 +512,51 @@ public abstract class ConfigurationChanger implements
DynamicConfigurationChange
* Internal configuration change method that completes provided future.
*
* @param src Configuration source.
+ * @param storageRevision Latest storage revision from the metastorage.
* @return Future that will be completed after changes are written to the
storage.
*/
- private CompletableFuture<Void> changeInternally0(StorageRoots localRoots,
ConfigurationSource src) {
- return CompletableFuture
- .supplyAsync(() -> {
- SuperRoot curRoots = localRoots.roots;
+ private CompletableFuture<Void> changeInternally0(ConfigurationSource src,
long storageRevision) {
+ // Read lock protects "storageRoots" field from being updated, thus
guaranteeing a thread-safe read of configuration inside of the
+ // change closure.
+ rwLock.readLock().lock();
- SuperRoot changes = curRoots.copy();
+ try {
+ // Put it into a variable to avoid volatile reads.
+ // This read and the following comparison MUST be performed while
holding read lock.
+ StorageRoots localRoots = storageRoots;
- src.reset();
+ if (localRoots.version < storageRevision) {
+ // Need to wait for the configuration updates from the
storage, then try to update again (loop).
+ return localRoots.changeFuture.thenCompose(v ->
changeInternally(src));
+ }
- src.descend(changes);
+ SuperRoot curRoots = localRoots.roots;
- addDefaults(changes);
+ SuperRoot changes = curRoots.copy();
- Map<String, Serializable> allChanges =
createFlattenedUpdatesMap(curRoots, changes);
+ src.reset();
- dropNulls(changes);
+ src.descend(changes);
- List<ValidationIssue> validationIssues =
ValidationUtil.validate(
- curRoots,
- changes,
- this::getRootNode,
- cachedAnnotations,
- validators
- );
+ addDefaults(changes);
- if (!validationIssues.isEmpty()) {
- throw new
ConfigurationValidationException(validationIssues);
- }
+ Map<String, Serializable> allChanges =
createFlattenedUpdatesMap(curRoots, changes);
- return allChanges;
- }, pool)
- .thenCompose(allChanges ->
- storage.write(allChanges, localRoots.version)
+ dropNulls(changes);
+
+ List<ValidationIssue> validationIssues = ValidationUtil.validate(
+ curRoots,
+ changes,
+ this::getRootNode,
+ cachedAnnotations,
+ validators
+ );
+
+ if (!validationIssues.isEmpty()) {
+ throw new ConfigurationValidationException(validationIssues);
+ }
+
+ return storage.write(allChanges, localRoots.version)
.thenCompose(casWroteSuccessfully -> {
if (casWroteSuccessfully) {
return localRoots.changeFuture;
@@ -557,8 +565,10 @@ public abstract class ConfigurationChanger implements
DynamicConfigurationChange
// because we work with async code (futures).
return localRoots.changeFuture.thenCompose(v ->
changeInternally(src));
}
- })
- );
+ });
+ } finally {
+ rwLock.readLock().unlock();
+ }
}
/**
@@ -581,7 +591,13 @@ public abstract class ConfigurationChanger implements
DynamicConfigurationChange
long newChangeId = changedEntries.changeId();
- storageRoots = new StorageRoots(newSuperRoot, newChangeId);
+ rwLock.writeLock().lock();
+
+ try {
+ storageRoots = new StorageRoots(newSuperRoot, newChangeId);
+ } finally {
+ rwLock.writeLock().unlock();
+ }
// Save revisions for recovery.
return storage.writeConfigurationRevision(oldStorageRoots.version,
storageRoots.version)
diff --git
a/modules/configuration/src/main/java/org/apache/ignite/internal/configuration/ConfigurationRegistry.java
b/modules/configuration/src/main/java/org/apache/ignite/internal/configuration/ConfigurationRegistry.java
index 82724c9d5e..781ed6be9a 100644
---
a/modules/configuration/src/main/java/org/apache/ignite/internal/configuration/ConfigurationRegistry.java
+++
b/modules/configuration/src/main/java/org/apache/ignite/internal/configuration/ConfigurationRegistry.java
@@ -42,8 +42,10 @@ import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
+import java.util.Objects;
import java.util.Set;
import java.util.concurrent.CompletableFuture;
+import java.util.function.Consumer;
import java.util.function.Function;
import org.apache.ignite.configuration.ConfigurationTree;
import org.apache.ignite.configuration.RootKey;
@@ -62,6 +64,7 @@ import
org.apache.ignite.internal.configuration.notifications.ConfigurationStora
import org.apache.ignite.internal.configuration.storage.ConfigurationStorage;
import org.apache.ignite.internal.configuration.tree.ConfigurationSource;
import org.apache.ignite.internal.configuration.tree.ConfigurationVisitor;
+import org.apache.ignite.internal.configuration.tree.ConstructableTreeNode;
import org.apache.ignite.internal.configuration.tree.InnerNode;
import org.apache.ignite.internal.configuration.tree.TraversableTreeNode;
import org.apache.ignite.internal.configuration.util.ConfigurationUtil;
@@ -267,6 +270,39 @@ public class ConfigurationRegistry implements
IgniteComponent, ConfigurationStor
return changer.change(changesSrc);
}
+ /**
+ * Change configuration. Gives the possibility to atomically update
several root trees.
+ *
+ * @param change Closure that would consume a mutable super root instance.
+ * @return Future that is completed on change completion.
+ */
+ public CompletableFuture<Void> change(Consumer<SuperRootChange> change) {
+ return change(new ConfigurationSource() {
+ @Override
+ public void descend(ConstructableTreeNode node) {
+ assert node instanceof SuperRoot : "Descending always starts
with super root: " + node;
+
+ SuperRoot superRoot = (SuperRoot) node;
+
+ change.accept(new SuperRootChange() {
+ @Override
+ public <V> V viewRoot(RootKey<? extends
ConfigurationTree<V, ?>, V> rootKey) {
+ return
Objects.requireNonNull(superRoot.getRoot(rootKey)).specificNode();
+ }
+
+ @Override
+ public <C> C changeRoot(RootKey<? extends
ConfigurationTree<?, C>, ?> rootKey) {
+ // "construct" does a field copying, which is what we
need before mutating it.
+ superRoot.construct(rootKey.key(),
ConfigurationUtil.EMPTY_CFG_SRC, true);
+
+ // "rootView" is not re-used here because of return
type incompatibility, although code is the same.
+ return
Objects.requireNonNull(superRoot.getRoot(rootKey)).specificNode();
+ }
+ });
+ }
+ });
+ }
+
/**
* Configuration change notifier.
*
diff --git
a/modules/configuration/src/main/java/org/apache/ignite/internal/configuration/SuperRootChange.java
b/modules/configuration/src/main/java/org/apache/ignite/internal/configuration/SuperRootChange.java
new file mode 100644
index 0000000000..f622c9b563
--- /dev/null
+++
b/modules/configuration/src/main/java/org/apache/ignite/internal/configuration/SuperRootChange.java
@@ -0,0 +1,36 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.configuration;
+
+import org.apache.ignite.configuration.ConfigurationTree;
+import org.apache.ignite.configuration.RootKey;
+
+/**
+ * Interface that represent a "change" for the conjunction of all roots in the
configuration.
+ */
+public interface SuperRootChange {
+ /**
+ * Returns a root view for the root key.
+ */
+ <V> V viewRoot(RootKey<? extends ConfigurationTree<V, ?>, V> rootKey);
+
+ /**
+ * Returns a root change for the root key.
+ */
+ <C> C changeRoot(RootKey<? extends ConfigurationTree<?, C>, ?> rootKey);
+}
diff --git
a/modules/configuration/src/main/java/org/apache/ignite/internal/configuration/asm/InnerNodeAsmGenerator.java
b/modules/configuration/src/main/java/org/apache/ignite/internal/configuration/asm/InnerNodeAsmGenerator.java
index 94afc231dc..fd6ac4a357 100644
---
a/modules/configuration/src/main/java/org/apache/ignite/internal/configuration/asm/InnerNodeAsmGenerator.java
+++
b/modules/configuration/src/main/java/org/apache/ignite/internal/configuration/asm/InnerNodeAsmGenerator.java
@@ -91,6 +91,7 @@ import java.util.function.BiFunction;
import java.util.function.Consumer;
import java.util.function.Function;
import
org.apache.ignite.configuration.ConfigurationWrongPolymorphicTypeIdException;
+import org.apache.ignite.configuration.NamedListChange;
import org.apache.ignite.configuration.NamedListView;
import org.apache.ignite.configuration.annotation.AbstractConfiguration;
import org.apache.ignite.configuration.annotation.InjectedName;
@@ -345,7 +346,7 @@ class InnerNodeAsmGenerator extends AbstractAsmGenerator {
}
// Add change methods.
- MethodDefinition changeMtd0 = addNodeChangeMethod(
+ List<MethodDefinition> changeMethods = addNodeChangeMethod(
innerNodeClassDef,
schemaField,
changeMtd -> getThisFieldCode(changeMtd, fieldDef),
@@ -353,7 +354,8 @@ class InnerNodeAsmGenerator extends AbstractAsmGenerator {
null
);
- addNodeChangeBridgeMethod(innerNodeClassDef,
changeClassName(schemaField.getDeclaringClass()), changeMtd0);
+ // Only first element requires a bridge. Please refer to
"addNodeChangeMethod" for explanation.
+ addNodeChangeBridgeMethod(innerNodeClassDef,
changeClassName(schemaField.getDeclaringClass()), changeMethods.get(0));
}
Map<Class<?>, List<Field>> polymorphicFieldsByExtension = Map.of();
@@ -659,24 +661,7 @@ class InnerNodeAsmGenerator extends AbstractAsmGenerator {
// return result;
bytecodeBlock.ret(schemaFieldType);
- if (getPolymorphicTypeIdFieldFun != null) {
- assert
isPolymorphicConfigInstance(schemaField.getDeclaringClass()) : schemaField;
-
- // tmpVar = this.typeId; OR this.field.typeId.
- BytecodeExpression getPolymorphicTypeIdFieldValue =
getPolymorphicTypeIdFieldFun.apply(viewMtd);
- String polymorphicInstanceId =
polymorphicInstanceId(schemaField.getDeclaringClass());
-
- // if (!"first".equals(tmpVar)) throw Ex;
- // else return value;
- viewMtd.getBody().append(
- new IfStatement()
-
.condition(not(constantString(polymorphicInstanceId).invoke(STRING_EQUALS_MTD,
getPolymorphicTypeIdFieldValue)))
-
.ifTrue(throwException(ConfigurationWrongPolymorphicTypeIdException.class,
getPolymorphicTypeIdFieldValue))
- .ifFalse(bytecodeBlock)
- );
- } else {
- viewMtd.getBody().append(bytecodeBlock);
- }
+ enrichWithPolymorphicTypeCheck(schemaField,
getPolymorphicTypeIdFieldFun, viewMtd, bytecodeBlock);
}
/**
@@ -684,9 +669,10 @@ class InnerNodeAsmGenerator extends AbstractAsmGenerator {
*
* @param classDef Node class definition.
* @param schemaField Configuration schema class field.
- * @return Definition of change method.
+ * @return List of method definition. First element is the "default"
change method that accepts closure or a value. Second, optional
+ * element, is a change methods with no parameters.
*/
- private MethodDefinition addNodeChangeMethod(
+ private List<MethodDefinition> addNodeChangeMethod(
ClassDefinition classDef,
Field schemaField,
Function<MethodDefinition, BytecodeExpression> getFieldCodeFun,
@@ -703,6 +689,8 @@ class InnerNodeAsmGenerator extends AbstractAsmGenerator {
arg("change", isValue(schemaField) ? type(schemaFieldType) :
type(Consumer.class))
);
+ MethodDefinition shortChangeMtd = null;
+
// var change;
BytecodeExpression changeVar =
changeMtd.getScope().getVariable("change");
@@ -731,56 +719,106 @@ class InnerNodeAsmGenerator extends AbstractAsmGenerator
{
// this.field = newValue;
bytecodeBlock.append(setFieldCodeFun.apply(changeMtd, newValue));
} else {
- BytecodeExpression newValue;
+ shortChangeMtd = createShortChangeMethod(classDef, schemaField,
getFieldCodeFun, setFieldCodeFun, getPolymorphicTypeIdFieldFun);
- if (isConfigValue(schemaField)) {
- // newValue = (this.field == null) ? new ValueNode() :
(ValueNode)this.field.copy();
- newValue = cgen.newOrCopyNodeField(schemaField,
getFieldCodeFun.apply(changeMtd));
- } else {
- assert isNamedConfigValue(schemaField) : schemaField;
+ // change.accept(this.field); OR
change.accept(this.field.specificNode());
+ bytecodeBlock.append(changeVar.invoke(ACCEPT,
changeMtd.getThis().invoke(shortChangeMtd, List.of())));
+ }
- // newValue = (ValueNode)this.field.copy();
- newValue = cgen.copyNodeField(schemaField,
getFieldCodeFun.apply(changeMtd));
- }
+ // return this;
+ bytecodeBlock.append(changeMtd.getThis()).retObject();
- // this.field = newValue;
- bytecodeBlock.append(setFieldCodeFun.apply(changeMtd, newValue));
+ enrichWithPolymorphicTypeCheck(schemaField,
getPolymorphicTypeIdFieldFun, changeMtd, bytecodeBlock);
- // this.field;
- BytecodeExpression getFieldCode = getFieldCodeFun.apply(changeMtd);
+ if (shortChangeMtd == null) {
+ return List.of(changeMtd);
+ }
- if (isPolymorphicConfig(schemaFieldType) &&
isConfigValue(schemaField)) {
- // this.field.specificNode();
- getFieldCode = getFieldCode.invoke(SPECIFIC_NODE_MTD);
- }
+ return List.of(changeMtd, shortChangeMtd);
+ }
- // change.accept(this.field); OR
change.accept(this.field.specificNode());
- bytecodeBlock.append(changeVar.invoke(ACCEPT, getFieldCode));
+ /**
+ * Creates a "short" method to return a changed field instance. Name is
the same as for {@code changeFoo(Consumer<FooChange> change)"}.
+ * This method will be reused to create the value that is passed to
"default" change method.
+ * Method's signature is {@code FooChange changeFoo()}, it returns a
mutable configuration value to be used for configuration updates.
+ */
+ private MethodDefinition createShortChangeMethod(
+ ClassDefinition classDef,
+ Field schemaField,
+ Function<MethodDefinition, BytecodeExpression> getFieldCodeFun,
+ BiFunction<MethodDefinition, BytecodeExpression,
BytecodeExpression> setFieldCodeFun,
+ @Nullable Function<MethodDefinition, BytecodeExpression>
getPolymorphicTypeIdFieldFun
+ ) {
+ MethodDefinition shortChangeMtd = classDef.declareMethod(
+ EnumSet.of(PUBLIC),
+ changeMethodName(schemaField.getName()),
+ isConfigValue(schemaField)
+ ?
typeFromJavaClassName(changeClassName(schemaField.getType()))
+ : type(NamedListChange.class)
+ );
+
+ BytecodeBlock shortBytecodeBlock = new BytecodeBlock();
+
+ BytecodeExpression newValue;
+
+ if (isConfigValue(schemaField)) {
+ // newValue = (this.field == null) ? new ValueNode() :
(ValueNode)this.field.copy();
+ newValue = cgen.newOrCopyNodeField(schemaField,
getFieldCodeFun.apply(shortChangeMtd));
+ } else {
+ assert isNamedConfigValue(schemaField) : schemaField;
+
+ // newValue = (ValueNode)this.field.copy();
+ newValue = cgen.copyNodeField(schemaField,
getFieldCodeFun.apply(shortChangeMtd));
}
- // return this;
- bytecodeBlock.append(changeMtd.getThis()).retObject();
+ // this.field = newValue;
+ shortBytecodeBlock.append(setFieldCodeFun.apply(shortChangeMtd,
newValue));
+
+ // this.field;
+ BytecodeExpression getFieldCode =
getFieldCodeFun.apply(shortChangeMtd);
+
+ if (isPolymorphicConfig(schemaField.getType()) &&
isConfigValue(schemaField)) {
+ // this.field.specificNode();
+ getFieldCode = getFieldCode.invoke(SPECIFIC_NODE_MTD);
+ }
+
+ shortBytecodeBlock.append(getFieldCode).retObject();
+
+ enrichWithPolymorphicTypeCheck(schemaField,
getPolymorphicTypeIdFieldFun, shortChangeMtd, shortBytecodeBlock);
+
+ shortChangeMtd.getBody().append(shortBytecodeBlock);
+ return shortChangeMtd;
+ }
+
+ /**
+ * Adds a check that expected polymorphic type ID matches the real one.
Throws exception otherwise. Simply adds {@code bytecodeBlock}
+ * into methods body if {@code getPolymorphicTypeIdFieldFun} is {@code
null} (this means that class is not polymorphic).
+ */
+ private void enrichWithPolymorphicTypeCheck(
+ Field schemaField,
+ @Nullable Function<MethodDefinition, BytecodeExpression>
getPolymorphicTypeIdFieldFun,
+ MethodDefinition method,
+ BytecodeBlock bytecodeBlock
+ ) {
if (getPolymorphicTypeIdFieldFun != null) {
assert
isPolymorphicConfigInstance(schemaField.getDeclaringClass()) : schemaField;
// tmpVar = this.typeId; OR this.field.typeId.
- BytecodeExpression getPolymorphicTypeIdFieldValue =
getPolymorphicTypeIdFieldFun.apply(changeMtd);
+ BytecodeExpression getPolymorphicTypeIdFieldValue =
getPolymorphicTypeIdFieldFun.apply(method);
String polymorphicInstanceId =
polymorphicInstanceId(schemaField.getDeclaringClass());
// if (!"first".equals(tmpVar)) throw Ex;
// else change_value;
- changeMtd.getBody().append(
+ method.getBody().append(
new IfStatement()
.condition(not(constantString(polymorphicInstanceId).invoke(STRING_EQUALS_MTD,
getPolymorphicTypeIdFieldValue)))
.ifTrue(throwException(ConfigurationWrongPolymorphicTypeIdException.class,
getPolymorphicTypeIdFieldValue))
.ifFalse(bytecodeBlock)
);
} else {
- changeMtd.getBody().append(bytecodeBlock);
+ method.getBody().append(bytecodeBlock);
}
-
- return changeMtd;
}
/**
@@ -1586,7 +1624,7 @@ class InnerNodeAsmGenerator extends AbstractAsmGenerator {
continue;
}
- MethodDefinition changeMtd0 = addNodeChangeMethod(
+ List<MethodDefinition> changeMethods = addNodeChangeMethod(
classDef,
schemaField,
changeMtd -> getThisFieldCode(changeMtd,
parentInnerNodeFieldDef, schemaFieldDef),
@@ -1594,7 +1632,8 @@ class InnerNodeAsmGenerator extends AbstractAsmGenerator {
null
);
- addNodeChangeBridgeMethod(classDef,
schemaClassInfo.changeClassName, changeMtd0);
+ // Only first element requires a bridge. Please refer to
"addNodeChangeMethod" for explanation.
+ addNodeChangeBridgeMethod(classDef,
schemaClassInfo.changeClassName, changeMethods.get(0));
}
FieldDefinition polymorphicTypeIdFieldDef =
fieldDefs.get(polymorphicIdField(schemaClass).getName());
@@ -1610,7 +1649,7 @@ class InnerNodeAsmGenerator extends AbstractAsmGenerator {
viewMtd -> getThisFieldCode(viewMtd,
parentInnerNodeFieldDef, polymorphicTypeIdFieldDef)
);
- MethodDefinition changeMtd0 = addNodeChangeMethod(
+ List<MethodDefinition> changeMethods = addNodeChangeMethod(
classDef,
polymorphicField,
changeMtd -> getThisFieldCode(changeMtd,
parentInnerNodeFieldDef, polymorphicFieldDef),
@@ -1618,7 +1657,8 @@ class InnerNodeAsmGenerator extends AbstractAsmGenerator {
changeMtd -> getThisFieldCode(changeMtd,
parentInnerNodeFieldDef, polymorphicTypeIdFieldDef)
);
- addNodeChangeBridgeMethod(classDef,
polymorphicExtensionClassInfo.changeClassName, changeMtd0);
+ // Only first element requires a bridge. Please refer to
"addNodeChangeMethod" for explanation.
+ addNodeChangeBridgeMethod(classDef,
polymorphicExtensionClassInfo.changeClassName, changeMethods.get(0));
}
ParameterizedType returnType =
typeFromJavaClassName(schemaClassInfo.changeClassName);
diff --git
a/modules/configuration/src/test/java/org/apache/ignite/internal/configuration/ConfigurationChangerTest.java
b/modules/configuration/src/test/java/org/apache/ignite/internal/configuration/ConfigurationChangerTest.java
index a65ad892d9..59b1be5292 100644
---
a/modules/configuration/src/test/java/org/apache/ignite/internal/configuration/ConfigurationChangerTest.java
+++
b/modules/configuration/src/test/java/org/apache/ignite/internal/configuration/ConfigurationChangerTest.java
@@ -142,6 +142,26 @@ public class ConfigurationChangerTest {
assertEquals("1", newRoot.elements().get("a").strCfg());
}
+ /**
+ * Test simple change of configuration.
+ */
+ @Test
+ public void testSimpleConfigurationChangeWithoutExtraLambdas() throws
Exception {
+ ConfigurationChanger changer = createChanger(KEY);
+ changer.start();
+
+ changer.change(source(KEY, (FirstChange parent) -> {
+ parent.changeChild().changeIntCfg(1).changeStrCfg("1");
+ parent.changeElements().create("a", element ->
element.changeStrCfg("1"));
+ })).get(1, SECONDS);
+
+ FirstView newRoot = (FirstView) changer.getRootNode(KEY);
+
+ assertEquals(1, newRoot.child().intCfg());
+ assertEquals("1", newRoot.child().strCfg());
+ assertEquals("1", newRoot.elements().get("a").strCfg());
+ }
+
/**
* Test subsequent change of configuration via different changers.
*/
diff --git
a/modules/configuration/src/test/java/org/apache/ignite/internal/configuration/ConfigurationRegistryTest.java
b/modules/configuration/src/test/java/org/apache/ignite/internal/configuration/ConfigurationRegistryTest.java
index e88356d1f7..bb6ca661ff 100644
---
a/modules/configuration/src/test/java/org/apache/ignite/internal/configuration/ConfigurationRegistryTest.java
+++
b/modules/configuration/src/test/java/org/apache/ignite/internal/configuration/ConfigurationRegistryTest.java
@@ -19,12 +19,18 @@ package org.apache.ignite.internal.configuration;
import static java.util.concurrent.TimeUnit.SECONDS;
import static
org.apache.ignite.configuration.annotation.ConfigurationType.LOCAL;
+import static
org.apache.ignite.internal.testframework.matchers.CompletableFutureMatcher.willCompleteSuccessfully;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.is;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertNotSame;
+import static org.junit.jupiter.api.Assertions.assertSame;
import static org.junit.jupiter.api.Assertions.assertThrows;
import java.util.List;
import java.util.Set;
+import java.util.concurrent.CompletableFuture;
import java.util.function.Consumer;
import org.apache.ignite.configuration.annotation.Config;
import org.apache.ignite.configuration.annotation.ConfigValue;
@@ -156,6 +162,49 @@ public class ConfigurationRegistryTest {
}
}
+ @Test
+ void testChangeSuperRoot() throws Exception {
+ TestConfigurationStorage storage = new TestConfigurationStorage(LOCAL);
+
+ var registry = new ConfigurationRegistry(
+ List.of(FirstRootConfiguration.KEY,
SecondRootConfiguration.KEY),
+ Set.of(),
+ storage,
+ List.of(),
+ List.of()
+ );
+
+ registry.start();
+
+ try {
+ FirstRootConfiguration firstConfiguration =
registry.getConfiguration(FirstRootConfiguration.KEY);
+ SecondRootConfiguration secondConfiguration =
registry.getConfiguration(SecondRootConfiguration.KEY);
+
+ CompletableFuture<Void> changeFuture =
registry.change(superRootChange -> {
+ assertNotNull(superRootChange);
+
+ // Check that originally we have the same value for the root.
+ assertSame(firstConfiguration.value(),
superRootChange.viewRoot(FirstRootConfiguration.KEY));
+
+ FirstRootChange firstRootChange =
superRootChange.changeRoot(FirstRootConfiguration.KEY);
+
+ // Check that the value of the root has changed.
+ assertNotSame(firstConfiguration.value(), firstRootChange);
+ assertSame(firstRootChange,
superRootChange.viewRoot(FirstRootConfiguration.KEY));
+
+ firstRootChange.changeStr("foo");
+
superRootChange.changeRoot(SecondRootConfiguration.KEY).changeStr("bar");
+ });
+
+ assertThat(changeFuture, willCompleteSuccessfully());
+
+ assertEquals("foo", firstConfiguration.str().value());
+ assertEquals("bar", secondConfiguration.str().value());
+ } finally {
+ registry.stop();
+ }
+ }
+
private Consumer<FourthPolymorphicChange> toFirst0Polymorphic(int v) {
return c ->
c.convert(Fourth0PolymorphicChange.class).changeIntVal(v).changeStrVal(Integer.toString(v));
}
diff --git
a/modules/configuration/src/test/java/org/apache/ignite/internal/configuration/asm/ConfigurationAsmGeneratorTest.java
b/modules/configuration/src/test/java/org/apache/ignite/internal/configuration/asm/ConfigurationAsmGeneratorTest.java
index 53d59b324c..36efcd9a66 100644
---
a/modules/configuration/src/test/java/org/apache/ignite/internal/configuration/asm/ConfigurationAsmGeneratorTest.java
+++
b/modules/configuration/src/test/java/org/apache/ignite/internal/configuration/asm/ConfigurationAsmGeneratorTest.java
@@ -203,6 +203,12 @@ public class ConfigurationAsmGeneratorTest {
((ExtendedTestChange) c).changeStr3("str3").changeStr2("str2");
((ExtendedSecondTestChange) c).changeI1(200).changeStr2("str2");
}).get(1, SECONDS);
+
+ rootConfig.change(r -> {
+ TestChange subCfg = r.changeSubCfg().changeStr2("str3");
+
+ assertTrue(subCfg instanceof ExtendedTestChange);
+ }).get(1, SECONDS);
}
@Test
@@ -305,6 +311,13 @@ public class ConfigurationAsmGeneratorTest {
assertEquals("strVal3", firstCfg.strVal().value());
assertEquals(3, firstCfg.intVal().value());
+ rootConfig.change(c -> ((FirstPolymorphicInstanceTestChange)
c.changePolymorphicSubCfg()).changeIntVal(4).changeStrVal("strVal4"))
+ .get(1, SECONDS);
+
+ assertEquals("first", firstCfg.typeId().value());
+ assertEquals("strVal4", firstCfg.strVal().value());
+ assertEquals(4, firstCfg.intVal().value());
+
// Check convert.
rootConfig.polymorphicSubCfg()
@@ -335,13 +348,13 @@ public class ConfigurationAsmGeneratorTest {
SecondPolymorphicInstanceTestConfiguration secondCfg =
(SecondPolymorphicInstanceTestConfiguration) rootConfig.polymorphicSubCfg();
assertEquals("second", secondCfg.typeId().value());
- assertEquals("strVal3", secondCfg.strVal().value());
+ assertEquals("strVal4", secondCfg.strVal().value());
assertEquals(0, secondCfg.intVal().value());
assertEquals(0L, secondCfg.longVal().value());
SecondPolymorphicInstanceTestView secondView =
(SecondPolymorphicInstanceTestView) secondCfg.value();
assertEquals("second", secondView.typeId());
- assertEquals("strVal3", secondView.strVal());
+ assertEquals("strVal4", secondView.strVal());
assertEquals(0, secondView.intVal());
assertEquals(0L, secondView.longVal());
@@ -349,7 +362,7 @@ public class ConfigurationAsmGeneratorTest {
firstCfg = (FirstPolymorphicInstanceTestConfiguration)
rootConfig.polymorphicSubCfg();
assertEquals("first", firstCfg.typeId().value());
- assertEquals("strVal3", firstCfg.strVal().value());
+ assertEquals("strVal4", firstCfg.strVal().value());
assertEquals(0, firstCfg.intVal().value());
}
@@ -644,6 +657,16 @@ public class ConfigurationAsmGeneratorTest {
assertEquals(2,
rootFromAbstractConfig.configFromAbstract().intVal().value());
assertTrue(rootFromAbstractConfig.configFromAbstract().booleanVal().value());
+
+ // Check "short" version of change method.
+ rootFromAbstractConfig.change(ch0 -> ch0
+
.changeConfigFromAbstract().changeBooleanVal(true).changeIntVal(3)
+ ).get(1, SECONDS);
+
+ RootFromAbstractView fromAbstractView2 =
rootFromAbstractConfig.value();
+
+ assertEquals(3, fromAbstractView2.configFromAbstract().intVal());
+ assertTrue(fromAbstractView2.configFromAbstract().booleanVal());
}
@Test
diff --git
a/modules/distribution-zones/src/main/java/org/apache/ignite/internal/distributionzones/DistributionZoneManager.java
b/modules/distribution-zones/src/main/java/org/apache/ignite/internal/distributionzones/DistributionZoneManager.java
index bd830de458..5d82f6cfae 100644
---
a/modules/distribution-zones/src/main/java/org/apache/ignite/internal/distributionzones/DistributionZoneManager.java
+++
b/modules/distribution-zones/src/main/java/org/apache/ignite/internal/distributionzones/DistributionZoneManager.java
@@ -368,7 +368,6 @@ public class DistributionZoneManager implements
IgniteComponent {
throw new DistributionZoneNotFoundException(name);
}
- //TODO: IGNITE-18516 Access to other configuration must be
thread safe.
NamedConfigurationTree<TableConfiguration, TableView,
TableChange> tables = tablesConfiguration.tables();
for (int i = 0; i < tables.value().size(); i++) {
diff --git
a/modules/runner/src/test/java/org/apache/ignite/internal/configuration/storage/DistributedConfigurationCatchUpTest.java
b/modules/runner/src/test/java/org/apache/ignite/internal/configuration/storage/DistributedConfigurationCatchUpTest.java
index 81cf5d61c7..c7b17fb09a 100644
---
a/modules/runner/src/test/java/org/apache/ignite/internal/configuration/storage/DistributedConfigurationCatchUpTest.java
+++
b/modules/runner/src/test/java/org/apache/ignite/internal/configuration/storage/DistributedConfigurationCatchUpTest.java
@@ -190,18 +190,10 @@ public class DistributedConfigurationCatchUpTest {
// On any invocation - trigger storage listener.
when(mock.invoke(any(), anyCollection(), any()))
- .then(invocation -> {
- triggerStorageListener();
-
- return CompletableFuture.completedFuture(true);
- });
+ .then(invocation -> triggerStorageListener());
when(mock.invoke(any(), any(Operation.class), any()))
- .then(invocation -> {
- triggerStorageListener();
-
- return CompletableFuture.completedFuture(true);
- });
+ .then(invocation -> triggerStorageListener());
// This captures the listener.
when(mock.registerPrefixWatch(any(), any())).then(invocation -> {
@@ -214,9 +206,13 @@ public class DistributedConfigurationCatchUpTest {
/**
* Triggers MetaStorage listener incrementing master key revision.
*/
- private void triggerStorageListener() {
- EntryEvent entryEvent = new EntryEvent(null, new
EntryImpl(MASTER_KEY.bytes(), null, ++masterKeyRevision, -1));
- lsnr.onUpdate(new WatchEvent(entryEvent));
+ private CompletableFuture<Boolean> triggerStorageListener() {
+ return CompletableFuture.supplyAsync(() -> {
+ EntryEvent entryEvent = new EntryEvent(null, new
EntryImpl(MASTER_KEY.bytes(), null, ++masterKeyRevision, -1));
+ lsnr.onUpdate(new WatchEvent(entryEvent));
+
+ return true;
+ });
}
private MetaStorageManager metaStorageManager() {
diff --git
a/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/ddl/DdlCommandHandler.java
b/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/ddl/DdlCommandHandler.java
index c79b5616e6..dc33133f40 100644
---
a/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/ddl/DdlCommandHandler.java
+++
b/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/ddl/DdlCommandHandler.java
@@ -210,7 +210,6 @@ public class DdlCommandHandler {
}
if (cmd.zone() != null) {
- //TODO: IGNITE-18516 Access to other configuration must be
thread safe.
tableChange.changeZoneId(distributionZoneManager.getZoneId(cmd.zone()));
} else {
tableChange.changeZoneId(DEFAULT_ZONE_ID);
