Skip to content

fix: HTTP connection eviction and gRPC keepalive#64

Open
Avinash-Kamath wants to merge 4 commits into
mainfrom
fix/stale-connection-grpc-keepalive
Open

fix: HTTP connection eviction and gRPC keepalive#64
Avinash-Kamath wants to merge 4 commits into
mainfrom
fix/stale-connection-grpc-keepalive

Conversation

@Avinash-Kamath
Copy link
Copy Markdown
Contributor

@Avinash-Kamath Avinash-Kamath commented May 19, 2026

Summary

  • ScalekitAuthClient: Configured PoolingHttpClientConnectionManager with explicit pool sizing (20 total, 10 per route) and added evictExpiredConnections() + evictIdleConnections(30s) to prevent stale pooled connections from being reused after the server has closed them
  • ScalekitClient: Added gRPC keepalive (keepAliveTime=60s, keepAliveTimeout=10s, keepAliveWithoutCalls=true) to proactively detect dead channels before RPCs hit them

Test plan

  • Full integration test suite passes (mvn test)
  • Manually verified HTTP connection eviction by observing pool cleanup after 30s idle
  • Verified gRPC keepalive compiles and connects to live environment

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Chores
    • Improved gRPC connection reliability with enhanced keepalive behavior for more stable connections.
    • Upgraded HTTP client pooling and idle-connection eviction for steadier request handling.
    • Release version bumped to 2.1.2.

Review Change Stack

Avinash-Kamath and others added 2 commits May 19, 2026 11:01
- Add evictExpiredConnections and evictIdleConnections(30s) to HttpClient
  to prevent reuse of stale pooled connections
- Configure connection pool sizing (20 total, 10 per route)
- Add gRPC keepAliveTime(60s), keepAliveTimeout(10s), and
  keepAliveWithoutCalls to detect dead channels proactively
- Add StaleConnectionTests to replicate connection reset scenarios

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 19, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 728cf5fb-0053-4a04-bdab-e12c10d3270b

📥 Commits

Reviewing files that changed from the base of the PR and between 4b4ec8d and 21a2868.

📒 Files selected for processing (1)
  • src/main/java/com/scalekit/ScalekitClient.java
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/main/java/com/scalekit/ScalekitClient.java

Walkthrough

gRPC channel keepalive enabled (60s/10s, pings without active calls); HTTP client uses a pooled connection manager with eviction of expired/idle connections; project version and Constants.version bumped to 2.1.2.

Changes

Connection Resilience

Layer / File(s) Summary
gRPC client keepalive configuration
src/main/java/com/scalekit/ScalekitClient.java
Imports TimeUnit; configures gRPC ManagedChannel with 60s keepalive time, 10s keepalive timeout, and keepalive pings enabled without active calls.
HTTP client connection pooling and eviction
src/main/java/com/scalekit/api/impl/ScalekitAuthClient.java
Adds PoolingHttpClientConnectionManager and TimeUnit imports; initializes CloseableHttpClient with pooled manager (max total and per-route limits), applies request timeouts, and evicts expired/idle connections.
Project version bump
pom.xml, src/main/java/com/scalekit/internal/Constants.java
Maven <version> and Constants.version updated from 2.1.1 to 2.1.2.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

Possibly related PRs

Suggested reviewers

  • AkshayParihar33

Poem

"I’m a rabbit in the code, I hop and I keep,
Pings that whisper steady so channels don’t sleep,
I tidy the sockets, idle threads swept,
Versions bumped higher — the changelog is kept,
I nibble bytes and watch connections sleep."

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main changes: HTTP connection eviction configuration in ScalekitAuthClient and gRPC keepalive settings in ScalekitClient.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/stale-connection-grpc-keepalive

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/main/java/com/scalekit/api/impl/ScalekitAuthClient.java`:
- Around line 57-62: ScalekitAuthClient and ScalekitClient must manage their
pooled resources: make both classes implement AutoCloseable and add a close()
method that explicitly releases their resources — in ScalekitAuthClient call
httpClient.close() and also shut down the underlying connection manager used to
build httpClient (the PoolingHttpClientConnectionManager instance), and in
ScalekitClient keep the created ManagedChannel in a private field (e.g.,
channel) instead of a local variable and in close() call channel.shutdown() (and
await termination or call shutdownNow() as appropriate) to ensure the gRPC
channel is cleaned up; update constructors/fields to store these resources so
close() can access them.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e5e45df9-1e63-445b-860e-62c497f90cd7

📥 Commits

Reviewing files that changed from the base of the PR and between 9d24257 and 938a11e.

📒 Files selected for processing (2)
  • src/main/java/com/scalekit/ScalekitClient.java
  • src/main/java/com/scalekit/api/impl/ScalekitAuthClient.java

Comment thread src/main/java/com/scalekit/api/impl/ScalekitAuthClient.java
Avinash-Kamath and others added 2 commits May 19, 2026 11:43
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Server does not set permitKeepAliveWithoutCalls=true, so client pings
on idle connections would trigger GOAWAY/ENHANCE_YOUR_CALM. Keepalive
pings will now only fire during active RPCs.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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.

2 participants