Skip to content

Do not fail a transient run on a throttled DescribeCluster - #106

Closed
Ian Streeter (istreeter) wants to merge 1 commit into
developfrom
fix/describe-throttling
Closed

Ian Streeter (istreeter) wants to merge 1 commit into
developfrom
fix/describe-throttling

Conversation

@istreeter

Copy link
Copy Markdown
Contributor

Production runs were reported as failed after a single ThrottlingException. None had failed: clusters had come up, or terminated cleanly, and we exited anyway — in the launch cases leaving a cluster running its steps unwatched.

The waiters swallow API errors and keep polling, so the throttled call was never a poll. It was the describe around one: the existence check before it, or the re-describe after it, since waiter.Wait discards its response and the status had to be fetched again. WaitForOutput hands back the response that satisfied the acceptor, so that second call is gone from the happy path entirely; what remains covers only a waiter that errored or timed out, and a TERMINATED that EMR has not yet attached a reason to. If that cannot get through, the status the waiter saw beats reporting an error. When the launch cannot be read at all we now terminate before giving up, rather than abandoning a billing cluster.

EMR reads retry on a budget chosen by the error. A throttle is waited out for minutes, because these reads are how a run learns whether it succeeded and each is issued while a cluster is already up or once it has gone. Everything else gets the few seconds it always had — enough that EMR briefly not recognising a jobflow ID it just issued is survivable, while a genuinely bad --emr-cluster is still reported in seconds instead of sitting in a waiter. The same treatment covers ListSteps and DescribeStep, which sit on the transient success path and could equally turn a completed run into a failed one.

To reduce the pressure in the first place, waiters poll at 30-90s while a cluster launches and 60-300s during a transient job, rather than a fixed 30s: smithy only jitters when the bounds differ, so every runner had been polling on the same beat against a budget shared by the whole account.

The client also moves to adaptive retry mode. Its raised attempt count is passed per read rather than set on the client, because the client is shared with RunJobFlow, which has no idempotency token — retrying a launch whose response was merely lost would create a second cluster and rerun the playbook.

Production runs were reported as failed after a single ThrottlingException.
None had failed: clusters had come up, or terminated cleanly, and we exited
anyway — in the launch cases leaving a cluster running its steps unwatched.

The waiters swallow API errors and keep polling, so the throttled call was never
a poll. It was the describe around one: the existence check before it, or the
re-describe after it, since waiter.Wait discards its response and the status had
to be fetched again. WaitForOutput hands back the response that satisfied the
acceptor, so that second call is gone from the happy path entirely; what remains
covers only a waiter that errored or timed out, and a TERMINATED that EMR has
not yet attached a reason to. When the launch cannot be read at all we now
terminate before giving up, rather than abandoning a billing cluster.

EMR reads retry on a budget chosen by the error. A throttle is waited out for
minutes, because these reads are how a run learns whether it succeeded and each
is issued while a cluster is already up or once it has gone. Everything else
gets the few seconds it always had — enough that EMR briefly not recognising a
jobflow ID it just issued is survivable, while a genuinely bad --emr-cluster is
still reported in seconds instead of sitting in a waiter. The same treatment
covers ListSteps and DescribeStep, which sit on the transient success path and
could equally turn a completed run into a failed one. Where a run still cannot
be classified, the error says the cluster could not be described rather than
only that it had no reason, so throttling is visible as the cause.

To reduce the pressure in the first place, waiters poll at 30-90s while a
cluster launches and 60-300s during a transient job, rather than a fixed 30s:
smithy only jitters when the bounds differ, so every runner had been polling on
the same beat against a budget shared by the whole account.

The client also moves to adaptive retry mode. Its raised attempt count is passed
per read rather than set on the client, because the client is shared with
RunJobFlow, which has no idempotency token — retrying a launch whose response
was merely lost would create a second cluster and rerun the playbook.

Co-Authored-By: Claude Opus 5 (1M context) <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.

1 participant