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 a29a3e3fee2 IGNITE-25458 Fix deletion of renamed configuration from
the storage (#6028)
a29a3e3fee2 is described below
commit a29a3e3fee2176b9e41ee6a2defaf45ddcf5ae60
Author: Ivan Bessonov <[email protected]>
AuthorDate: Mon Jun 16 10:29:43 2025 +0300
IGNITE-25458 Fix deletion of renamed configuration from the storage (#6028)
---
.../configuration/ConfigurationChanger.java | 69 ++++++------
.../configuration/util/ConfigurationFlattener.java | 122 +++++++++++++-------
.../configuration/util/ConfigurationUtil.java | 77 +++++++++++++
.../util/KeysTrackingConfigurationVisitor.java | 125 ++++++++++-----------
.../configuration/RenamedConfigurationTest.java | 106 +++++++++++++++--
.../configuration/util/ConfigurationUtilTest.java | 33 ++++--
.../storage/LocalFileConfigurationStorage.java | 3 +-
7 files changed, 380 insertions(+), 155 deletions(-)
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 e4c40dbd1f9..8e51df632e0 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
@@ -34,6 +34,7 @@ import static
org.apache.ignite.internal.configuration.util.ConfigurationUtil.dr
import static
org.apache.ignite.internal.configuration.util.ConfigurationUtil.escape;
import static
org.apache.ignite.internal.configuration.util.ConfigurationUtil.fillFromPrefixMap;
import static
org.apache.ignite.internal.configuration.util.ConfigurationUtil.findEx;
+import static
org.apache.ignite.internal.configuration.util.ConfigurationUtil.ignoreLegacyKeys;
import static
org.apache.ignite.internal.configuration.util.ConfigurationUtil.toPrefixMap;
import static
org.apache.ignite.internal.util.CompletableFutures.nullCompletedFuture;
@@ -44,9 +45,11 @@ import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
+import java.util.NavigableMap;
import java.util.NoSuchElementException;
import java.util.RandomAccess;
import java.util.StringJoiner;
+import java.util.TreeMap;
import java.util.UUID;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ExecutionException;
@@ -71,7 +74,6 @@ import
org.apache.ignite.internal.configuration.tree.ConstructableTreeNode;
import org.apache.ignite.internal.configuration.tree.InnerNode;
import org.apache.ignite.internal.configuration.tree.NamedListNode;
import org.apache.ignite.internal.configuration.util.ConfigurationUtil;
-import
org.apache.ignite.internal.configuration.util.KeysTrackingConfigurationVisitor;
import
org.apache.ignite.internal.configuration.validation.ConfigurationValidator;
import org.apache.ignite.internal.lang.IgniteInternalException;
import org.apache.ignite.internal.lang.NodeStoppingException;
@@ -156,8 +158,11 @@ public abstract class ConfigurationChanger implements
DynamicConfigurationChange
/** Immutable forest, so to say. */
private final SuperRoot roots;
+ /** Change ID that corresponds to this state. */
+ private final long changeId;
+
/** Full storage data. */
- private final Data data;
+ private final NavigableMap<String, ? extends Serializable> storageData;
/** Future that signifies update of current configuration. */
private final CompletableFuture<Void> changeFuture = new
CompletableFuture<>();
@@ -167,12 +172,19 @@ public abstract class ConfigurationChanger implements
DynamicConfigurationChange
*
* @param rootsWithoutDefaults Forest without the defaults
* @param roots Forest with the defaults filled in
- * @param data Configuration storage state.
+ * @param changeId Change ID that corresponds to this state.
+ * @param storageData Full storage data.
*/
- private StorageRoots(SuperRoot rootsWithoutDefaults, SuperRoot roots,
Data data) {
+ private StorageRoots(
+ SuperRoot rootsWithoutDefaults,
+ SuperRoot roots,
+ long changeId,
+ NavigableMap<String, ? extends Serializable> storageData
+ ) {
this.rootsWithoutDefaults = rootsWithoutDefaults;
this.roots = roots;
- this.data = data;
+ this.changeId = changeId;
+ this.storageData = storageData;
makeImmutable(roots);
makeImmutable(rootsWithoutDefaults);
@@ -315,7 +327,7 @@ public abstract class ConfigurationChanger implements
DynamicConfigurationChange
// The root WITHOUT the defaults is used to calculate which properties
to write to the underlying storage,
// in other words it allows us to persist the defaults from the code.
// After the storage listener fires for the first time both roots are
supposed to become equal.
- storageRoots = new StorageRoots(superRootNoDefaults, superRoot, data);
+ storageRoots = new StorageRoots(superRootNoDefaults, superRoot,
data.changeId(), new TreeMap<>(data.values()));
storage.registerConfigurationListener(configurationStorageListener());
@@ -333,7 +345,7 @@ public abstract class ConfigurationChanger implements
DynamicConfigurationChange
private void persistModifiedConfiguration() {
// If the storage version is 0, it indicates that the storage is empty.
// In this case, write the defaults along with the initial
configuration.
- ConfigurationSource cfgSrc = storageRoots.data.changeId() == 0 ?
initialConfiguration : ConfigurationUtil.EMPTY_CFG_SRC;
+ ConfigurationSource cfgSrc = storageRoots.changeId == 0 ?
initialConfiguration : ConfigurationUtil.EMPTY_CFG_SRC;
changeInternally(cfgSrc, true)
.whenComplete((v, e) -> {
@@ -619,7 +631,7 @@ public abstract class ConfigurationChanger implements
DynamicConfigurationChange
// This read and the following comparison MUST be performed while
holding read lock.
StorageRoots localRoots = storageRoots;
- if (localRoots.data.changeId() < storageRevision) {
+ if (localRoots.changeId < storageRevision) {
// Need to wait for the configuration updates from the
storage, then try to update again (loop).
return localRoots.changeFuture.thenCompose(v ->
changeInternally(src, onStartup));
}
@@ -639,7 +651,7 @@ public abstract class ConfigurationChanger implements
DynamicConfigurationChange
Map<String, Serializable> allChanges = createFlattenedUpdatesMap(
localRoots.rootsWithoutDefaults,
changes,
- localRoots.data.values()
+ localRoots.storageData
);
if (onStartup) {
@@ -663,7 +675,7 @@ public abstract class ConfigurationChanger implements
DynamicConfigurationChange
// still try to write the empty update, because local
configuration can be obsolete. If this is the case, then the CAS will
// fail and the update will be recalculated and there is a chance
that the new local configuration will produce a non-empty
// update.
- return storage.write(allChanges, localRoots.data.changeId())
+ return storage.write(allChanges, localRoots.changeId)
.thenCompose(casWroteSuccessfully -> {
if (casWroteSuccessfully) {
return localRoots.changeFuture;
@@ -709,10 +721,9 @@ public abstract class ConfigurationChanger implements
DynamicConfigurationChange
// We need to ignore deletion of deprecated values.
ignoreDeleted(changedValues, keyIgnorer);
- // We need to ignore deletion of legacy values.
- ignoreLegacyKeys(oldStorageRoots, changedValues);
Map<String, ?> dataValuesPrefixMap =
toPrefixMap(changedValues);
+ ignoreLegacyKeys(oldStorageRoots.roots, dataValuesPrefixMap);
compressDeletedEntries(dataValuesPrefixMap);
@@ -721,7 +732,8 @@ public abstract class ConfigurationChanger implements
DynamicConfigurationChange
long newChangeId = changedEntries.changeId();
- var newStorageRoots = new StorageRoots(newSuperNoDefaults,
newSuperRoot, mergeData(oldStorageRoots.data, changedEntries));
+ NavigableMap<String, ? extends Serializable> newData =
mergeData(oldStorageRoots.storageData, changedEntries.values());
+ var newStorageRoots = new StorageRoots(newSuperNoDefaults,
newSuperRoot, newChangeId, newData);
rwLock.writeLock().lock();
@@ -751,12 +763,13 @@ public abstract class ConfigurationChanger implements
DynamicConfigurationChange
};
}
- private static Data mergeData(Data currentData, Data delta) {
- assert delta.changeId() > currentData.changeId() :
currentData.changeId() + " " + delta.changeId();
-
- Map<String, Serializable> newState = new
HashMap<>(currentData.values());
+ private static NavigableMap<String, ? extends Serializable> mergeData(
+ NavigableMap<String, ? extends Serializable> currentData,
+ Map<String, ? extends Serializable> delta
+ ) {
+ NavigableMap<String, Serializable> newState = new
TreeMap<>(currentData);
- for (Entry<String, ? extends Serializable> entry :
delta.values().entrySet()) {
+ for (Entry<String, ? extends Serializable> entry : delta.entrySet()) {
if (entry.getValue() == null) {
newState.remove(entry.getKey());
} else {
@@ -764,25 +777,15 @@ public abstract class ConfigurationChanger implements
DynamicConfigurationChange
}
}
- return new Data(newState, delta.changeId());
- }
-
- private static void ignoreLegacyKeys(StorageRoots oldStorageRoots,
Map<String, ? extends Serializable> changedValues) {
- oldStorageRoots.roots.traverseChildren(new
KeysTrackingConfigurationVisitor<>() {
- @Override
- protected Object doVisitLeafNode(Field field, String key,
Serializable val) {
- processLegacyPaths(changedValues::remove);
-
- return null;
- }
- }, true);
+ return newState;
}
/**
- * Remove keys from {@code allChanges}, that are associated with nulls in
this map, and already absent in {@link StorageRoots#data}.
+ * Remove keys from {@code allChanges}, that are associated with nulls in
this map, and already absent in
+ * {@link StorageRoots#storageData}.
*/
private static void dropUnnecessarilyDeletedKeys(Map<String, Serializable>
allChanges, StorageRoots localRoots) {
- allChanges.entrySet().removeIf(entry -> entry.getValue() == null &&
!localRoots.data.values().containsKey(entry.getKey()));
+ allChanges.entrySet().removeIf(entry -> entry.getValue() == null &&
!localRoots.storageData.containsKey(entry.getKey()));
}
/**
@@ -798,7 +801,7 @@ public abstract class ConfigurationChanger implements
DynamicConfigurationChange
return configurationUpdateListener.onConfigurationUpdated(
null,
storageRoots.roots,
- storageRoots.data.changeId(),
+ storageRoots.changeId,
notificationCount
);
}
diff --git
a/modules/configuration/src/main/java/org/apache/ignite/internal/configuration/util/ConfigurationFlattener.java
b/modules/configuration/src/main/java/org/apache/ignite/internal/configuration/util/ConfigurationFlattener.java
index 72e26208e9b..ff92a8a6d18 100644
---
a/modules/configuration/src/main/java/org/apache/ignite/internal/configuration/util/ConfigurationFlattener.java
+++
b/modules/configuration/src/main/java/org/apache/ignite/internal/configuration/util/ConfigurationFlattener.java
@@ -26,11 +26,15 @@ import java.util.ArrayDeque;
import java.util.Deque;
import java.util.HashMap;
import java.util.Map;
+import java.util.NavigableMap;
import java.util.Objects;
+import java.util.SortedMap;
import java.util.UUID;
+import java.util.function.Supplier;
import org.apache.ignite.internal.configuration.SuperRoot;
import org.apache.ignite.internal.configuration.tree.InnerNode;
import org.apache.ignite.internal.configuration.tree.NamedListNode;
+import org.jetbrains.annotations.Nullable;
/** Utility class that has {@link
ConfigurationFlattener#createFlattenedUpdatesMap} method. */
public class ConfigurationFlattener {
@@ -44,7 +48,7 @@ public class ConfigurationFlattener {
public static Map<String, Serializable> createFlattenedUpdatesMap(
SuperRoot curRoot,
SuperRoot updates,
- Map<String, ? extends Serializable> storageData
+ NavigableMap<String, ? extends Serializable> storageData
) {
// Resulting map.
Map<String, Serializable> resMap = new HashMap<>();
@@ -96,7 +100,7 @@ public class ConfigurationFlattener {
private final Map<String, Serializable> resMap;
/** Map with values in the configuration storage. */
- private final Map<String, ? extends Serializable> storageData;
+ private final NavigableMap<String, ? extends Serializable> storageData;
/** Flag indicates that "old" and "new" trees are literally the same
at the moment. */
private boolean singleTreeTraversal;
@@ -116,33 +120,48 @@ public class ConfigurationFlattener {
FlattenerVisitor(
Deque<InnerNode> oldInnerNodesStack,
Map<String, Serializable> resMap,
- Map<String, ? extends Serializable> storageData
+ NavigableMap<String, ? extends Serializable> storageData
) {
this.oldInnerNodesStack = oldInnerNodesStack;
this.resMap = resMap;
this.storageData = storageData;
}
+ private void putToMap(boolean mustOverride, boolean delete,
Supplier<String> key, @Nullable Serializable newVal) {
+ if (mustOverride) {
+ // This branch does the unconditional update, because it is
known that the value must be updated.
+ resMap.put(key.get(), delete ? null : newVal);
+ } else {
+ String currentKey = key.get();
+
+ // Here the value must not be updated, but it could be
updated. This code corresponds to a scenario where node restarts on
+ // a new version of Ignite and the name of the configuration
key is changed to a new one. There's a separate piece of code
+ // that deletes all usages of old name. This particular code
makes sure that we re-write each value with their new keys.
+ if (!storageData.containsKey(currentKey)) {
+ resMap.put(currentKey, newVal);
+ }
+ }
+ }
+
/** {@inheritDoc} */
@Override
public Void doVisitLeafNode(Field field, String key, Serializable
newVal) {
- boolean isDeprecated = field.isAnnotationPresent(Deprecated.class);
-
// Read same value from old tree.
Serializable oldVal =
oldInnerNodesStack.element().traverseChild(key,
ConfigurationUtil.leafNodeVisitor(), true);
// Do not put duplicates into the resulting map.
- if (isDeprecated || singleTreeTraversal ||
!Objects.deepEquals(oldVal, newVal)) {
- boolean deletion = this.deletion || isDeprecated;
+ putToMap(singleTreeTraversal || !Objects.deepEquals(oldVal,
newVal), deletion, this::currentKey, newVal);
- resMap.put(currentKey(), deletion ? null : newVal);
- }
+ return null;
+ }
- processLegacyPaths(legacyKey -> {
- if (storageData.containsKey(legacyKey)) {
- resMap.put(legacyKey, null);
- }
- });
+ @Override
+ protected Object doVisitLegacyLeafNode(Field field, String key,
Serializable val, boolean isDeprecated) {
+ String currentKey = currentKey();
+
+ if (storageData.containsKey(currentKey)) {
+ resMap.put(currentKey, null);
+ }
return null;
}
@@ -150,8 +169,6 @@ public class ConfigurationFlattener {
/** {@inheritDoc} */
@Override
public Void doVisitInnerNode(Field field, String key, InnerNode
newNode) {
- boolean isDeprecated = field != null &&
field.isAnnotationPresent(Deprecated.class);
-
// Read same node from old tree.
InnerNode oldNode =
oldInnerNodesStack.element().traverseChild(key,
ConfigurationUtil.innerNodeVisitor(), true);
@@ -162,16 +179,14 @@ public class ConfigurationFlattener {
}
if (oldNode == null) {
- visitAsymmetricInnerNode(newNode, isDeprecated);
+ visitAsymmetricInnerNode(newNode, false);
} else if (oldNode.schemaType() != newNode.schemaType()) {
// At the moment, we do not separate the general fields from
the fields of
// specific instances of the polymorphic configuration, so we
will assume
// that all the fields have changed, perhaps we will fix this
later.
visitAsymmetricInnerNode(oldNode, true);
- visitAsymmetricInnerNode(newNode, isDeprecated);
- } else if (isDeprecated) {
- visitAsymmetricInnerNode(newNode, true);
+ visitAsymmetricInnerNode(newNode, false);
} else {
oldInnerNodesStack.push(oldNode);
@@ -183,11 +198,16 @@ public class ConfigurationFlattener {
return null;
}
+ @Override
+ protected Object doVisitLegacyInnerNode(Field field, String key,
InnerNode node, boolean isDeprecated) {
+ dropOutdatedData();
+
+ return null;
+ }
+
/** {@inheritDoc} */
@Override
public Void doVisitNamedListNode(Field field, String key,
NamedListNode<?> newNode) {
- boolean isDeprecated = field.isAnnotationPresent(Deprecated.class);
-
// Read same named list node from old tree.
NamedListNode<?> oldNode =
oldInnerNodesStack.element().traverseChild(key,
ConfigurationUtil.namedListNodeVisitor(), true);
@@ -204,7 +224,7 @@ public class ConfigurationFlattener {
String namedListFullKey = currentKey();
withTracking(field, newNodeInternalId.toString(), false,
false, () -> {
- InnerNode newNamedElement = isDeprecated ? null :
newNode.getInnerNode(newNodeKey);
+ InnerNode newNamedElement =
newNode.getInnerNode(newNodeKey);
String oldNodeKey =
oldNode.keyByInternalId(newNodeInternalId);
InnerNode oldNamedElement =
oldNode.getInnerNode(oldNodeKey);
@@ -217,14 +237,14 @@ public class ConfigurationFlattener {
if (newNamedElement == null) {
visitAsymmetricInnerNode(oldNamedElement, true);
} else if (oldNamedElement == null) {
- visitAsymmetricInnerNode(newNamedElement,
isDeprecated);
+ visitAsymmetricInnerNode(newNamedElement, false);
} else if (newNamedElement.schemaType() !=
oldNamedElement.schemaType()) {
// At the moment, we do not separate the general
fields from the fields of
// specific instances of the polymorphic
configuration, so we will assume
// that all the fields have changed, perhaps we will
fix this later.
visitAsymmetricInnerNode(oldNamedElement, true);
- visitAsymmetricInnerNode(newNamedElement,
isDeprecated);
+ visitAsymmetricInnerNode(newNamedElement, false);
} else {
oldInnerNodesStack.push(oldNamedElement);
@@ -237,20 +257,20 @@ public class ConfigurationFlattener {
Integer oldIdx = oldKeysToOrderIdxMap == null ? null :
oldKeysToOrderIdxMap.get(newNodeKey);
// We should "persist" changed indexes only.
- if (!Objects.equals(newIdx, oldIdx) || singleTreeTraversal
|| newNamedElement == null) {
- String orderKey = currentKey() +
NamedListNode.ORDER_IDX;
-
- resMap.put(orderKey, deletion || newNamedElement ==
null ? null : newIdx);
- }
+ putToMap(
+ !Objects.equals(newIdx, oldIdx) ||
singleTreeTraversal || newNamedElement == null,
+ deletion || newNamedElement == null,
+ () -> currentKey() + NamedListNode.ORDER_IDX,
+ newIdx
+ );
// If it's creation / deletion / rename.
- if (singleTreeTraversal || oldNamedElement == null ||
newNamedElement == null
- || !oldNodeKey.equals(newNodeKey)
- ) {
- String nameKey = currentKey() + NamedListNode.NAME;
-
- resMap.put(nameKey, deletion || newNamedElement ==
null ? null : newNodeKey);
- }
+ putToMap(
+ singleTreeTraversal || oldNamedElement == null ||
newNamedElement == null || !oldNodeKey.equals(newNodeKey),
+ deletion || newNamedElement == null,
+ () -> currentKey() + NamedListNode.NAME,
+ newNodeKey
+ );
if (singleTreeTraversal) {
if (deletion) {
@@ -276,6 +296,12 @@ public class ConfigurationFlattener {
}
}
+ // Don't use "putToMap" method here because it would be
too complicated due to all the conditions above.
+ String idKey = idKey(namedListFullKey, newNodeKey);
+ if (!storageData.containsKey(idKey)) {
+ resMap.put(idKey, newNodeInternalId);
+ }
+
return null;
});
}
@@ -283,10 +309,30 @@ public class ConfigurationFlattener {
return null;
}
+ @Override
+ protected Object doVisitLegacyNamedListNode(Field field, String key,
NamedListNode<?> node, boolean isDeprecated) {
+ dropOutdatedData();
+
+ return null;
+ }
+
+ private void dropOutdatedData() {
+ String currentKey = currentKey();
+ SortedMap<String, ? extends Serializable> tailMap =
storageData.tailMap(currentKey);
+
+ for (String storageKey : tailMap.keySet()) {
+ if (!storageKey.startsWith(currentKey)) {
+ break;
+ }
+
+ resMap.put(storageKey, null);
+ }
+ }
+
/**
* Creates key {@code prefix.<ids>.nodeKey}, escaping {@code nodeKey}
before appending it.
*/
- private String idKey(String prefix, String nodeKey) {
+ private static String idKey(String prefix, String nodeKey) {
return prefix + NamedListNode.IDS + KEY_SEPARATOR +
escape(nodeKey);
}
diff --git
a/modules/configuration/src/main/java/org/apache/ignite/internal/configuration/util/ConfigurationUtil.java
b/modules/configuration/src/main/java/org/apache/ignite/internal/configuration/util/ConfigurationUtil.java
index f2890cc5734..54fad1800de 100644
---
a/modules/configuration/src/main/java/org/apache/ignite/internal/configuration/util/ConfigurationUtil.java
+++
b/modules/configuration/src/main/java/org/apache/ignite/internal/configuration/util/ConfigurationUtil.java
@@ -64,8 +64,10 @@ import
org.apache.ignite.configuration.annotation.NamedConfigValue;
import org.apache.ignite.configuration.annotation.PolymorphicConfig;
import org.apache.ignite.configuration.annotation.PolymorphicConfigInstance;
import org.apache.ignite.configuration.annotation.PolymorphicId;
+import org.apache.ignite.configuration.annotation.PublicName;
import org.apache.ignite.configuration.annotation.Value;
import org.apache.ignite.internal.configuration.DynamicConfiguration;
+import org.apache.ignite.internal.configuration.SuperRoot;
import org.apache.ignite.internal.configuration.direct.KeyPathNode;
import org.apache.ignite.internal.configuration.storage.ConfigurationStorage;
import org.apache.ignite.internal.configuration.tree.ConfigurationSource;
@@ -940,6 +942,81 @@ public class ConfigurationUtil {
};
}
+ /**
+ * Removes {@code null} values that correspond to non-deprecated legacy
keys from the configuration tree.
+ *
+ * @param roots Super root.
+ * @param prefixMap Mutable prefix map with updates received from the
storage.
+ * @see PublicName#legacyNames()
+ */
+ public static void ignoreLegacyKeys(SuperRoot roots, Map<String, ?>
prefixMap) {
+ roots.traverseChildren(new KeysTrackingConfigurationVisitor<>() {
+ /** Map that correspond to current recursive call. */
+ private Map<String, ?> currentMap = prefixMap;
+
+ @Override
+ protected Object doVisitLegacyLeafNode(Field field, String key,
Serializable val, boolean isDeprecated) {
+ if (!isDeprecated) {
+ currentMap.remove(key);
+ }
+
+ return null;
+ }
+
+ @Override
+ protected Object doVisitInnerNode(Field field, String key,
InnerNode node) {
+ if (!currentMap.containsKey(key)) {
+ return null;
+ }
+
+ Map<String, ?> prev = currentMap;
+ currentMap = (Map<String, ?>) currentMap.get(key);
+
+ node.traverseChildren(this, true);
+
+ currentMap = prev;
+
+ return null;
+ }
+
+ @Override
+ protected Object doVisitLegacyInnerNode(Field field, String key,
InnerNode node, boolean isDeprecated) {
+ currentMap.remove(key);
+
+ return null;
+ }
+
+ @Override
+ protected Object doVisitNamedListNode(Field field, String key,
NamedListNode<?> node) {
+ if (!currentMap.containsKey(key)) {
+ return null;
+ }
+
+ Map<String, ?> prev = currentMap;
+ currentMap = (Map<String, ? extends Serializable>)
currentMap.get(key);
+
+ for (String namedListKey : node.namedListKeys()) {
+ withTracking(field,
node.internalId(namedListKey).toString(), false, false, () -> {
+ doVisitInnerNode(field, namedListKey,
node.getInnerNode(namedListKey));
+
+ return null;
+ });
+ }
+
+ currentMap = prev;
+
+ return null;
+ }
+
+ @Override
+ protected Object doVisitLegacyNamedListNode(Field field, String
key, NamedListNode<?> node, boolean isDeprecated) {
+ currentMap.remove(key);
+
+ return null;
+ }
+ }, true);
+ }
+
/**
* Leaf configuration source.
*/
diff --git
a/modules/configuration/src/main/java/org/apache/ignite/internal/configuration/util/KeysTrackingConfigurationVisitor.java
b/modules/configuration/src/main/java/org/apache/ignite/internal/configuration/util/KeysTrackingConfigurationVisitor.java
index 0a5405f3f35..cd36d4bf635 100644
---
a/modules/configuration/src/main/java/org/apache/ignite/internal/configuration/util/KeysTrackingConfigurationVisitor.java
+++
b/modules/configuration/src/main/java/org/apache/ignite/internal/configuration/util/KeysTrackingConfigurationVisitor.java
@@ -18,8 +18,6 @@
package org.apache.ignite.internal.configuration.util;
import static
org.apache.ignite.internal.configuration.asm.ConfigurationAsmGenerator.legacyNames;
-import static
org.apache.ignite.internal.configuration.util.ConfigurationUtil.appendKey;
-import static
org.apache.ignite.internal.configuration.util.ConfigurationUtil.join;
import static org.apache.ignite.internal.util.ArrayUtils.STRING_EMPTY_ARRAY;
import java.io.Serializable;
@@ -27,7 +25,6 @@ import java.lang.reflect.Field;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
-import java.util.function.Consumer;
import java.util.function.Supplier;
import org.apache.ignite.internal.configuration.tree.ConfigurationVisitor;
import org.apache.ignite.internal.configuration.tree.InnerNode;
@@ -41,19 +38,31 @@ public abstract class KeysTrackingConfigurationVisitor<T>
implements Configurati
/** Current keys list, almost the same as {@link #currentKey}. */
private final List<String> currentPath = new ArrayList<>();
- /** For every part of the current path stores corresponding legacy names.
*/
- private final List<String[]> currentLegacyNames = new ArrayList<>();
+ private static boolean isDeprecated(Field field) {
+ return field != null && field.getAnnotation(Deprecated.class) != null;
+ }
- /** Total amount of legacy names, corresponding to the current path. */
- private int currentLegacyNamesCount = 0;
+ private static String[] getLegacyNames(Field field) {
+ return field == null ? STRING_EMPTY_ARRAY : legacyNames(field);
+ }
/** {@inheritDoc} */
@Override
public final T visitLeafNode(Field field, String key, Serializable val) {
+ for (String legacyKey : getLegacyNames(field)) {
+ int prevPos = startVisit(field, legacyKey, false, true);
+
+ try {
+ doVisitLegacyLeafNode(field, legacyKey, val, false);
+ } finally {
+ endVisit(prevPos);
+ }
+ }
+
int prevPos = startVisit(field, key, false, true);
try {
- return doVisitLeafNode(field, key, val);
+ return isDeprecated(field) ? doVisitLegacyLeafNode(field, key,
val, true) : doVisitLeafNode(field, key, val);
} finally {
endVisit(prevPos);
}
@@ -62,10 +71,20 @@ public abstract class KeysTrackingConfigurationVisitor<T>
implements Configurati
/** {@inheritDoc} */
@Override
public final T visitInnerNode(Field field, String key, InnerNode node) {
+ for (String legacyKey : getLegacyNames(field)) {
+ int prevPos = startVisit(field, legacyKey, false, false);
+
+ try {
+ doVisitLegacyInnerNode(field, legacyKey, node, false);
+ } finally {
+ endVisit(prevPos);
+ }
+ }
+
int prevPos = startVisit(field, key, false, false);
try {
- return doVisitInnerNode(field, key, node);
+ return isDeprecated(field) ? doVisitLegacyInnerNode(field, key,
node, true) : doVisitInnerNode(field, key, node);
} finally {
endVisit(prevPos);
}
@@ -74,10 +93,20 @@ public abstract class KeysTrackingConfigurationVisitor<T>
implements Configurati
/** {@inheritDoc} */
@Override
public final T visitNamedListNode(Field field, String key,
NamedListNode<?> node) {
+ for (String legacyKey : getLegacyNames(field)) {
+ int prevPos = startVisit(field, legacyKey, false, false);
+
+ try {
+ doVisitLegacyNamedListNode(field, legacyKey, node, false);
+ } finally {
+ endVisit(prevPos);
+ }
+ }
+
int prevPos = startVisit(field, key, false, false);
try {
- return doVisitNamedListNode(field, key, node);
+ return isDeprecated(field) ? doVisitLegacyNamedListNode(field,
key, node, true) : doVisitNamedListNode(field, key, node);
} finally {
endVisit(prevPos);
}
@@ -94,6 +123,13 @@ public abstract class KeysTrackingConfigurationVisitor<T>
implements Configurati
return null;
}
+ /**
+ * Almost the same as {@link #doVisitLeafNode(Field, String,
Serializable)}, but used for legacy fields.
+ */
+ protected T doVisitLegacyLeafNode(Field field, String key, Serializable
val, boolean isDeprecated) {
+ return null;
+ }
+
/**
* To be used instead of {@link ConfigurationVisitor#visitInnerNode(Field,
String, InnerNode)}.
*
@@ -107,6 +143,13 @@ public abstract class KeysTrackingConfigurationVisitor<T>
implements Configurati
return null;
}
+ /**
+ * Almost the same as {@link #doVisitInnerNode(Field, String, InnerNode)},
but used for legacy fields.
+ */
+ protected T doVisitLegacyInnerNode(Field field, String key, InnerNode
node, boolean isDeprecated) {
+ return null;
+ }
+
/**
* To be used instead of {@link ConfigurationVisitor#visitNamedListNode}}.
*
@@ -128,6 +171,13 @@ public abstract class KeysTrackingConfigurationVisitor<T>
implements Configurati
return null;
}
+ /**
+ * Almost the same as {@link #doVisitNamedListNode(Field, String,
NamedListNode)}, but used for legacy fields.
+ */
+ protected T doVisitLegacyNamedListNode(Field field, String key,
NamedListNode<?> node, boolean isDeprecated) {
+ return null;
+ }
+
/**
* Tracks passed key to reflect it in {@link #currentKey()} and {@link
#currentPath()}.
*
@@ -185,11 +235,6 @@ public abstract class KeysTrackingConfigurationVisitor<T>
implements Configurati
currentPath.add(key);
- String[] legacyNames = field == null ? STRING_EMPTY_ARRAY :
legacyNames(field);
-
- currentLegacyNames.add(legacyNames);
- currentLegacyNamesCount += legacyNames.length;
-
return previousKeyLength;
}
@@ -202,55 +247,5 @@ public abstract class KeysTrackingConfigurationVisitor<T>
implements Configurati
currentKey.setLength(previousKeyLength);
currentPath.remove(currentPath.size() - 1);
-
- String[] legacyNames =
currentLegacyNames.remove(currentLegacyNames.size() - 1);
- currentLegacyNamesCount -= legacyNames.length;
- }
-
- /** Calls consumer for all variations of legacy paths, i.e. to remove them
from the storage. */
- protected void processLegacyPaths(Consumer<String> legacyKeyConsumer) {
- // Current path doesn't contain any legacy names.
- if (currentLegacyNamesCount == 0) {
- return;
- }
-
- processLegacyPaths(new ArrayList<>(), legacyKeyConsumer);
- }
-
- private void processLegacyPaths(List<String> path, Consumer<String>
legacyKeyConsumer) {
- // We reached the leaf. If path joined with leaf name != current key,
it is legacy and should be processed.
- if (path.size() == currentPath().size() - 1) {
- for (String leafName : currentLegacyNames.get(currentPath().size()
- 1)) {
- processLeaf(path, legacyKeyConsumer, leafName);
- }
-
- // Process current name for cases when legacy name was in the
middle of the path.
- processLeaf(path, legacyKeyConsumer,
currentPath().get(currentPath().size() - 1));
-
- return;
- }
-
- // For inner nodes we should all legacy names and current name.
- for (String innerNodeName : currentLegacyNames.get(path.size())) {
- path.add(innerNodeName);
-
- processLegacyPaths(path, legacyKeyConsumer);
-
- path.remove(path.size() - 1);
- }
-
- path.add(currentPath.get(path.size()));
-
- processLegacyPaths(path, legacyKeyConsumer);
-
- path.remove(path.size() - 1);
- }
-
- private void processLeaf(List<String> path, Consumer<String>
legacyKeyConsumer, String leafName) {
- String key = join(appendKey(path, leafName));
-
- if (!key.equals(currentKey())) {
- legacyKeyConsumer.accept(key);
- }
}
}
diff --git
a/modules/configuration/src/test/java/org/apache/ignite/internal/configuration/RenamedConfigurationTest.java
b/modules/configuration/src/test/java/org/apache/ignite/internal/configuration/RenamedConfigurationTest.java
index f19758b62b2..7a8542f4d1f 100644
---
a/modules/configuration/src/test/java/org/apache/ignite/internal/configuration/RenamedConfigurationTest.java
+++
b/modules/configuration/src/test/java/org/apache/ignite/internal/configuration/RenamedConfigurationTest.java
@@ -17,6 +17,7 @@
package org.apache.ignite.internal.configuration;
+import static java.util.Map.entry;
import static
org.apache.ignite.configuration.annotation.ConfigurationType.LOCAL;
import static
org.apache.ignite.internal.configuration.hocon.HoconConverter.hoconSource;
import static
org.apache.ignite.internal.testframework.matchers.CompletableFutureMatcher.willBe;
@@ -24,10 +25,18 @@ import static
org.apache.ignite.internal.testframework.matchers.CompletableFutur
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.nullValue;
+import static org.junit.jupiter.api.Assertions.assertEquals;
import com.typesafe.config.ConfigFactory;
+import java.io.Serializable;
+import java.util.Map;
+import java.util.Map.Entry;
import java.util.Set;
+import java.util.UUID;
import java.util.concurrent.CompletableFuture;
+import java.util.function.Function;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
import org.apache.ignite.configuration.RootKey;
import org.apache.ignite.configuration.annotation.Config;
import org.apache.ignite.configuration.annotation.ConfigValue;
@@ -41,13 +50,13 @@ import
org.apache.ignite.configuration.annotation.PublicName;
import org.apache.ignite.configuration.annotation.Value;
import org.apache.ignite.internal.configuration.storage.Data;
import
org.apache.ignite.internal.configuration.storage.TestConfigurationStorage;
+import org.apache.ignite.internal.configuration.util.ConfigurationUtil;
import
org.apache.ignite.internal.configuration.validation.TestConfigurationValidator;
import org.apache.ignite.internal.manager.ComponentContext;
import org.apache.ignite.internal.testframework.BaseIgniteAbstractTest;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
-import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
class RenamedConfigurationTest extends BaseIgniteAbstractTest {
@@ -156,14 +165,26 @@ class RenamedConfigurationTest extends
BaseIgniteAbstractTest {
}
@Test
- @Disabled("IGNITE-25458")
public void testNamedListLegacyNameIsRecognisedOnStartup() {
assertThat(
registry.getConfiguration(RenamedTestNewConfiguration.KEY).newListName().get("listInstance").newName().value(),
equalTo("oldValue")
);
- // TODO IGNITE-25458 Add check that old value is deleted
+ validateStorageContent(
+ entry("key.newInnerName.name", "oldDefault"),
+ entry("key.newInnerName.newName", "oldDefault"),
+ entry("key.newListName.$listId.<name>", "listInstance"),
+ entry("key.newListName.$listId.<order>", 0),
+ entry("key.newListName.$listId.name", "oldDefault"),
+ entry("key.newListName.$listId.newName", "oldValue"),
+ entry("key.newListName.<ids>.listInstance", "$listId"),
+ entry("key.newPolymorphicName.<ids>.polymorphicType",
"$polymorphicId"),
+ entry("key.newPolymorphicName.$polymorphicId.<name>",
"polymorphicType"),
+ entry("key.newPolymorphicName.$polymorphicId.<order>", 0),
+ entry("key.newPolymorphicName.$polymorphicId.newName",
"oldValue"),
+ entry("key.newPolymorphicName.$polymorphicId.type",
"polymorphicType")
+ );
}
@Test
@@ -178,18 +199,43 @@ class RenamedConfigurationTest extends
BaseIgniteAbstractTest {
equalTo(newValue)
);
- // TODO IGNITE-25458 Add check that old value is deleted
+ validateStorageContent(
+ entry("key.newInnerName.name", "oldDefault"),
+ entry("key.newInnerName.newName", "oldDefault"),
+ entry("key.newListName.$listId.<name>", "listInstance"),
+ entry("key.newListName.$listId.<order>", 0),
+ entry("key.newListName.$listId.name", "oldDefault"),
+ entry("key.newListName.$listId.newName", newValue),
+ entry("key.newListName.<ids>.listInstance", "$listId"),
+ entry("key.newPolymorphicName.<ids>.polymorphicType",
"$polymorphicId"),
+ entry("key.newPolymorphicName.$polymorphicId.<name>",
"polymorphicType"),
+ entry("key.newPolymorphicName.$polymorphicId.<order>", 0),
+ entry("key.newPolymorphicName.$polymorphicId.newName",
"oldValue"),
+ entry("key.newPolymorphicName.$polymorphicId.type",
"polymorphicType")
+ );
}
@Test
- @Disabled("IGNITE-25458")
public void testPolymorphicLegacyNameIsRecognisedOnStartup() {
assertThat(
registry.getConfiguration(RenamedTestNewConfiguration.KEY).newPolymorphicName().get("polymorphicType").newName().value(),
equalTo("oldValue")
);
- // TODO IGNITE-25458 Add check that old value is deleted
+ validateStorageContent(
+ entry("key.newInnerName.name", "oldDefault"),
+ entry("key.newInnerName.newName", "oldDefault"),
+ entry("key.newListName.$listId.<name>", "listInstance"),
+ entry("key.newListName.$listId.<order>", 0),
+ entry("key.newListName.$listId.name", "oldDefault"),
+ entry("key.newListName.$listId.newName", "oldValue"),
+ entry("key.newListName.<ids>.listInstance", "$listId"),
+ entry("key.newPolymorphicName.<ids>.polymorphicType",
"$polymorphicId"),
+ entry("key.newPolymorphicName.$polymorphicId.<name>",
"polymorphicType"),
+ entry("key.newPolymorphicName.$polymorphicId.<order>", 0),
+ entry("key.newPolymorphicName.$polymorphicId.newName",
"oldValue"),
+ entry("key.newPolymorphicName.$polymorphicId.type",
"polymorphicType")
+ );
}
@Test
@@ -203,7 +249,53 @@ class RenamedConfigurationTest extends
BaseIgniteAbstractTest {
equalTo(newValue)
);
- // TODO IGNITE-25458 Add check that old value is deleted
+ validateStorageContent(
+ entry("key.newInnerName.name", "oldDefault"),
+ entry("key.newInnerName.newName", "oldDefault"),
+ entry("key.newListName.$listId.<name>", "listInstance"),
+ entry("key.newListName.$listId.<order>", 0),
+ entry("key.newListName.$listId.name", "oldDefault"),
+ entry("key.newListName.$listId.newName", "oldValue"),
+ entry("key.newListName.<ids>.listInstance", "$listId"),
+ entry("key.newPolymorphicName.<ids>.polymorphicType",
"$polymorphicId"),
+ entry("key.newPolymorphicName.$polymorphicId.<name>",
"polymorphicType"),
+ entry("key.newPolymorphicName.$polymorphicId.<order>", 0),
+ entry("key.newPolymorphicName.$polymorphicId.newName",
newValue),
+ entry("key.newPolymorphicName.$polymorphicId.type",
"polymorphicType")
+ );
+ }
+
+ /**
+ * Validates that the storage content matches the expected values. Because
storage contains several "dynamic" values, whose IDs are not
+ * constants, we denote them as strings {@code "$listId"} and {@code
"$polymorphicId"}.All occasions of these strings will be replaced
+ * with real values of these identifiers.
+ */
+ @SafeVarargs
+ private void validateStorageContent(Map.Entry<String, Serializable>
...values) {
+ CompletableFuture<Data> dataFuture = storage.readDataOnRecovery();
+ assertThat(dataFuture, willCompleteSuccessfully());
+
+ RenamedTestNewView node =
registry.getConfiguration(RenamedTestNewConfiguration.KEY).value();
+
+ UUID listId = ConfigurationUtil.internalId(node.newListName(),
"listInstance");
+ UUID polymorphicId =
ConfigurationUtil.internalId(node.newPolymorphicName(), "polymorphicType");
+
+ Function<Entry<String, ? extends Serializable>, String> keyMapper = e
->
+ e.getKey().replace("$listId",
listId.toString()).replace("$polymorphicId", polymorphicId.toString());
+
+ Function<Entry<String, ? extends Serializable>, Serializable>
valueMapper = e -> {
+ if ("$listId".equals(e.getValue())) {
+ return listId;
+ } else if ("$polymorphicId".equals(e.getValue())) {
+ return polymorphicId;
+ } else {
+ return e.getValue();
+ }
+ };
+
+ Map<String, ? extends Serializable> expectedMap =
Stream.of(values).collect(Collectors.toMap(keyMapper, valueMapper));
+
+ assertEquals(expectedMap, dataFuture.join().values());
}
private static void updateConfig(ConfigurationRegistry registry, String
updatedConfig) {
diff --git
a/modules/configuration/src/test/java/org/apache/ignite/internal/configuration/util/ConfigurationUtilTest.java
b/modules/configuration/src/test/java/org/apache/ignite/internal/configuration/util/ConfigurationUtilTest.java
index 18fb0324b4a..2d2528ff6d7 100644
---
a/modules/configuration/src/test/java/org/apache/ignite/internal/configuration/util/ConfigurationUtilTest.java
+++
b/modules/configuration/src/test/java/org/apache/ignite/internal/configuration/util/ConfigurationUtilTest.java
@@ -18,6 +18,7 @@
package org.apache.ignite.internal.configuration.util;
import static java.util.Collections.emptyList;
+import static java.util.Collections.emptyNavigableMap;
import static java.util.Collections.singletonMap;
import static java.util.function.Function.identity;
import static java.util.stream.Collectors.toList;
@@ -61,9 +62,11 @@ import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
+import java.util.NavigableMap;
import java.util.NoSuchElementException;
import java.util.Set;
import java.util.Spliterators;
+import java.util.TreeMap;
import java.util.UUID;
import java.util.function.Consumer;
import java.util.stream.StreamSupport;
@@ -465,17 +468,19 @@ public class ConfigurationUtilTest {
Map.of(ParentConfiguration.KEY,
newNodeInstance(ParentConfigurationSchema.class))
);
- assertThat(flattenedMap(superRoot, ParentConfiguration.KEY, node -> {
- }), is(anEmptyMap()));
+ assertThat(flattenedMap(superRoot, ParentConfiguration.KEY,
emptyNavigableMap(), node -> {}), is(anEmptyMap()));
- assertThat(
- flattenedMap(superRoot, ParentConfiguration.KEY, node ->
((ParentChange) node)
+ Map<String, Serializable> flattenedMap =
+ flattenedMap(superRoot, ParentConfiguration.KEY,
emptyNavigableMap(), node -> ((ParentChange) node)
.changeElements(elements -> elements
.create("name", element -> element
.changeChild(child ->
child.changeStr("foo"))
)
)
- ),
+ );
+
+ assertThat(
+ flattenedMap,
is(allOf(
aMapWithSize(4),
hasEntry(matchesPattern("root[.]elements[.]<ids>[.]name"),
hasToString(matchesPattern("[-\\w]{36}"))),
@@ -486,14 +491,14 @@ public class ConfigurationUtilTest {
);
assertThat(
- flattenedMap(superRoot, ParentConfiguration.KEY, node ->
((ParentChange) node)
+ flattenedMap(superRoot, ParentConfiguration.KEY, new
TreeMap<>(flattenedMap), node -> ((ParentChange) node)
.changeElements(elements1 -> elements1.delete("void"))
),
is(anEmptyMap())
);
assertThat(
- flattenedMap(superRoot, ParentConfiguration.KEY, node ->
((ParentChange) node)
+ flattenedMap(superRoot, ParentConfiguration.KEY, new
TreeMap<>(flattenedMap), node -> ((ParentChange) node)
.changeElements(elements -> elements.delete("name"))
),
is(allOf(
@@ -1046,6 +1051,7 @@ public class ConfigurationUtilTest {
final Map<String, Serializable> act = flattenedMap(
superRoot,
rootKey,
+ emptyNavigableMap(),
node -> ((PolymorphicRootChange)
node).changePolymorphicSubCfg(c ->
c.convert(SecondPolymorphicInstanceChange.class))
);
@@ -1077,9 +1083,12 @@ public class ConfigurationUtilTest {
SuperRoot superRoot = new SuperRoot(key -> null, Map.of(rootKey,
polymorphicRootInnerNode));
- final Map<String, Serializable> act = flattenedMap(
+ var storageData = new TreeMap<>(flattenedMap(superRoot, rootKey,
emptyNavigableMap(), node -> {}));
+
+ Map<String, Serializable> act = flattenedMap(
superRoot,
rootKey,
+ storageData,
node -> ((PolymorphicRootChange)
node).changePolymorphicNamedCfg(c ->
c.createOrUpdate("0", c1 ->
c1.convert(SecondPolymorphicInstanceChange.class)))
);
@@ -1174,12 +1183,14 @@ public class ConfigurationUtilTest {
* method execution is completed.
*
* @param superRoot Super root to patch.
- * @param patch Closure to change inner node.
+ * @param storageData Full storage data.
+ * @param patch Closure to change inner node.
* @return Flat map with all changes from the patch.
*/
- private Map<String, Serializable> flattenedMap(
+ private static Map<String, Serializable> flattenedMap(
SuperRoot superRoot,
RootKey<?, ?> rootKey,
+ NavigableMap<String, ? extends Serializable> storageData,
Consumer<InnerNode> patch
) {
// Preserve a copy of the super root to use it as a golden source of
data.
@@ -1192,7 +1203,7 @@ public class ConfigurationUtilTest {
patch.accept(superRoot.getRoot(rootKey));
// Create flat diff between two super trees.
- return createFlattenedUpdatesMap(originalSuperRoot, superRoot,
Map.of());
+ return createFlattenedUpdatesMap(originalSuperRoot, superRoot,
storageData);
}
/**
diff --git
a/modules/runner/src/main/java/org/apache/ignite/internal/configuration/storage/LocalFileConfigurationStorage.java
b/modules/runner/src/main/java/org/apache/ignite/internal/configuration/storage/LocalFileConfigurationStorage.java
index 120f63883b0..4e6c8a0748d 100644
---
a/modules/runner/src/main/java/org/apache/ignite/internal/configuration/storage/LocalFileConfigurationStorage.java
+++
b/modules/runner/src/main/java/org/apache/ignite/internal/configuration/storage/LocalFileConfigurationStorage.java
@@ -17,6 +17,7 @@
package org.apache.ignite.internal.configuration.storage;
+import static java.util.Collections.emptyNavigableMap;
import static java.util.stream.Collectors.toMap;
import static
org.apache.ignite.internal.configuration.util.ConfigurationFlattener.createFlattenedUpdatesMap;
import static
org.apache.ignite.internal.configuration.util.ConfigurationUtil.fillFromPrefixMap;
@@ -175,7 +176,7 @@ public class LocalFileConfigurationStorage implements
ConfigurationStorage {
Config hocon = readHoconFromFile();
HoconConverter.hoconSource(hocon.root(),
keyIgnorer).descend(copiedSuperRoot);
- Map<String, Serializable> flattenedUpdatesMap =
createFlattenedUpdatesMap(superRoot, copiedSuperRoot, Map.of());
+ Map<String, Serializable> flattenedUpdatesMap =
createFlattenedUpdatesMap(superRoot, copiedSuperRoot, emptyNavigableMap());
flattenedUpdatesMap.forEach((key, value) -> {
if (value != null) { // Filter defaults.
latest.put(key, value);
