GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — Allow scheduling of CF completion - Attempt 1 by bbakerman · Pull Request #275 · graphql-java/java-dataloader

Conversation

bbakerman commented May 17, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link

Copy Markdown

Member

The completion of CF promises can now be scheduled on another thread

A step in the batch loading process is the completion of the futures that have been promised earlier via
Dataloader.load(). This can also be scheduled on another thread if need be, allowing the dispatch call
to return quicker.

You might want to schedule the completion of values on another thread if there is following work t do such as :

var cfStage1 = dataLoader.load(key);
var cfStage2 = cfStage1.thenApply(v -> doSomethingSlow(v));
//...
var dispatchCF = dataLoader.dispatch();

In the above example the .doSomethingSlow(v) call will happen inside the completion
of the cfStage1 code path since CompletableFuture by design eagerly runs dependent chained methods.

Perhaps you want this completion to happen more asynchronously so that the .dispatch() returns more
quickly and is not bound to the .doSomethingSlow(v) call.

By default, the dispatch completion is done on the current thread in a synchronous manner, which will include
any extra CompletableFuture chained methods.

This is an example of running the completion step in an asynchronous manner :

@Override
public <K> CompletionStage<?> scheduleCompletion(Runnable completeValuesRunnable, List<K> keys, BatchLoaderEnvironment environment) {
    return CompletableFuture.runAsync(completeValuesRunnable);
}

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions


          Allow scheduling of CF completion

3075fcc

bbakerman commented

May 17, 2026

View reviewed changes

Comment thread

src/main/java/org/dataloader/DataLoaderHelper.java

		private CompletableFuture<List<V>> dispatchQueueBatch(List<K> keys, List<Object> keyContexts, List<CompletableFuture<V>> queuedFutures) {
		stats.incrementBatchLoadCountBy(keys.size(), new IncrementBatchLoadCountByStatisticsContext<>(keys, keyContexts));

		BatchLoaderEnvironment environment = mkBatchLoaderEnv(keys, keyContexts);

bbakerman May 17, 2026

Copy link

Copy Markdown

Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

We now create the BatchLoaderEnvironment earlier and send it down into the other methods earlier

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

bbakerman commented

May 17, 2026

View reviewed changes

Comment thread

src/main/java/org/dataloader/DataLoaderHelper.java

		Try<V> tryValue = (Try<V>) value;
		if (tryValue.isSuccess()) {
		future.complete(tryValue.get());
		} else {

bbakerman May 17, 2026

Copy link

Copy Markdown

Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

The above is now run in the runnable and hence can be scheduled onto some other thread

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

fxbonnet May 17, 2026

Copy link

Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

This moved the CF completions to another thread but the for loop still invokes them sequentially which is a problem if there are many keys in the batch. Could we schedule the completion for each key in a separate thread instead?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

bbakerman May 18, 2026

Copy link

Copy Markdown

Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

So are you saying we pass in

 public CompleableFuture<Void> completeThese(List<CompletableFuture> cfs, List<Object> values)

And the the caller could sequentially run them as today OR choose to run them in parralel themselves say?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

bbakerman commented

May 17, 2026

View reviewed changes

Comment thread

src/main/java/org/dataloader/DataLoaderHelper.java

		if (value instanceof Throwable) {
		stats.incrementLoadErrorCount(new IncrementLoadErrorCountStatisticsContext<>(key, callContext));
		future.completeExceptionally(tryValue.getThrowable());
		future.completeExceptionally((Throwable) value);

bbakerman May 17, 2026

Copy link

Copy Markdown

Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

diff is whacky. This has not changed

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions


          Allow scheduling of CF completion

8eadcb2

bbakerman commented

May 17, 2026

View reviewed changes

Comment thread

src/main/java/org/dataloader/DataLoaderHelper.java

		Object context = loaderOptions.getBatchLoaderContextProvider().getContext();
		return BatchLoaderEnvironment.newBatchLoaderEnvironment()
		.context(context).keyContexts(keys, keyContexts).build();
		}

bbakerman May 17, 2026

Copy link

Copy Markdown

Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Called in two places - common method

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

bbakerman commented

May 17, 2026

View reviewed changes

Comment thread

src/test/java/org/dataloader/impl/CompletableFutureKitTest.java

		import static org.hamcrest.Matchers.instanceOf;
		import static org.junit.jupiter.api.Assertions.assertThrows;

		class CompletableFutureKitTest {

bbakerman May 17, 2026

Copy link

Copy Markdown

Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

never had tests

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

fxbonnet reviewed

May 17, 2026

View reviewed changes

Comment thread

src/main/java/org/dataloader/impl/CompletableFutureKit.java

		return future;
		}

		public static <V> CompletableFuture<V> success(V v) {

fxbonnet May 17, 2026

Copy link

Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Isn't this the same as CompletableFuture.completedFuture(v)?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

bbakerman May 18, 2026

Copy link

Copy Markdown

Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Yes - but here for "completeness" ;)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

Comment thread

src/main/java/org/dataloader/DataLoaderHelper.java

		Try<V> tryValue = (Try<V>) value;
		if (tryValue.isSuccess()) {
		future.complete(tryValue.get());
		} else {

fxbonnet May 17, 2026

Copy link

Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

andimarek mentioned this pull request

May 18, 2026

Allow scheduling of CF completion in parallel #277

Open

bbakerman changed the title ~~Allow scheduling of CF completion~~ Allow scheduling of CF completion - Attempt 1

Jun 21, 2026

Reviewers

1 more reviewer

fxbonnet

fxbonnet left review comments

Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

2 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

Conversation

bbakerman commented May 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

bbakerman commented May 17, 2026 •

edited

Loading

Uh oh!

There was an error while loading. Please reload this page.