feat(http-delivery): Use HttpClient.sendAsync for deliverBatch by rawo · Pull Request #77 · softwaremill/okapi

rawo · 2026-07-20T09:44:15Z

Overrides HttpMessageDeliverer.deliverBatch to fire all requests via httpClient.sendAsync() up front instead of blocking sequentially on httpClient.send(). Each future is chained with .thenApply/.exceptionally so it always settles successfully, then .join()'d in input order.

KOJAK-74 — HTTP deliverBatch
Builds on KOJAK-72 (deliverBatch interface, already on main) — same shape as KOJAK-73 (Kafka fire-flush-await).

Headline numbers

Full JMH config vs the sequential baseline (MacBook M3 Max, JDK 21 LTS, Postgres 16 + WireMock via Testcontainers, fork=2, warmup=3×10s, iter=5×30s):

batchSize	latency 20ms — Baseline	Optimized	Improvement	latency 100ms — Baseline	Optimized	Improvement
10	26.429 ms/op	5.041 ms/op	5.24×	108.515 ms/op	14.119 ms/op	7.69×
50	24.892 ms/op	2.464 ms/op	10.10×	105.313 ms/op	7.484 ms/op	14.07×
100	26.545 ms/op	2.175 ms/op	12.21×	107.714 ms/op	7.059 ms/op	15.26×

Translated: batchSize=50/latency=20ms goes from ~40 msg/s to ~406 msg/s.

Full results + interpretation: benchmarks/results-postopt-kojak-74.md.

Honest caveat: measured gains (5.2×–15.3×) fall short of the ticket's 1000+ msg/s aspiration at batchSize=50/latency=20ms. Root cause: with network wait now parallelized away, per-batch DB overhead (claim query + N individual UPDATEs) dominates — the same effect KOJAK-73's results doc already flagged as motivation for KOJAK-75 (batch UPDATE via executeBatch), which should lift these numbers further with no further change to HttpMessageDeliverer. latencyMs=0 regressed slightly (0.48×–0.91×) as expected — CompletableFuture overhead has no I/O wait to hide behind at zero latency.

Test coverage

New unit tests in HttpMessageDelivererBatchTest:

Empty input, all-success order preservation, mixed success/failure classification, all-fail batch
Connection reset → RetriableFailure
Poison-pill metadata isolated to one entry, others unaffected
Custom injected HttpClient honored
TLS handshake failure → PermanentFailure

New HttpMessageDelivererBatchConcurrencyTest proves concurrency directly via WireMock's request journal (allServeEvents) — all requests' logged timestamps land within one delay window, not spread sequentially.

Existing single-entry HttpMessageDelivererTest passes unmodified. Integration tests (HttpEndToEndTest, MysqlHttpEndToEndTest) continue to pass with real Postgres + WireMock, now exercising deliverBatch end-to-end.

Test plan

./gradlew ktlintCheck — clean project-wide
./gradlew build — full unit test suite green
./gradlew :okapi-integration-tests:test --tests "Http" — Testcontainers Postgres + WireMock suite passes
./gradlew :okapi-benchmarks:jmhJar + full HttpThroughputBenchmark matrix — captured numbers above

…entation

Copilot

Pull request overview

This PR improves HTTP batch delivery throughput by overriding HttpMessageDeliverer.deliverBatch to fire all HTTP requests concurrently via HttpClient.sendAsync() and then await completion in input order, while preserving per-entry classification and non-throwing semantics.

Changes:

Refactors HttpMessageDeliverer to share request-building and response/exception classification between deliver() and the new concurrent deliverBatch().
Adds unit + concurrency tests to verify per-entry classification, order preservation, and that requests are actually in flight concurrently.
Adds benchmark artifacts documenting the observed throughput improvement and raw JMH output.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
okapi-http/src/main/kotlin/com/softwaremill/okapi/http/HttpMessageDeliverer.kt	Implements concurrent `deliverBatch()` via `sendAsync()` and refactors shared helpers.
okapi-http/src/test/kotlin/com/softwaremill/okapi/http/HttpMessageDelivererBatchTest.kt	Adds unit tests covering ordering, per-entry classification, and failure modes.
okapi-http/src/test/kotlin/com/softwaremill/okapi/http/HttpMessageDelivererBatchConcurrencyTest.kt	Adds a test that validates concurrent in-flight requests using WireMock request timestamps.
benchmarks/results-postopt-kojak-74.md	Adds a written benchmark summary and interpretation for KOJAK-74.
benchmarks/http-deliverbatch.json	Adds raw JMH benchmark output used by the results writeup.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (2)

benchmarks/results-postopt-kojak-74.md:4

This results doc says the benchmarks were run on JDK 25.0.2, while the PR description claims JDK 21 LTS. That makes the headline numbers hard to compare/reproduce; please align the stated JDK version(s) (either rerun on the intended JDK or call out the difference explicitly).

Measured on MacBook M3 Max, JDK 25 (25.0.2), Postgres 16 + WireMock (in-JVM) via Testcontainers,
full JMH config: `fork=2, warmup=3 × 10s, iter=5 × 30s` — n=10 samples per benchmark.

benchmarks/http-deliverbatch.json:10

The committed JMH JSON includes an absolute local JVM path with a username (/Users/rawo/...). This is accidental PII/environment leakage and also reduces portability of the artifact. Consider stripping machine-specific fields (like "jvm") before committing, or storing this JSON as a build artifact instead of in-repo.

        "threads" : 1,
        "forks" : 2,
        "jvm" : "/Users/rawo/.sdkman/candidates/java/25.0.2-open/bin/java",
        "jvmArgs" : [
            "-Xms8g",

…enchmark results

… in tests

The KDoc states the JDK connection pool multiplexes same-host requests via HTTP/2 automatically, but HTTP/2 is only used when the server supports it (otherwise the client falls back to HTTP/1.1, potentially with multiple connections). Consider rewording so it doesn’t imply HTTP/2/multiplexing is guaranteed, while still describing the intended overlap of requests. Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Rafał Wokacz <rafal.wokacz@gmail.com>

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

…ults. Using HttpRequest.Builder.header(...) for both the default Content-Type and user-provided headers appends values; it does not replace existing ones. If a user sets Content-Type in HttpDeliveryInfo.headers, the request will likely contain two Content-Type headers rather than overriding the default (the existing HttpMessageDelivererTest implies overriding is desired). Prefer setHeader(...) so user-provided headers replace defaults. Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Rafał Wokacz <rafal.wokacz@gmail.com>

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

okapi-http/src/main/kotlin/com/softwaremill/okapi/http/HttpMessageDeliverer.kt:65

CompletableFuture.join() ignores thread interruption, so if the processor thread is interrupted (e.g., during shutdown) this method can keep blocking until the per-request timeout elapses. That’s a behavioral regression vs the sequential HttpClient.send() path, which can surface InterruptedException immediately and restore the interrupt flag. Consider awaiting with get() (or get(timeout, ...)) and mapping InterruptedException to RetriableFailure while restoring the interrupt, so shutdown remains responsive.

                is SendAttempt.InFlight -> attempt.future.join()

…er() and deliverBatch()

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (3)

benchmarks/results-postopt-kojak-74.md:10

The results doc says the optimized implementation awaits with join, but the code now awaits with CompletableFuture.get() (to observe interrupts). This line should be updated to avoid misleading readers.

Optimized is parallel `httpClient.sendAsync()` fire-all + `thenApply`/`exceptionally` + `join`.

benchmarks/results-postopt-kojak-74.md:52

This section describes the await step as .join() and claims it never throws. The implementation actually uses CompletableFuture.get() so the caller can be interrupted; InterruptedException is handled and classified as RetriableFailure. The doc should reflect the current behavior.

3. **Await** — `.join()` per entry, in input order; since step 2 guarantees no exceptional
   completion, `join()` never throws. The JDK `HttpClient`'s own per-request 30s timeout
   (`HttpRequest.timeout()`) bounds the wait — no separate await-timeout needed (unlike Kafka's
   `flush()`, which has no equivalent per-record deadline).

okapi-http/src/test/kotlin/com/softwaremill/okapi/http/HttpMessageDelivererBatchConcurrencyTest.kt:61

The timestamp-spread assertion is very strict (spreadMs < delayMs) and can be flaky due to scheduling jitter / timestamp granularity even when requests are clearly overlapped. Adding a small epsilon keeps the intent (prove non-sequential send) while reducing spurious failures.

        val spreadMs = loggedTimestamps.maxOrNull()!! - loggedTimestamps.minOrNull()!!
        spreadMs.shouldBeLessThan(delayMs)

…rency

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

okapi-http/src/test/kotlin/com/softwaremill/okapi/http/HttpMessageDelivererInterruptionTest.kt:34

awaitBlocked() waits for Thread.State.WAITING/TIMED_WAITING, but threads blocked in HttpClient.send() are typically reported as RUNNABLE (native socket read), so this polling can time out and make the interruption tests flaky on some JDKs/CI environments. Consider detecting "blocked" by inspecting the thread's stack trace for java.net.http / CompletableFuture.get() frames instead of relying on Thread.state.

private fun awaitBlocked(thread: Thread, timeoutMs: Long = 5_000) {
    val deadline = System.currentTimeMillis() + timeoutMs
    while (thread.state != Thread.State.WAITING && thread.state != Thread.State.TIMED_WAITING) {
        check(System.currentTimeMillis() < deadline) { "Thread never blocked (state=${thread.state})" }
        Thread.sleep(5)

benchmarks/results-postopt-kojak-74.md:4

Benchmark write-up reports running on JDK 25 here, but the PR description says the headline numbers were captured on JDK 21 LTS. Since the results JSON in this PR also references JDK 25, please reconcile the JDK version across the PR description and the committed benchmark artifacts so the performance comparison context is unambiguous.

Measured on MacBook M3 Max, JDK 25 (25.0.2), Postgres 16 + WireMock (in-JVM) via Testcontainers,
full JMH config: `fork=2, warmup=3 × 10s, iter=5 × 30s` — n=10 samples per benchmark.

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

+        val start = System.currentTimeMillis()
+        val results = deliverer.deliverBatch(entries)
+        val elapsedMs = System.currentTimeMillis() - start
+


+private fun awaitBlocked(thread: Thread, timeoutMs: Long = 5_000) {
+    val deadline = System.currentTimeMillis() + timeoutMs
+    while (thread.state != Thread.State.WAITING && thread.state != Thread.State.TIMED_WAITING) {
+        check(System.currentTimeMillis() < deadline) { "Thread never blocked (state=${thread.state})" }
+        Thread.sleep(5)
+    }


…ions

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

…quests have been received, rather than relying on [Thread.State]

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

feat(http-delivery): Use HttpClient.sendAsync for deliverBatch implem…

bb6ff12

…entation

rawo requested review from Copilot and endrju19 July 20, 2026 09:44

Copilot started reviewing on behalf of rawo July 20, 2026 09:44 View session

Copilot AI reviewed Jul 20, 2026

View reviewed changes

refactor(scheduler): move exception catch to a common method

9a6b3b0

rawo requested a review from Copilot July 21, 2026 13:25

Copilot started reviewing on behalf of rawo July 21, 2026 13:26 View session

Copilot AI reviewed Jul 21, 2026

View reviewed changes

Comment thread okapi-http/src/main/kotlin/com/softwaremill/okapi/http/HttpMessageDeliverer.kt Outdated

Comment thread okapi-http/src/main/kotlin/com/softwaremill/okapi/http/HttpMessageDeliverer.kt Outdated

rawo added 2 commits July 21, 2026 15:31

chore(scheduler): replace user specific jvm path to generic path in b…

592346a

…enchmark results

chore(scheduler): use maxOrNull() minOrNUll() in place of max() min()…

f0cc72a

… in tests

Copilot AI review requested due to automatic review settings July 21, 2026 13:38

Copilot started reviewing on behalf of rawo July 21, 2026 13:39 View session

Copilot AI reviewed Jul 21, 2026

View reviewed changes

Comment thread okapi-http/src/main/kotlin/com/softwaremill/okapi/http/HttpMessageDeliverer.kt

Comment thread okapi-http/src/main/kotlin/com/softwaremill/okapi/http/HttpMessageDeliverer.kt

Comment thread okapi-http/src/main/kotlin/com/softwaremill/okapi/http/HttpMessageDeliverer.kt Outdated

Copilot AI review requested due to automatic review settings July 21, 2026 13:42

Copilot started reviewing on behalf of rawo July 21, 2026 13:42 View session

Copilot AI reviewed Jul 21, 2026

View reviewed changes

fix(http-delivery): gracefully handle threads interruptions for deliv…

35405ca

…er() and deliverBatch()

Copilot AI review requested due to automatic review settings July 21, 2026 13:58

Copilot started reviewing on behalf of rawo July 21, 2026 13:58 View session

Copilot AI reviewed Jul 21, 2026

View reviewed changes

rawo requested a review from Copilot July 21, 2026 14:20

Copilot started reviewing on behalf of rawo July 21, 2026 14:21 View session

Copilot AI reviewed Jul 21, 2026

View reviewed changes

Comment thread okapi-http/src/test/kotlin/com/softwaremill/okapi/http/HttpMessageDelivererInterruptionTest.kt Outdated

Comment thread okapi-http/src/test/kotlin/com/softwaremill/okapi/http/HttpMessageDelivererInterruptionTest.kt Outdated

fix(http-delivery): make multithreaded test less flacky due to concur…

d8b390f

…rency

Copilot AI review requested due to automatic review settings July 21, 2026 16:57

Copilot started reviewing on behalf of rawo July 21, 2026 16:57 View session

Copilot AI reviewed Jul 21, 2026

View reviewed changes

refactor(http-delivery): add awaitTermination() in tests

d2007b4

Copilot AI review requested due to automatic review settings July 21, 2026 17:14

Copilot started reviewing on behalf of rawo July 21, 2026 17:14 View session

Copilot AI reviewed Jul 21, 2026

View reviewed changes

Merge branch 'main' into feat/kojak-74-deliver-batch

3ebdb4f

Copilot AI review requested due to automatic review settings July 23, 2026 08:43

Copilot started reviewing on behalf of rawo July 23, 2026 08:44 View session

Copilot AI reviewed Jul 23, 2026

View reviewed changes

fix(http-delivery): use wiremock.httpsPort() in testing https connect…

cac8e7e

…ions

Copilot AI review requested due to automatic review settings July 23, 2026 11:04

Copilot started reviewing on behalf of rawo July 23, 2026 11:05 View session

Copilot AI reviewed Jul 23, 2026

View reviewed changes

fix(http-delivery): Polls WireMock's request journal until [count] re…

e7f6e7c

…quests have been received, rather than relying on [Thread.State]

Copilot AI review requested due to automatic review settings July 23, 2026 11:47

Copilot started reviewing on behalf of rawo July 23, 2026 11:47 View session

Copilot AI reviewed Jul 23, 2026

View reviewed changes

Uh oh!

Conversation

rawo commented Jul 20, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants