Skip to content

Implement shared FIFO execution queue for Async/H2 client#776

Open
arturobernalg wants to merge 2 commits intoapache:masterfrom
arturobernalg:h2-client-shared-exec-queue
Open

Implement shared FIFO execution queue for Async/H2 client#776
arturobernalg wants to merge 2 commits intoapache:masterfrom
arturobernalg:h2-client-shared-exec-queue

Conversation

@arturobernalg
Copy link
Copy Markdown
Member

@arturobernalg arturobernalg commented Jan 6, 2026

Add a shared FIFO execution queue to cap concurrently executing async requests per client instance. Queue overflow requests and execute them when in-flight execution completes. Ensure queue slots are released on all terminal paths, including synchronous start failures.

@arturobernalg arturobernalg requested a review from ok2c January 6, 2026 16:55
@arturobernalg arturobernalg force-pushed the h2-client-shared-exec-queue branch from 369c6db to 97067f4 Compare January 6, 2026 18:04
@arturobernalg arturobernalg changed the title Implement shared FIFO execution queue for H2 client Implement shared FIFO execution queue for Async/H2 client Jan 7, 2026
@ok2c
Copy link
Copy Markdown
Member

ok2c commented Apr 4, 2026

@arturobernalg Please rebase this change-set whenever you have a minute

@arturobernalg arturobernalg force-pushed the h2-client-shared-exec-queue branch 2 times, most recently from 42909c4 to 964f1ae Compare April 7, 2026 07:18
@arturobernalg
Copy link
Copy Markdown
Member Author

arturobernalg commented Apr 7, 2026

@arturobernalg Please rebase this change-set whenever you have a minute

@ok2c done

@arturobernalg arturobernalg force-pushed the h2-client-shared-exec-queue branch from 964f1ae to fe239e5 Compare April 7, 2026 08:06
ok2c
ok2c requested changes Apr 26, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@ok2c ok2c left a comment

Choose a reason for hiding this comment

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

@arturobernalg Good, overall

Comment thread httpclient5/src/main/java/org/apache/hc/client5/http/impl/async/InternalH2AsyncExecRuntime.java Outdated
Comment thread ...lient5/src/main/java/org/apache/hc/client5/http/impl/async/InternalHttpAsyncExecRuntime.java
Comment thread ...t5/src/test/java/org/apache/hc/client5/http/examples/AsyncSharedClientQueueLocalExample.java Outdated
ok2c
ok2c reviewed Apr 26, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@ok2c ok2c left a comment

Choose a reason for hiding this comment

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

@arturobernalg Lookd good overall.

@arturobernalg arturobernalg force-pushed the h2-client-shared-exec-queue branch from e315c7a to 86b2bed Compare April 29, 2026 17:36
@arturobernalg
Copy link
Copy Markdown
Member Author

arturobernalg commented Apr 29, 2026

@arturobernalg Lookd good overall.

@ok2c please do one last pass

@arturobernalg arturobernalg requested a review from ok2c April 29, 2026 17:37
ok2c
ok2c requested changes May 1, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@ok2c ok2c left a comment

Choose a reason for hiding this comment

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

@arturobernalg Just a few minor tweaks.

Comment thread ...client5/src/main/java/org/apache/hc/client5/http/impl/async/SharedRequestExecutionQueue.java Outdated
Comment thread ...client5/src/main/java/org/apache/hc/client5/http/impl/async/SharedRequestExecutionQueue.java
Comment thread ...client5/src/test/java/org/apache/hc/client5/http/examples/AsyncSharedClientQueueExample.java
Comment thread ...ient5/src/test/java/org/apache/hc/client5/http/examples/H2SharedClientQueueLocalExample.java Outdated
@arturobernalg arturobernalg force-pushed the h2-client-shared-exec-queue branch 3 times, most recently from cf01866 to c431769 Compare May 2, 2026 14:23
@arturobernalg arturobernalg requested a review from ok2c May 2, 2026 14:24
@ok2c
Copy link
Copy Markdown
Member

ok2c commented May 3, 2026 •
edited
Loading

@arturobernalg You overlooked one my comment about AtomicReference. Looks good otherwise.

@arturobernalg
Implement shared FIFO execution queue for H2 client
58cec36 
Introduce one shared per-client queue to cap concurrently executing requests and enqueue overflow.
Ensure queued starts release the slot on any terminal path, including synchronous start failures.
@arturobernalg arturobernalg force-pushed the h2-client-shared-exec-queue branch from 8e05e36 to 58cec36 Compare May 3, 2026 13:14
@arturobernalg arturobernalg requested a review from ok2c May 3, 2026 13:15
@ok2c
Copy link
Copy Markdown
Member

ok2c commented May 4, 2026

@arturobernalg I think we may have overlooked one important detail. What is going to happen if the client gets shut down and there are still pending requests stuck in the execution queue. We need to make sure those requests get properly cancelled.

@arturobernalg
Copy link
Copy Markdown
Member Author

arturobernalg commented May 4, 2026

@arturobernalg I think we may have overlooked one important detail. What is going to happen if the client gets shut down and there are still pending requests stuck in the execution queue. We need to make sure those requests get properly cancelled.

@ok2c
I added a close path to the shared execution queue and close it from the async client shutdown path. Pending queued exchanges are now cancelled instead of being left stuck in the queue.

@arturobernalg arturobernalg requested a review from ok2c May 4, 2026 07:07
ok2c
ok2c reviewed May 5, 2026
View reviewed changes
Comment thread ...client5/src/main/java/org/apache/hc/client5/http/impl/async/SharedRequestExecutionQueue.java Outdated
private static final int STARTED = 1;
private static final int CANCELLED = 2;

private static final AtomicIntegerFieldUpdater<Entry> STATE_UPDATER =
Copy link
Copy Markdown
Member

@ok2c ok2c May 5, 2026

Choose a reason for hiding this comment

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

@arturobernalg Is there a reason you reverted your implementation back to AtomicIntegerFieldUpdater?

@arturobernalg arturobernalg force-pushed the h2-client-shared-exec-queue branch from 1f2ea10 to e8c6f46 Compare May 5, 2026 11:00
@arturobernalg arturobernalg requested a review from ok2c May 5, 2026 11:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ok2c ok2c ok2c approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@arturobernalg @ok2c