rpuch commented on code in PR #5276:
URL: https://github.com/apache/ignite-3/pull/5276#discussion_r1975159209
##########
modules/index/src/main/java/org/apache/ignite/internal/index/IndexBuildController.java:
##########
@@ -184,60 +233,113 @@ 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);
+ // TODO https://issues.apache.org/jira/browse/IGNITE-24375
+ // Need to remove TablePartitionId check here and below.
+ assert parameters.groupId() instanceof ZonePartitionId ||
parameters.groupId() instanceof TablePartitionId :
+ "Primary replica ID must be of type ZonePartitionId or
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 (parameters.groupId() instanceof ZonePartitionId) {
+ assert enabledColocation() : "Primary replica ID must be
of type ZonePartitionId";
- return getMvTableStorageFuture(parameters.causalityToken(),
primaryReplicaId)
- .thenCompose(mvTableStorage ->
awaitPrimaryReplica(primaryReplicaId, parameters.startTime())
- .thenAccept(replicaMeta ->
tryScheduleBuildIndexesForNewPrimaryReplica(
- catalog.version(),
- primaryReplicaId,
- mvTableStorage,
- replicaMeta
- ))
- );
+ 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();
+ }
+
+ 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?
+ CompletableFuture<?> future =
+
getMvTableStorageFuture(parameters.causalityToken(), tableDescriptor.id())
+ .thenCompose(mvTableStorage ->
awaitPrimaryReplica(primaryReplicaId, parameters.startTime())
+ .thenAccept(replicaMeta ->
tryScheduleBuildIndexesForNewPrimaryReplica(
+ catalog.version(),
+ tableDescriptor,
+ primaryReplicaId,
+ mvTableStorage,
+ replicaMeta)));
+
+ indexFutures.add(future);
+ }
+
+ return
allOf(indexFutures.toArray(CompletableFuture[]::new));
Review Comment:
Please note: there is `CompletableFuture#allOf()` from the standard library
which you use here, it accepts an array; and there is
`CompletableFutures#allOf()` defined in our code (mind the 's' in which the
class name ends) which accepts a collection
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
