You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by GitBox <gi...@apache.org> on 2022/08/19 11:15:31 UTC
[GitHub] [ignite-3] sashapolo commented on a diff in pull request #1023: IGNITE-17555 Fix AssertionError and NPE in configuration
sashapolo commented on code in PR #1023:
URL: https://github.com/apache/ignite-3/pull/1023#discussion_r949968986
##########
modules/configuration/src/main/java/org/apache/ignite/internal/configuration/notifications/ConfigurationNotifier.java:
##########
@@ -425,9 +425,13 @@ public Void visitNamedListNode(String key, NamedListNode<?> newNamedList) {
// Lazy initialization.
Collection<DynamicConfiguration<InnerNode, ?>> newAnyConfigs = null;
+ NamedListConfiguration<?, InnerNode, ?> namedListCfg = namedDynamicConfig(config, key);
+
+ namedListCfg.touchMembers();
Review Comment:
Please add a comment about why we need to "touch members"
##########
modules/configuration/src/main/java/org/apache/ignite/internal/configuration/storage/ConfigurationStorage.java:
##########
@@ -27,11 +27,11 @@
*/
public interface ConfigurationStorage extends AutoCloseable {
/**
- * Read all configuration values and current storage version.
+ * Reads all configuration values and current storage version on the start.
Review Comment:
I don't understand why the "start" part is so important. What will happen if I call this method not on node start?
##########
modules/metastorage-client/src/main/java/org/apache/ignite/internal/metastorage/client/EntryImpl.java:
##########
@@ -48,7 +48,7 @@ public final class EntryImpl implements Entry {
* @param rev Revision.
* @param updCntr Update counter.
*/
- EntryImpl(@NotNull ByteArray key, @Nullable byte[] val, long rev, long updCntr) {
+ public EntryImpl(@NotNull ByteArray key, @Nullable byte[] val, long rev, long updCntr) {
Review Comment:
Not related to this PR, but can you please fix `Nullable` usages in this class?
##########
modules/configuration/src/main/java/org/apache/ignite/internal/configuration/ConfigurationChanger.java:
##########
@@ -572,20 +572,23 @@ private CompletableFuture<Void> updateFromListener(Data changedEntries) {
storageRoots = new StorageRoots(newSuperRoot, newChangeId);
- if (dataValuesPrefixMap.isEmpty()) {
- oldStorageRoots.changeFuture.complete(null);
-
- return CompletableFuture.completedFuture(null);
- } else {
- return notificator.notify(oldSuperRoot, newSuperRoot, newChangeId, notificationListenerCnt.incrementAndGet())
- .whenComplete((v, t) -> {
- if (t == null) {
- oldStorageRoots.changeFuture.complete(null);
- } else {
- oldStorageRoots.changeFuture.completeExceptionally(t);
- }
- });
- }
+ // Save revisions for recovery.
+ return storage.writeConfigurationRevision(oldStorageRoots.version, storageRoots.version).thenCompose(unused -> {
Review Comment:
Can you move the `thenCompose` call to the next line? I think it makes the code more readable
##########
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:
Review Comment:
Why did you remove this comment?
##########
modules/core/src/main/java/org/apache/ignite/internal/util/ByteUtils.java:
##########
@@ -86,7 +86,7 @@ public static byte[] longToBytes(long l) {
* @param limit Limit of bytes to write into output.
* @return Number of bytes overwritten in {@code bytes} array.
*/
- private static byte[] longToBytes(long l, byte[] bytes, int off, int limit) {
+ public static byte[] longToBytes(long l, byte[] bytes, int off, int limit) {
Review Comment:
I think that the `limit` parameter makes little sense. It's always equal to 8 anyway
##########
modules/runner/src/test/java/org/apache/ignite/internal/configuration/storage/DistributedConfigurationStorageTest.java:
##########
@@ -74,6 +93,145 @@ void stop() throws Exception {
vaultManager.stop();
}
+ /**
+ * Dummy configuration.
+ */
+ @ConfigurationRoot(rootName = "someKey", type = DISTRIBUTED)
+ public static class DistributedTestConfigurationSchema {
+ @Value(hasDefault = true)
+ public final int foobar = 0;
+ }
+
+ /**
+ * Tests that distributed configuration storage correctly picks up latest configuration MetaStorage revision
+ * during recovery process.
+ *
+ * @throws Exception If failed.
+ */
+ @Test
+ public void testMetaStorageRevisionDifferentFromConfigurationOnRestart() throws Exception {
+ RootKey<DistributedTestConfiguration, DistributedTestView> rootKey = DistributedTestConfiguration.KEY;
+
+ ConfigurationAsmGenerator cgen = new ConfigurationAsmGenerator();
+
+ MetaStorageMockWrapper wrapper = new MetaStorageMockWrapper();
+
+ int configurationChangesCount = 7;
+
+ try (var storage = new DistributedConfigurationStorage(wrapper.metaStorageManager(), vaultManager)) {
+ var changer = new TestConfigurationChanger(cgen, List.of(rootKey), Collections.emptyMap(),
+ storage, Collections.emptyList(), Collections.emptyList());
+
+ changer.start();
+
+ for (int i = 0; i < configurationChangesCount; i++) {
+ ConfigurationSource source = source(
+ rootKey,
+ (DistributedTestChange change) -> change.changeFoobar(1)
+ );
+
+ CompletableFuture<Void> change = changer.change(source);
+
+ change.get();
+ }
+
+ changer.stop();
+ }
+
+ // Put a value to the configuration, so we start on non-empty vault.
+ vaultManager.put(MetaStorageMockWrapper.TEST_KEY, new byte[]{4, 1, 2, 3, 4}).get();
+
+ // This emulates a change in MetaStorage that is not related to the configuration.
+ vaultManager.put(MetaStorageManager.APPLIED_REV, ByteUtils.longToBytes(configurationChangesCount + 1)).get();
+
+ try (var storage = new DistributedConfigurationStorage(wrapper.metaStorageManager(), vaultManager)) {
+ var changer = new TestConfigurationChanger(cgen, List.of(rootKey), Collections.emptyMap(),
+ storage, Collections.emptyList(), Collections.emptyList());
+
+ changer.start();
+
+ CompletableFuture<Long> longCompletableFuture = storage.lastRevision();
+
+ // Should return last configuration change, not last MetaStorage change.
+ Long lastConfigurationChangeRevision = longCompletableFuture.get();
+
+ assertEquals(configurationChangesCount, lastConfigurationChangeRevision);
+
+ changer.stop();
+ }
+ }
+
+ /**
+ * This class stores data for {@link MetaStorageManager}'s mock.
+ */
+ private static class MetaStorageMockWrapper {
+ private static final String DISTRIBUTED_PREFIX = "dst-cfg.";
+
+ /**
+ * This and previous field are copy-pasted intentionally, so in case if something changes,
+ * this test should fail and be reviewed and re-wrote.
Review Comment:
```suggestion
* this test should fail and be reviewed and re-written.
```
##########
modules/runner/src/main/java/org/apache/ignite/internal/configuration/storage/DistributedConfigurationStorage.java:
##########
@@ -185,38 +190,52 @@ public CompletableFuture<Serializable> readLatest(String key) {
/** {@inheritDoc} */
@Override
- public CompletableFuture<Data> readAll() throws StorageException {
+ public CompletableFuture<Data> readAllOnStart() throws StorageException {
return registerFuture(vaultMgr.get(MetaStorageManager.APPLIED_REV)
- .thenApplyAsync(appliedRevEntry -> {
- long appliedRevision = appliedRevEntry == null ? 0L : ByteUtils.bytesToLong(appliedRevEntry.value());
+ .thenComposeAsync(appliedRevEntry -> {
Review Comment:
I have several comments here:
1. `thenComposeAsync` is not needed here, since the lambda inside is already an async operation, `thenCompose` should be enough.
2. `thenCompose` enforces serialized reads from the vault, which we don't need. I think that `thenCombine` method is more effective here.
3. The big lambda inside should be extracted into a separate method (e.g. `readAll`).
Taking this into account, we can refactor the method using a following approach:
```
CompletableFuture<Data> future = vaultMgr.get(MetaStorageManager.APPLIED_REV)
.thenCombineAsync(vaultMgr.get(CONFIGURATION_REVISIONS_KEY), this::readAll, threadPool);
return registerFuture(future);
```
##########
modules/runner/src/test/java/org/apache/ignite/internal/configuration/storage/DistributedConfigurationStorageTest.java:
##########
@@ -74,6 +93,145 @@ void stop() throws Exception {
vaultManager.stop();
}
+ /**
+ * Dummy configuration.
+ */
+ @ConfigurationRoot(rootName = "someKey", type = DISTRIBUTED)
+ public static class DistributedTestConfigurationSchema {
+ @Value(hasDefault = true)
+ public final int foobar = 0;
+ }
+
+ /**
+ * Tests that distributed configuration storage correctly picks up latest configuration MetaStorage revision
+ * during recovery process.
+ *
+ * @throws Exception If failed.
+ */
+ @Test
+ public void testMetaStorageRevisionDifferentFromConfigurationOnRestart() throws Exception {
+ RootKey<DistributedTestConfiguration, DistributedTestView> rootKey = DistributedTestConfiguration.KEY;
+
+ ConfigurationAsmGenerator cgen = new ConfigurationAsmGenerator();
+
+ MetaStorageMockWrapper wrapper = new MetaStorageMockWrapper();
+
+ int configurationChangesCount = 7;
+
+ try (var storage = new DistributedConfigurationStorage(wrapper.metaStorageManager(), vaultManager)) {
+ var changer = new TestConfigurationChanger(cgen, List.of(rootKey), Collections.emptyMap(),
+ storage, Collections.emptyList(), Collections.emptyList());
+
+ changer.start();
+
+ for (int i = 0; i < configurationChangesCount; i++) {
+ ConfigurationSource source = source(
+ rootKey,
+ (DistributedTestChange change) -> change.changeFoobar(1)
+ );
+
+ CompletableFuture<Void> change = changer.change(source);
+
+ change.get();
+ }
+
+ changer.stop();
+ }
+
+ // Put a value to the configuration, so we start on non-empty vault.
+ vaultManager.put(MetaStorageMockWrapper.TEST_KEY, new byte[]{4, 1, 2, 3, 4}).get();
+
+ // This emulates a change in MetaStorage that is not related to the configuration.
+ vaultManager.put(MetaStorageManager.APPLIED_REV, ByteUtils.longToBytes(configurationChangesCount + 1)).get();
+
+ try (var storage = new DistributedConfigurationStorage(wrapper.metaStorageManager(), vaultManager)) {
+ var changer = new TestConfigurationChanger(cgen, List.of(rootKey), Collections.emptyMap(),
+ storage, Collections.emptyList(), Collections.emptyList());
+
+ changer.start();
+
+ CompletableFuture<Long> longCompletableFuture = storage.lastRevision();
+
+ // Should return last configuration change, not last MetaStorage change.
+ Long lastConfigurationChangeRevision = longCompletableFuture.get();
+
+ assertEquals(configurationChangesCount, lastConfigurationChangeRevision);
+
+ changer.stop();
+ }
+ }
+
+ /**
+ * This class stores data for {@link MetaStorageManager}'s mock.
+ */
+ private static class MetaStorageMockWrapper {
Review Comment:
we now have two mock metastorage managers here. I think this test should be extracted into a separate class
##########
modules/runner/src/test/java/org/apache/ignite/internal/configuration/storage/DistributedConfigurationStorageTest.java:
##########
@@ -74,6 +93,145 @@ void stop() throws Exception {
vaultManager.stop();
}
+ /**
+ * Dummy configuration.
+ */
+ @ConfigurationRoot(rootName = "someKey", type = DISTRIBUTED)
+ public static class DistributedTestConfigurationSchema {
+ @Value(hasDefault = true)
+ public final int foobar = 0;
+ }
+
+ /**
+ * Tests that distributed configuration storage correctly picks up latest configuration MetaStorage revision
+ * during recovery process.
+ *
+ * @throws Exception If failed.
+ */
+ @Test
+ public void testMetaStorageRevisionDifferentFromConfigurationOnRestart() throws Exception {
+ RootKey<DistributedTestConfiguration, DistributedTestView> rootKey = DistributedTestConfiguration.KEY;
+
+ ConfigurationAsmGenerator cgen = new ConfigurationAsmGenerator();
+
+ MetaStorageMockWrapper wrapper = new MetaStorageMockWrapper();
+
+ int configurationChangesCount = 7;
+
+ try (var storage = new DistributedConfigurationStorage(wrapper.metaStorageManager(), vaultManager)) {
+ var changer = new TestConfigurationChanger(cgen, List.of(rootKey), Collections.emptyMap(),
+ storage, Collections.emptyList(), Collections.emptyList());
+
+ changer.start();
+
+ for (int i = 0; i < configurationChangesCount; i++) {
+ ConfigurationSource source = source(
+ rootKey,
+ (DistributedTestChange change) -> change.changeFoobar(1)
+ );
+
+ CompletableFuture<Void> change = changer.change(source);
+
+ change.get();
+ }
+
+ changer.stop();
+ }
+
+ // Put a value to the configuration, so we start on non-empty vault.
+ vaultManager.put(MetaStorageMockWrapper.TEST_KEY, new byte[]{4, 1, 2, 3, 4}).get();
+
+ // This emulates a change in MetaStorage that is not related to the configuration.
+ vaultManager.put(MetaStorageManager.APPLIED_REV, ByteUtils.longToBytes(configurationChangesCount + 1)).get();
+
+ try (var storage = new DistributedConfigurationStorage(wrapper.metaStorageManager(), vaultManager)) {
+ var changer = new TestConfigurationChanger(cgen, List.of(rootKey), Collections.emptyMap(),
+ storage, Collections.emptyList(), Collections.emptyList());
+
+ changer.start();
+
+ CompletableFuture<Long> longCompletableFuture = storage.lastRevision();
+
+ // Should return last configuration change, not last MetaStorage change.
+ Long lastConfigurationChangeRevision = longCompletableFuture.get();
+
+ assertEquals(configurationChangesCount, lastConfigurationChangeRevision);
+
+ changer.stop();
+ }
+ }
+
+ /**
+ * This class stores data for {@link MetaStorageManager}'s mock.
+ */
+ private static class MetaStorageMockWrapper {
+ private static final String DISTRIBUTED_PREFIX = "dst-cfg.";
+
+ /**
+ * This and previous field are copy-pasted intentionally, so in case if something changes,
+ * this test should fail and be reviewed and re-wrote.
+ */
+ private static final ByteArray MASTER_KEY = new ByteArray(DISTRIBUTED_PREFIX + "$master$key");
+
+ private static final ByteArray TEST_KEY = new ByteArray(DISTRIBUTED_PREFIX + "someKey.foobar");
+
+ /** MetaStorage mock. */
+ private final MetaStorageManager mock;
+
+ /** Captured MetaStorage listener. */
+ private WatchListener lsnr;
+
+ /** Current master key revision. */
+ private long masterKeyRevision;
+
+ private MetaStorageMockWrapper() {
+ mock = mock(MetaStorageManager.class);
+
+ setup();
+ }
+
+ private void setup() {
+ // Returns current master key revision.
+ when(mock.get(MASTER_KEY)).then(invocation -> {
+ return CompletableFuture.completedFuture(new EntryImpl(MASTER_KEY, null, masterKeyRevision, -1));
+ });
+
+ // On any invocation - trigger storage listener.
+ when(mock.invoke(any(), Mockito.<Collection<Operation>>any(), Mockito.any()))
Review Comment:
can be `when(mock.invoke(any(), anyCollection(), any()))`
##########
modules/runner/src/test/java/org/apache/ignite/internal/configuration/storage/DistributedConfigurationStorageTest.java:
##########
@@ -74,6 +93,145 @@ void stop() throws Exception {
vaultManager.stop();
}
+ /**
+ * Dummy configuration.
+ */
+ @ConfigurationRoot(rootName = "someKey", type = DISTRIBUTED)
+ public static class DistributedTestConfigurationSchema {
+ @Value(hasDefault = true)
+ public final int foobar = 0;
+ }
+
+ /**
+ * Tests that distributed configuration storage correctly picks up latest configuration MetaStorage revision
+ * during recovery process.
+ *
+ * @throws Exception If failed.
+ */
+ @Test
+ public void testMetaStorageRevisionDifferentFromConfigurationOnRestart() throws Exception {
+ RootKey<DistributedTestConfiguration, DistributedTestView> rootKey = DistributedTestConfiguration.KEY;
+
+ ConfigurationAsmGenerator cgen = new ConfigurationAsmGenerator();
+
+ MetaStorageMockWrapper wrapper = new MetaStorageMockWrapper();
+
+ int configurationChangesCount = 7;
+
+ try (var storage = new DistributedConfigurationStorage(wrapper.metaStorageManager(), vaultManager)) {
+ var changer = new TestConfigurationChanger(cgen, List.of(rootKey), Collections.emptyMap(),
+ storage, Collections.emptyList(), Collections.emptyList());
+
+ changer.start();
+
+ for (int i = 0; i < configurationChangesCount; i++) {
+ ConfigurationSource source = source(
+ rootKey,
+ (DistributedTestChange change) -> change.changeFoobar(1)
+ );
+
+ CompletableFuture<Void> change = changer.change(source);
+
+ change.get();
+ }
+
+ changer.stop();
+ }
+
+ // Put a value to the configuration, so we start on non-empty vault.
+ vaultManager.put(MetaStorageMockWrapper.TEST_KEY, new byte[]{4, 1, 2, 3, 4}).get();
+
+ // This emulates a change in MetaStorage that is not related to the configuration.
+ vaultManager.put(MetaStorageManager.APPLIED_REV, ByteUtils.longToBytes(configurationChangesCount + 1)).get();
+
+ try (var storage = new DistributedConfigurationStorage(wrapper.metaStorageManager(), vaultManager)) {
+ var changer = new TestConfigurationChanger(cgen, List.of(rootKey), Collections.emptyMap(),
+ storage, Collections.emptyList(), Collections.emptyList());
+
+ changer.start();
+
+ CompletableFuture<Long> longCompletableFuture = storage.lastRevision();
+
+ // Should return last configuration change, not last MetaStorage change.
+ Long lastConfigurationChangeRevision = longCompletableFuture.get();
+
+ assertEquals(configurationChangesCount, lastConfigurationChangeRevision);
+
+ changer.stop();
+ }
+ }
+
+ /**
+ * This class stores data for {@link MetaStorageManager}'s mock.
+ */
+ private static class MetaStorageMockWrapper {
+ private static final String DISTRIBUTED_PREFIX = "dst-cfg.";
+
+ /**
+ * This and previous field are copy-pasted intentionally, so in case if something changes,
+ * this test should fail and be reviewed and re-wrote.
+ */
+ private static final ByteArray MASTER_KEY = new ByteArray(DISTRIBUTED_PREFIX + "$master$key");
+
+ private static final ByteArray TEST_KEY = new ByteArray(DISTRIBUTED_PREFIX + "someKey.foobar");
+
+ /** MetaStorage mock. */
+ private final MetaStorageManager mock;
+
+ /** Captured MetaStorage listener. */
+ private WatchListener lsnr;
+
+ /** Current master key revision. */
+ private long masterKeyRevision;
+
+ private MetaStorageMockWrapper() {
+ mock = mock(MetaStorageManager.class);
+
+ setup();
+ }
+
+ private void setup() {
+ // Returns current master key revision.
+ when(mock.get(MASTER_KEY)).then(invocation -> {
+ return CompletableFuture.completedFuture(new EntryImpl(MASTER_KEY, null, masterKeyRevision, -1));
+ });
+
+ // On any invocation - trigger storage listener.
+ when(mock.invoke(any(), Mockito.<Collection<Operation>>any(), Mockito.any()))
+ .then(invocation -> {
+ triggerStorageListener();
+
+ return CompletableFuture.completedFuture(true);
+ });
+
+ when(mock.invoke(any(), Mockito.<Operation>any(), Mockito.any()))
Review Comment:
can be `when(mock.invoke(any(), any(Operation.class), any()))`
##########
modules/runner/src/test/java/org/apache/ignite/internal/configuration/storage/DistributedConfigurationStorageTest.java:
##########
@@ -74,6 +93,145 @@ void stop() throws Exception {
vaultManager.stop();
}
+ /**
+ * Dummy configuration.
+ */
+ @ConfigurationRoot(rootName = "someKey", type = DISTRIBUTED)
+ public static class DistributedTestConfigurationSchema {
+ @Value(hasDefault = true)
+ public final int foobar = 0;
+ }
+
+ /**
+ * Tests that distributed configuration storage correctly picks up latest configuration MetaStorage revision
+ * during recovery process.
+ *
+ * @throws Exception If failed.
+ */
+ @Test
+ public void testMetaStorageRevisionDifferentFromConfigurationOnRestart() throws Exception {
+ RootKey<DistributedTestConfiguration, DistributedTestView> rootKey = DistributedTestConfiguration.KEY;
+
+ ConfigurationAsmGenerator cgen = new ConfigurationAsmGenerator();
+
+ MetaStorageMockWrapper wrapper = new MetaStorageMockWrapper();
+
+ int configurationChangesCount = 7;
+
+ try (var storage = new DistributedConfigurationStorage(wrapper.metaStorageManager(), vaultManager)) {
+ var changer = new TestConfigurationChanger(cgen, List.of(rootKey), Collections.emptyMap(),
+ storage, Collections.emptyList(), Collections.emptyList());
+
+ changer.start();
+
+ for (int i = 0; i < configurationChangesCount; i++) {
+ ConfigurationSource source = source(
+ rootKey,
+ (DistributedTestChange change) -> change.changeFoobar(1)
+ );
+
+ CompletableFuture<Void> change = changer.change(source);
+
+ change.get();
+ }
+
+ changer.stop();
+ }
+
+ // Put a value to the configuration, so we start on non-empty vault.
+ vaultManager.put(MetaStorageMockWrapper.TEST_KEY, new byte[]{4, 1, 2, 3, 4}).get();
+
+ // This emulates a change in MetaStorage that is not related to the configuration.
+ vaultManager.put(MetaStorageManager.APPLIED_REV, ByteUtils.longToBytes(configurationChangesCount + 1)).get();
+
+ try (var storage = new DistributedConfigurationStorage(wrapper.metaStorageManager(), vaultManager)) {
+ var changer = new TestConfigurationChanger(cgen, List.of(rootKey), Collections.emptyMap(),
+ storage, Collections.emptyList(), Collections.emptyList());
+
+ changer.start();
+
+ CompletableFuture<Long> longCompletableFuture = storage.lastRevision();
+
+ // Should return last configuration change, not last MetaStorage change.
+ Long lastConfigurationChangeRevision = longCompletableFuture.get();
+
+ assertEquals(configurationChangesCount, lastConfigurationChangeRevision);
+
+ changer.stop();
Review Comment:
I guess this should be moved to a `finally` block
##########
modules/runner/src/test/java/org/apache/ignite/internal/configuration/storage/DistributedConfigurationStorageTest.java:
##########
@@ -74,6 +93,145 @@ void stop() throws Exception {
vaultManager.stop();
}
+ /**
+ * Dummy configuration.
+ */
+ @ConfigurationRoot(rootName = "someKey", type = DISTRIBUTED)
+ public static class DistributedTestConfigurationSchema {
+ @Value(hasDefault = true)
+ public final int foobar = 0;
+ }
+
+ /**
+ * Tests that distributed configuration storage correctly picks up latest configuration MetaStorage revision
+ * during recovery process.
+ *
+ * @throws Exception If failed.
+ */
+ @Test
+ public void testMetaStorageRevisionDifferentFromConfigurationOnRestart() throws Exception {
+ RootKey<DistributedTestConfiguration, DistributedTestView> rootKey = DistributedTestConfiguration.KEY;
+
+ ConfigurationAsmGenerator cgen = new ConfigurationAsmGenerator();
+
+ MetaStorageMockWrapper wrapper = new MetaStorageMockWrapper();
+
+ int configurationChangesCount = 7;
Review Comment:
why 7? Isn't one change enough?
##########
modules/runner/src/test/java/org/apache/ignite/internal/configuration/storage/DistributedConfigurationStorageTest.java:
##########
@@ -74,6 +93,145 @@ void stop() throws Exception {
vaultManager.stop();
}
+ /**
+ * Dummy configuration.
+ */
+ @ConfigurationRoot(rootName = "someKey", type = DISTRIBUTED)
+ public static class DistributedTestConfigurationSchema {
+ @Value(hasDefault = true)
+ public final int foobar = 0;
+ }
+
+ /**
+ * Tests that distributed configuration storage correctly picks up latest configuration MetaStorage revision
+ * during recovery process.
+ *
+ * @throws Exception If failed.
+ */
+ @Test
+ public void testMetaStorageRevisionDifferentFromConfigurationOnRestart() throws Exception {
+ RootKey<DistributedTestConfiguration, DistributedTestView> rootKey = DistributedTestConfiguration.KEY;
+
+ ConfigurationAsmGenerator cgen = new ConfigurationAsmGenerator();
+
+ MetaStorageMockWrapper wrapper = new MetaStorageMockWrapper();
+
+ int configurationChangesCount = 7;
+
+ try (var storage = new DistributedConfigurationStorage(wrapper.metaStorageManager(), vaultManager)) {
+ var changer = new TestConfigurationChanger(cgen, List.of(rootKey), Collections.emptyMap(),
+ storage, Collections.emptyList(), Collections.emptyList());
+
+ changer.start();
+
+ for (int i = 0; i < configurationChangesCount; i++) {
+ ConfigurationSource source = source(
+ rootKey,
+ (DistributedTestChange change) -> change.changeFoobar(1)
+ );
+
+ CompletableFuture<Void> change = changer.change(source);
+
+ change.get();
+ }
+
+ changer.stop();
+ }
+
+ // Put a value to the configuration, so we start on non-empty vault.
+ vaultManager.put(MetaStorageMockWrapper.TEST_KEY, new byte[]{4, 1, 2, 3, 4}).get();
+
+ // This emulates a change in MetaStorage that is not related to the configuration.
+ vaultManager.put(MetaStorageManager.APPLIED_REV, ByteUtils.longToBytes(configurationChangesCount + 1)).get();
+
+ try (var storage = new DistributedConfigurationStorage(wrapper.metaStorageManager(), vaultManager)) {
+ var changer = new TestConfigurationChanger(cgen, List.of(rootKey), Collections.emptyMap(),
+ storage, Collections.emptyList(), Collections.emptyList());
+
+ changer.start();
+
+ CompletableFuture<Long> longCompletableFuture = storage.lastRevision();
Review Comment:
I guess this future can be inlined
--
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