sk0x50 commented on code in PR #5276:
URL: https://github.com/apache/ignite-3/pull/5276#discussion_r1973515684


##########
modules/index/src/main/java/org/apache/ignite/internal/index/IndexBuildController.java:
##########
@@ -184,60 +224,116 @@ private CompletableFuture<?> 
onIndexRemoved(RemoveIndexEventParameters parameter
 
     private CompletableFuture<?> 
onPrimaryReplicaElected(PrimaryReplicaEventParameters parameters) {
         return inBusyLockAsync(busyLock, () -> {
-            TablePartitionId primaryReplicaId = (TablePartitionId) 
parameters.groupId();
-
             if (isLocalNode(clusterService, parameters.leaseholderId())) {
-                primaryReplicaIds.add(primaryReplicaId);
+                // This assert is added for testing purposes, at least for now.
+                assert (enabledColocation() && parameters.groupId() instanceof 
ZonePartitionId)
+                        || (!enabledColocation() && parameters.groupId() 
instanceof TablePartitionId) :
+                        "Primary replica ID must be of type ZonePartitionId if 
colocation is enabled, "
+                                + "otherwise it must be of type 
TablePartitionId";
+
+                primaryReplicaIds.add(parameters.groupId());
 
                 // It is safe to get the latest version of the catalog because 
the PRIMARY_REPLICA_ELECTED event is handled on the
                 // metastore thread.
                 Catalog catalog = 
catalogService.catalog(catalogService.latestCatalogVersion());
 
-                // TODO: IGNITE-22656 It is necessary not to generate an event 
for a destroyed table by LWM
-                if (catalog == null || 
catalog.table(primaryReplicaId.tableId()) == null) {
-                    return nullCompletedFuture();
-                }
+                if (enabledColocation()) {
+                    ZonePartitionId primaryReplicaId = (ZonePartitionId) 
parameters.groupId();
+
+                    CatalogZoneDescriptor zoneDescriptor = 
catalog.zone(primaryReplicaId.zoneId());
+                    // TODO: IGNITE-22656 It is necessary not to generate an 
event for a destroyed zone by LWM
+                    if (zoneDescriptor == null) {
+                        return nullCompletedFuture();
+                    }
 
-                return getMvTableStorageFuture(parameters.causalityToken(), 
primaryReplicaId)
-                        .thenCompose(mvTableStorage -> 
awaitPrimaryReplica(primaryReplicaId, parameters.startTime())
-                                .thenAccept(replicaMeta -> 
tryScheduleBuildIndexesForNewPrimaryReplica(
-                                        catalog.version(),
-                                        primaryReplicaId,
-                                        mvTableStorage,
-                                        replicaMeta
-                                ))
-                        );
+                    var indexFutures = new ArrayList<CompletableFuture<?>>();
+                    for (CatalogTableDescriptor tableDescriptor : 
catalog.tables()) {
+                        // 1. Perhaps, it makes sense to get primary replica 
future first and then get table storage future,
+                        // because, it will be the same for all tables in the 
zone for the given partition.
+                        // 2. Is it possible to filter out tables in efficient 
way
+                        // that do not have indices to avoid creating 
unnecessary futures?
+                        // It looks like 
catalog.indexes(tableDescriptor.id()).isEmpty() is good enough.
+                        // However, what about PK index?

Review Comment:
   It seems, that we should not check that the list of the indices is not empty 
here. We use the latest version of the catalog, so the list can be updated (an 
index can be removed, for example). In this case, we need to stop building 
anyway.
   So, my comment was incorrect, I think.



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