Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ISPN-14406 Synchronize the partial reindex with refresh and flush #10536

Merged
merged 1 commit into from Dec 14, 2022

Conversation

fax4ever
Copy link
Contributor

indexUpdater.refresh(classSet);

if (!updates.isEmpty()) {
CompletableFuture.allOf(updates.toArray(new CompletableFuture[0])).join();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use org.infinispan.commons.util.concurrent.CompletableFutures.sequence(updates)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure! I'm fixing it

@fax4ever
Copy link
Contributor Author

Thanks @tristantarrant pushed the fix. Let me know if there is anything else we can improve here.

@@ -77,18 +81,25 @@ public Void apply(EmbeddedCacheManager embeddedCacheManager) {
postIndex(indexUpdater, progressState, notifier);
} else {
DataConversion keyDataConversion = cache.getKeyDataConversion();
Set<Class<?>> classSet = new HashSet<>();
Set<Class<?>> classSet = new HashSet<>(keys.size());
List<CompletableFuture<Object>> updates = new ArrayList<>(keys.size());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry to ping pong back and forth. The preferred way of doing this is with https://github.com/infinispan/infinispan/blob/main/core/src/main/java/org/infinispan/util/concurrent/CompletionStages.java#L44

This way you don't need to create another collection and the stages are as minimal as possible.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea. I don't know the utility very well, I tried to see other usages and tried to use it here.

@fax4ever
Copy link
Contributor Author

Thank for the idea @wburns. Tried to applied the change here. Let me know if we can do better...

@tristantarrant tristantarrant merged commit 8a36c52 into infinispan:main Dec 14, 2022
@tristantarrant
Copy link
Member

Merged, thanks

@tristantarrant
Copy link
Member

can you backport to 14.0.x please _

@fax4ever
Copy link
Contributor Author

can you backport to 14.0.x please _

sure!

@fax4ever
Copy link
Contributor Author

can you backport to 14.0.x please _

#10542

@fax4ever fax4ever deleted the ISPN-14406 branch December 15, 2022 15:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants