You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by "rpuch (via GitHub)" <gi...@apache.org> on 2025/06/05 15:16:00 UTC

[PR] IGNITE-25579 Fix index meta storage recovery for dropped indices [ignite-3]

rpuch opened a new pull request, #5997:
URL: https://github.com/apache/ignite-3/pull/5997

   Also, another bug is fixed in ReplicatorUtils, where we could fail if Catalog still contains information about an index that is already removed from the index meta storage
   
   https://issues.apache.org/jira/browse/IGNITE-25579


-- 
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


Re: [PR] IGNITE-25579 Fix index meta storage recovery for dropped indices [ignite-3]

Posted by "rpuch (via GitHub)" <gi...@apache.org>.
rpuch merged PR #5997:
URL: https://github.com/apache/ignite-3/pull/5997


-- 
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


Re: [PR] IGNITE-25579 Fix index meta storage recovery for dropped indices [ignite-3]

Posted by "sashapolo (via GitHub)" <gi...@apache.org>.
sashapolo commented on code in PR #5997:
URL: https://github.com/apache/ignite-3/pull/5997#discussion_r2131611651


##########
modules/network/src/main/java/org/apache/ignite/internal/network/DefaultMessagingService.java:
##########
@@ -373,8 +373,6 @@ private CompletableFuture<Void> sendViaNetwork(
                             () -> triggerChannelCreation(nodeId, type, addr)
                     );
                 })
-                // TODO: IGNITE-25375 - consider removing logging after the fix as it might be too much

Review Comment:
   ?



##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/index/IndexMetaStorage.java:
##########
@@ -360,18 +365,27 @@ private CompletableFuture<Void> recoverIndexMetas() {
             }
 
             // Let's identify the indexes that were removed from the catalog in this version.
+            Set<Integer> knownRemovedIndexIds = new HashSet<>();

Review Comment:
   Why do we need this intermediate set? Can we add elements to `indexIds` directly?



-- 
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


Re: [PR] IGNITE-25579 Fix index meta storage recovery for dropped indices [ignite-3]

Posted by "rpuch (via GitHub)" <gi...@apache.org>.
rpuch commented on code in PR #5997:
URL: https://github.com/apache/ignite-3/pull/5997#discussion_r2131988829


##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/index/IndexMetaStorage.java:
##########
@@ -360,18 +365,27 @@ private CompletableFuture<Void> recoverIndexMetas() {
             }
 
             // Let's identify the indexes that were removed from the catalog in this version.
+            Set<Integer> knownRemovedIndexIds = new HashSet<>();

Review Comment:
   We can, but this way it makes it more difficult to understand the code because `indexIds` collected before the internal loop only contains 'from-catalog' indices, but now we suddenly add 'not-from-catalog' indices to them. This looks pretty confusing even for me who just grokked the code; for others it will be even more confusing



-- 
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


Re: [PR] IGNITE-25579 Fix index meta storage recovery for dropped indices [ignite-3]

Posted by "rpuch (via GitHub)" <gi...@apache.org>.
rpuch commented on code in PR #5997:
URL: https://github.com/apache/ignite-3/pull/5997#discussion_r2131986677


##########
modules/network/src/main/java/org/apache/ignite/internal/network/DefaultMessagingService.java:
##########
@@ -373,8 +373,6 @@ private CompletableFuture<Void> sendViaNetwork(
                             () -> triggerChannelCreation(nodeId, type, addr)
                     );
                 })
-                // TODO: IGNITE-25375 - consider removing logging after the fix as it might be too much

Review Comment:
   This change is not related to this PR, I'm just piggy-backing. The mentioned issue is fixed by another fix, I want to close it; but I still want to leave the logging here as it's now for `CriticalHandshakeException`s (which are always to be seen)



-- 
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