Skip to content

feat(BA-6894): implement network timeout idle checker#12878

Open
seedspirit wants to merge 3 commits into
mainfrom
feat/BA-6894
Open

feat(BA-6894): implement network timeout idle checker#12878
seedspirit wants to merge 3 commits into
mainfrom
feat/BA-6894

Conversation

@seedspirit

Copy link
Copy Markdown
Contributor

Summary

  • Add NetworkTimeoutChecker to the sokovan reconciler idle-check stage: judges interactive sessions idle when they have no active app connections and the last network access exceeds the configured timeout.
  • Define NetworkTimeoutSpec.idle_timeout_seconds (0 disables the checker definition) and register the checker via DI (valkey_live wired through orchestration composers to build_idle_check_stage).
  • Add ValkeyLiveClient.count_active_connections_batch to fetch per-session connection counts in a single batch alongside batched last-access reads.

Test plan

  • pants test on test_network_timeout_checker.py, test_stage.py, test_valkey_live_client.py (checker judgments, DI wiring, batch count against real Valkey)
  • pants fmt/lint/check on changed files
  • CI green

Resolves BA-6894

🤖 Generated with Claude Code

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@github-actions github-actions Bot added the size:L 100~500 LoC label Jul 15, 2026
@github-actions github-actions Bot added comp:manager Related to Manager component comp:common Related to Common component labels Jul 15, 2026
@seedspirit
seedspirit marked this pull request as ready for review July 15, 2026 07:55
@seedspirit
seedspirit requested a review from a team as a code owner July 15, 2026 07:55
Copilot AI review requested due to automatic review settings July 15, 2026 07:55
…ons_batch

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds reconciler-based network timeout checking for interactive sessions using Valkey activity signals.

Changes:

  • Implements configurable network-idle judgments.
  • Adds batched active-connection counting.
  • Wires Valkey dependencies and adds tests.

Reviewed changes

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

Show a summary per file
File Description
tests/unit/manager/sokovan/idle_check/test_stage.py Updates stage dependency tests.
tests/unit/manager/sokovan/idle_check/test_session_lifetime_checker.py Updates network spec fixture.
tests/unit/manager/sokovan/idle_check/test_network_timeout_checker.py Tests timeout judgments and validation.
tests/unit/manager/sokovan/idle_check/test_handler.py Updates handler fixtures.
tests/unit/manager/repositories/idle_checker/test_repository.py Updates persisted test specification.
tests/unit/manager/dependencies/orchestration/test_sokovan.py Supplies Valkey live dependency.
tests/unit/manager/dependencies/orchestration/test_composer.py Updates orchestration composition tests.
tests/unit/common/clients/valkey_client/test_valkey_live_client.py Tests batched connection counts.
src/ai/backend/manager/sokovan/stages/idle_check.py Registers the network checker.
src/ai/backend/manager/sokovan/stages/factory.py Passes Valkey into the stage.
src/ai/backend/manager/sokovan/idle_check/checkers/network_timeout.py Implements network-idle evaluation.
src/ai/backend/manager/dependencies/orchestration/sokovan.py Adds orchestrator Valkey input.
src/ai/backend/manager/dependencies/orchestration/composer.py Propagates Valkey through orchestration.
src/ai/backend/manager/dependencies/composer.py Supplies infrastructure Valkey client.
src/ai/backend/common/data/idle_checker/types.py Defines network timeout configuration.
src/ai/backend/common/clients/valkey_client/valkey_live/client.py Adds batched connection counting.
changes/12878.feature.md Adds the feature changelog entry.

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

Comment on lines +99 to +101
self._valkey_live.count_active_connections_batch([
str(session_id) for session_id in session_ids
]),
Comment on lines +69 to +70
if state.last_access is None:
continue
Comment on lines +46 to +66
# Fetch session states in one batch to avoid repeated I/O per assignment.
sessions: list[IdleCheckSession] = []
for assignment in assignments:
sessions.extend(assignment.sessions)
states = await self._prepare_states(sessions)

# Judge each assignment in one pass, using the pre-fetched states.
judgments: list[IdleJudgment] = []
for assignment in assignments:
network_spec = assignment.definition.spec.network
if network_spec is None:
log.error(
"Network timeout checker {} has mismatched spec type: {}",
assignment.definition.checker_id,
assignment.definition.spec.type,
)
continue
# Skip sessions with no timeout configured (0 means "never idle").
if network_spec.idle_timeout_seconds == 0:
continue
idle_timeout_seconds = Decimal(network_spec.idle_timeout_seconds)
if state.last_access is None:
continue
current_time = Decimal(str(context.current_time.timestamp()))
idle_seconds = (current_time - state.last_access).normalize()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Are current_time and last_access from the same source?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

"current time" is injected by the idle checker, and "last_access" is the value stored in valkey.

Comment on lines +95 to +100
last_access_values, active_connection_counts = await asyncio.gather(
self._valkey_live.get_multiple_live_data([
f"session.{session_id}.last_access" for session_id in session_ids
]),
self._valkey_live.count_active_connections_batch(session_ids),
)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Are they grouped for concurrent query?

@seedspirit seedspirit Jul 15, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Could have run the queries separately, but I used gather because we thought running them simultaneously would yield slightly higher accuracy. It’s not required, so we can run them separately if you prefer.

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

Labels

comp:common Related to Common component comp:manager Related to Manager component size:L 100~500 LoC

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants