Skip to content

fix: use Http2ClientStream.sendPing() again#787

Merged
anthony-swirldslabs merged 1 commit intomainfrom
769-sendPingHelidon
Apr 15, 2026
Merged

fix: use Http2ClientStream.sendPing() again#787
anthony-swirldslabs merged 1 commit intomainfrom
769-sendPingHelidon

Conversation

@anthony-swirldslabs
Copy link
Copy Markdown
Contributor

Description:
Removing our custom sendPing() implementation and restoring the calls to Helidon's Http2ClientStream.sendPing(). This is to avoid a potential (although never experienced yet) thread-race in the Helidon code due to the way we had to implement our custom sendPing(). This is now possible because Helidon fixed a bug in their sendPing() under helidon-io/helidon#11442 , and that fix is released in Helidon 4.4.1 to which we've upgraded just yesterday: #783

I've run the existing PbjGrpcStressTest that we used to validate the ping situation, and it passes w/o any issues.

Related issue(s):

Fixes #769

Notes for reviewer:
All tests should pass.

Checklist

  • Documented (Code comments, README, etc.)
  • Tested (unit, integration, etc.)

Signed-off-by: Anthony Petrov <anthony@swirldslabs.com>
@anthony-swirldslabs anthony-swirldslabs self-assigned this Apr 15, 2026
@anthony-swirldslabs anthony-swirldslabs requested review from a team as code owners April 15, 2026 20:47
@github-actions
Copy link
Copy Markdown

JUnit Test Report

    81 files  ±0      81 suites  ±0   3m 12s ⏱️ -4s
 1 519 tests ±0   1 515 ✅ ±0   4 💤 ±0  0 ❌ ±0 
10 407 runs  ±0  10 379 ✅ ±0  28 💤 ±0  0 ❌ ±0 

Results for commit 7e0abe1. ± Comparison against base commit dc36e0b.

@github-actions
Copy link
Copy Markdown

Integration Test Report

    419 files  ±0      419 suites  ±0   26m 27s ⏱️ + 7m 39s
114 982 tests ±0  114 982 ✅ ±0  0 💤 ±0  0 ❌ ±0 
115 224 runs  ±0  115 224 ✅ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit 7e0abe1. ± Comparison against base commit dc36e0b.

This pull request removes 3 and adds 3 tests. Note that renamed tests count towards both.
com.hedera.pbj.integration.test.ParserNeverWrapsTest ‑ [1] com.hedera.pbj.integration.test.ParserNeverWrapsTest$$Lambda/0x0000000093c63a10@160c4d33
com.hedera.pbj.integration.test.ParserNeverWrapsTest ‑ [2] com.hedera.pbj.integration.test.ParserNeverWrapsTest$$Lambda/0x0000000093c63c58@fe32439
com.hedera.pbj.integration.test.ParserNeverWrapsTest ‑ [3] com.hedera.pbj.integration.test.ParserNeverWrapsTest$$Lambda/0x0000000093c66000@55c832cb
com.hedera.pbj.integration.test.ParserNeverWrapsTest ‑ [1] com.hedera.pbj.integration.test.ParserNeverWrapsTest$$Lambda/0x0000000090b97d78@7bdca57b
com.hedera.pbj.integration.test.ParserNeverWrapsTest ‑ [2] com.hedera.pbj.integration.test.ParserNeverWrapsTest$$Lambda/0x0000000090b92000@4075a4e2
com.hedera.pbj.integration.test.ParserNeverWrapsTest ‑ [3] com.hedera.pbj.integration.test.ParserNeverWrapsTest$$Lambda/0x0000000090b92248@6550814f

@anthony-swirldslabs anthony-swirldslabs merged commit 3ffd6c6 into main Apr 15, 2026
16 checks passed
@anthony-swirldslabs anthony-swirldslabs deleted the 769-sendPingHelidon branch April 15, 2026 21:16
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.

Use Http2ClientStream.sendPing() again

2 participants