sashapolo commented on code in PR #5882:
URL: https://github.com/apache/ignite-3/pull/5882#discussion_r2102663300


##########
modules/configuration/src/main/java/org/apache/ignite/internal/configuration/ConfigurationChanger.java:
##########
@@ -692,49 +705,55 @@ private void validateConfiguration(SuperRoot curRoots, 
SuperRoot changes) {
 
     private ConfigurationStorageListener configurationStorageListener() {
         return changedEntries -> {
-            Map<String, ? extends Serializable> changedValues = 
changedEntries.values();
+            StorageRoots oldStorageRoots = storageRoots;
 
-            // We need to ignore deletion of deprecated values.
-            ignoreDeleted(changedValues, keyIgnorer);
+            try {
+                Map<String, ? extends Serializable> changedValues = 
changedEntries.values();
 
-            StorageRoots oldStorageRoots = storageRoots;
+                // We need to ignore deletion of deprecated values.
+                ignoreDeleted(changedValues, keyIgnorer);
 
-            SuperRoot oldSuperRoot = oldStorageRoots.roots;
-            SuperRoot oldSuperRootNoDefaults = 
oldStorageRoots.rootsWithoutDefaults;
-            SuperRoot newSuperRoot = oldSuperRoot.copy();
-            SuperRoot newSuperNoDefaults = oldSuperRootNoDefaults.copy();
+                SuperRoot oldSuperRoot = oldStorageRoots.roots;
+                SuperRoot oldSuperRootNoDefaults = 
oldStorageRoots.rootsWithoutDefaults;
+                SuperRoot newSuperRoot = oldSuperRoot.copy();
+                SuperRoot newSuperNoDefaults = oldSuperRootNoDefaults.copy();
 
-            Map<String, ?> dataValuesPrefixMap = toPrefixMap(changedValues);
+                Map<String, ?> dataValuesPrefixMap = 
toPrefixMap(changedValues);
 
-            compressDeletedEntries(dataValuesPrefixMap);
+                compressDeletedEntries(dataValuesPrefixMap);
 
-            fillFromPrefixMap(newSuperRoot, dataValuesPrefixMap);
-            fillFromPrefixMap(newSuperNoDefaults, dataValuesPrefixMap);
+                fillFromPrefixMap(newSuperRoot, dataValuesPrefixMap);
+                fillFromPrefixMap(newSuperNoDefaults, dataValuesPrefixMap);
 
-            long newChangeId = changedEntries.changeId();
+                long newChangeId = changedEntries.changeId();
 
-            var newStorageRoots = new StorageRoots(newSuperNoDefaults, 
newSuperRoot, mergeData(oldStorageRoots.data, changedEntries));
+                var newStorageRoots = new StorageRoots(newSuperNoDefaults, 
newSuperRoot, mergeData(oldStorageRoots.data, changedEntries));
 
-            rwLock.writeLock().lock();
+                rwLock.writeLock().lock();
 
-            try {
-                storageRoots = newStorageRoots;
-            } finally {
-                rwLock.writeLock().unlock();
-            }
+                try {
+                    storageRoots = newStorageRoots;
+                } finally {
+                    rwLock.writeLock().unlock();
+                }
 
-            long notificationNumber = 
notificationListenerCnt.incrementAndGet();
+                long notificationNumber = 
notificationListenerCnt.incrementAndGet();
 
-            CompletableFuture<Void> notificationFuture = 
configurationUpdateListener
-                    .onConfigurationUpdated(oldSuperRoot, newSuperRoot, 
newChangeId, notificationNumber);
+                CompletableFuture<Void> notificationFuture = 
configurationUpdateListener
+                        .onConfigurationUpdated(oldSuperRoot, newSuperRoot, 
newChangeId, notificationNumber);
 
-            return notificationFuture.whenComplete((v, t) -> {
-                if (t == null) {
-                    oldStorageRoots.changeFuture.complete(null);
-                } else {
-                    oldStorageRoots.changeFuture.completeExceptionally(t);
-                }
-            });
+                return notificationFuture.whenComplete((v, t) -> {
+                    if (t == null) {
+                        oldStorageRoots.changeFuture.complete(null);
+                    } else {
+                        oldStorageRoots.changeFuture.completeExceptionally(t);
+                    }
+                });
+            } catch (Throwable e) {

Review Comment:
   Note to reviewer: without this "catch", errors happening outside of 
`notificationFuture` will never be propagated to the caller.



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

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

Reply via email to