Skip to content

fix: give the attach path the same 10 minutes as session creation - #200

Open
ajma wants to merge 1 commit into
mainfrom
fix/session-available-timeout
Open

ajma wants to merge 1 commit into
mainfrom
fix/session-available-timeout

Conversation

@ajma

@ajma ajma commented Sep 24, 2026

Copy link
Copy Markdown
Contributor

What

_wait_for_session_available now defaults to a 600s budget instead of 300s.

Why

getOrCreate has two paths that each wait for a session to become usable, and their budgets had drifted apart:

  • Create (__create) polls the long-running operation returned by create_session, with timeout=600 on the polling retry.
  • Attach (_get_exiting_active_session) has no operation handle to poll, so it hand-rolls a GetSession loop waiting for the "Spark Connect Server" key to appear in runtime_info.endpoints — and that loop allowed only 300s.

Both are waiting on the same underlying thing: a server finishing its boot. Because get_active_s8s_session_response treats CREATING as reusable, the attach path can land on a session that hasn't started yet and then give up at half the budget the create path would have allowed. The most common way to hit this is passing a custom session ID that resolves to a session someone else just started.

Raising the default to 600s means a caller gets the same 10 minutes either way.

Not addressed here

The loop only evaluates its deadline between iterations, and get_session is issued with no per-call timeout (the Dataproc GAPIC sets default_timeout=None for every method). So the wall-clock can still overshoot the stated budget by the duration of a stalled final call — it will now read "after 600 seconds" while taking longer. Bounding that properly means threading the remaining budget into the get_session call; left for a separate change. _wait_for_termination has the same shape with a 180s budget.

Testing

pytest tests/unit/test_session.py -k wait_for_session_available passes (both tests pass an explicit timeout, so neither depended on the default). pyink --check clean.

getOrCreate has two paths that each wait for a session to become
usable, and their budgets had drifted apart. Creating a new session
polls the long-running operation for up to 600s. Attaching to a
session that already exists cannot use that machinery -- there is no
operation handle -- so it hand-rolls a GetSession poll loop waiting
for the Spark Connect endpoint to appear, and that loop allowed only
300s.

Both are waiting on the same thing: a server finishing its boot.
Since get_active_s8s_session_response treats CREATING as reusable,
the attach path can land on a session that has not started yet and
then give up at half the budget the create path would have allowed.
Raise it to 600s so a caller gets the same 10 minutes either way.

@gemini-code-assist gemini-code-assist Bot 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.

Code Review

This pull request increases the default timeout in the _wait_for_session_available method from 300 to 600 seconds in google/cloud/managed_spark_connect/session.py. There are no review comments, and I have no feedback to provide.

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.

1 participant