Skip to content

Fix kotlin-ktor benchmark CI failures (see #472)#506

Open
yalishevant wants to merge 12 commits intorenaissance-benchmarks:masterfrom
yalishevant:fix-zavidnyi-pr
Open

Fix kotlin-ktor benchmark CI failures (see #472)#506
yalishevant wants to merge 12 commits intorenaissance-benchmarks:masterfrom
yalishevant:fix-zavidnyi-pr

Conversation

@yalishevant
Copy link
Copy Markdown

Summary

Fixes CI failures in the kotlin-ktor benchmark originally introduced in #472 by @zavidnyi and continued by @nomisRev.

The benchmark worked locally but failed on all CI platforms. Root causes:

  • Validation mismatch on odd CPU counts: The validator computed expected task count using floating-point arithmetic, while ClientManager.assignTasks uses .toInt() truncation. On macOS CI runners (3 vCPUs), this produced a mismatch (expected 450 vs actual 400).
  • Fragile transitive dependencies: ktor-server-tests-jvm and ktor-server-auth-jvm pulled in classes the benchmark actually needs (WebSockets, DefaultHeaders, client CIO) as transitive deps. Replaced with explicit dependencies.
  • DNS resolution in CI containers: localhost can be unreliable in containerized environments. Switched to 127.0.0.1 in both server and client.
  • Race condition on server startup: Clients could attempt to connect before the server was ready. Added a socket-based readiness check.
  • Potential indefinite hang: waitForMessage had no timeout, risking CI job timeouts. Added a 30-second timeout via withTimeout.
  • Stale files: Removed output.csv and output.json committed by mistake.

zavidnyi and others added 12 commits April 28, 2024 15:24
Kotlin plugin for SBT was added. All dependencies specified
Fix validation mismatch caused by floating-point vs integer arithmetic
when computing expected task counts. The validator used floating-point
multiplication while ClientManager.assignTasks uses .toInt() truncation,
producing different results on hosts with odd CPU counts (e.g., macOS
CI runners with 3 vCPUs: expected 450 vs actual 400).

Replace fragile transitive dependencies (ktor-server-tests-jvm,
ktor-server-auth-jvm) with explicit ones that the benchmark actually
uses: ktor-server-websockets-jvm, ktor-server-default-headers-jvm,
ktor-client-core-jvm, ktor-client-cio-jvm. Also fix the
ktor-server-call-logging artifact to use the -jvm variant.

Use 127.0.0.1 instead of localhost in both server and client to avoid
DNS resolution issues in containerized CI environments. Add a
socket-based server readiness check before creating WebSocket clients
to prevent races on slow CI hosts. Add a 30-second timeout to
waitForMessage to prevent indefinite hangs if a frame is never received.

Remove stale output.csv and output.json that were committed by mistake.
@yalishevant
Copy link
Copy Markdown
Author

Hi @vhotspur, could you please approve the workflow run for this PR (and for #507)?

This is a continuation of #472 — I've fixed the CI failures that @nomisRev encountered (validation mismatch on odd CPU counts, fragile transitive dependencies, server startup race condition). All checks pass locally, including base/standalone/JMH modes with different client counts.

@yalishevant
Copy link
Copy Markdown
Author

Just a gentle ping — all 21 CI checks are passing across the full matrix (as well as in #507). Would be great if you could take a look when you get a chance 🙂

Happy to address any feedback!

@lbulej
Copy link
Copy Markdown
Member

lbulej commented Apr 14, 2026

Just a gentle ping — all 21 CI checks are passing across the full matrix (as well as in #507). Would be great if you could take a look when you get a chance 🙂

Thanks for pushing this through! I'll have a look.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants