Skip to content

fix(vehicle): single-flight config writes to stop the batch replay storm - #11

Merged
Bre77 merged 2 commits into
mainfrom
fm/tstream-batch-retry-fix
Jul 27, 2026
Merged

fix(vehicle): single-flight config writes to stop the batch replay storm#11
Bre77 merged 2 commits into
mainfrom
fm/tstream-batch-retry-fix

Conversation

@Bre77

@Bre77 Bre77 commented Jul 26, 2026

Copy link
Copy Markdown
Member

Intent

TeslemetryStreamVehicle.update_config() logged and returned on an error-shaped response ({"response": null, "error": ...}) without clearing the merged pending config. Every already-scheduled listener task then independently replayed the same full PATCH:

  • At integration setup, HA's async_setup_stream() schedules one add_field() task per streaming listener. Each call merges into the shared pending dict, waits out the 1s debounce, and then PATCHes.
  • On a deterministic upstream rejection, the pending dict was left non-empty, so the next waiter saw it as still dirty and sent the identical payload again - a queue of independently scheduled tasks replaying serially, not a bounded retry loop. Production showed 20 identical 80-field PATCHes over ~10 minutes against a per-VIN write latch; nothing in the library bounds how many listeners can queue this way.

TeslemetryStreamVehicle now funnels every caller through one per-vehicle flush (_flush):

  • The first caller to arrive starts the flush; every other caller in the same window merges its fields into the same pending config and awaits that flush instead of starting its own.
  • A body-shaped error is terminal for that batch - it is not replayed, but the desired config is kept in place so the next explicit trigger (a new listener, reconnect, or manual retry) can attempt it again.
  • A transport-level failure (aiohttp.ClientError/timeout) gets one bounded retry inside the same flush before the batch is treated as failed, since that's not the server's deterministic rejection shape.

tests/test_batch_retry_storm.py reproduces the storm with 80 concurrent listener tasks against a failing mock and asserts exactly one PATCH per failed batch (confirmed this fails against main, sending 80 PATCHes, before this fix). tests/test_config_update.py continues to cover the response-shape handling this builds on.

update_config() logged and returned on an error-shaped response without
clearing the merged pending config, so every already-scheduled listener
task (HA's async_setup_stream fans out one add_field task per streaming
entity) independently replayed the identical full PATCH - 80 requested
fields observed replaying serially against a deterministic upstream
rejection.

TeslemetryStreamVehicle now funnels every caller through one per-vehicle
flush: the first caller starts it, later callers merge into the same
pending config and await that flush instead of starting their own. A
body-shaped error is terminal for the batch - it is not replayed, but the
desired config is kept for the next explicit update_config call (a new
listener, reconnect, or manual retry). A transport-level failure gets one
bounded retry inside the same flush before it is treated as failed.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5ef5a76a19

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread teslemetry_stream/vehicle.py Outdated
flight = asyncio.ensure_future(self._flush())
self._flight = flight

await flight

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Shield the shared flush from waiter cancellation

If any concurrent update_config caller is cancelled or wrapped in a timeout during the debounce or PATCH, cancellation propagates through this direct await and cancels the shared task. Every other caller awaiting the same flight then receives CancelledError, and the entire coalesced configuration can remain unapplied; await asyncio.shield(flight) so cancelling one waiter does not abort the per-vehicle single-flight operation.

AGENTS.md reference: AGENTS.md:L11-L11

Useful? React with 👍 / 👎.

A waiter joined the shared per-vehicle flush with a plain await, so
cancelling (or timing out) one caller threw CancelledError into the flush
itself, aborting it for every other concurrent waiter and leaving the
coalesced config unapplied. Join with asyncio.shield instead: the cancelled
caller still observes CancelledError, but the flush it was waiting on keeps
running to completion for the remaining waiters.
@Bre77
Bre77 merged commit 2e630dc into main Jul 27, 2026
15 checks passed
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