Skip to content

fix(log-stream): replace an overdue alarm instead of trusting it - #260

Merged
alukach merged 1 commit into
mainfrom
fix/log-stream-overdue-alarm
Oct 20, 2026
Merged

alukach merged 1 commit into
mainfrom
fix/log-stream-overdue-alarm

Conversation

@alukach

@alukach alukach commented Oct 20, 2026

Copy link
Copy Markdown
Contributor

Follow-up to #257. That PR made storage the only record of a pending tick alarm, re-arming when getAlarm() returns null. This covers the other way the alarm can be stuck: storage still holds an alarm, but it is long past due and never fires. Then getAlarm() stays non-null, ensureAlarm never re-arms, and the globe stays silent even after a restart clears in-memory state.

The evidence for that case: on 2026-10-20 production was rolled back to b0af0f99 and forward to 912faa3f about 40 seconds apart, and Durable Object analytics show no alarm invocation since (as of 17:10 UTC); a fresh wss://logs.source.coop/ws still receives nothing. No invocation was recorded on b0af0f99 and the open WebSockets weren't dropped, so the object may simply never have restarted — which is why this is a guard rather than a confirmed diagnosis.

ensureAlarm now treats an alarm more than ten emit intervals (5s at the production EMIT_INTERVAL_MS) overdue as dead and overwrites it with setAlarm. A healthy alarm is never more than one interval out, so the guard costs nothing in normal operation.

Shipping this in the same release as #257 means one release resolves production whichever way the alarm is stuck.

Testing

  • tsc --noEmit passes; node --test src/aggregate.test.mjs passes 3/3; the pre-push hook's Rust suite passes.
  • Not reproducible locally. After the release: open wss://logs.source.coop/ws and expect tick frames within a second, and confirm alarm invocations resume in the Durable Object analytics for public-log-stream.

No ADR covers the log stream's internals; nothing to update in adrs/.

🤖 Generated with Claude Code

Rolling prod back and forward on 2026-10-20 left the feed silent, with no alarm invocations afterwards. Whether or not that rollback restarted the object, a stored alarm that is well past due and never fires would keep getAlarm() non-null, and ensureAlarm would never re-arm. Treat an alarm more than ten emit intervals overdue as dead and overwrite it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@claude

claude Bot commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @alukach's task in 16s —— View job


✅ No blocking issues — safe to merge.

I read ensureAlarm in workers/public-log-stream/src/location-hub.ts:90 and its two callers (handleWebSocket and handleLocation). I didn't run anything.

  • Correctness: The guard is sound. getAlarm() returns null or a future or recent timestamp for a healthy alarm. A healthy alarm is never more than one interval overdue, so the 10× threshold won't misfire.
  • Overwriting a live alarm: Calling setAlarm over an existing alarm replaces it. It doesn't stack a second alarm, so two tick loops can't run.
  • Alarm retries: If alarm() throws, the runtime retries it with backoff. A retry-pending alarm could be replaced once it is more than 10 intervals overdue. That is harmless, because the replacement just resumes ticking.
  • Cost: Each ensureAlarm call already did a getAlarm() read, so the extra comparison adds nothing measurable.
  • Not verified: As the PR says, this is a guard and not a confirmed diagnosis. If the alarm is still silent after the release, the cause is elsewhere. The post-release check you describe (tick frames on the WebSocket and alarm invocations in the Durable Object analytics) is the right way to confirm.

Simplify (ponytail)

  • None. The change is one condition, with no new abstractions.

💰 Estimated review cost: $0.10 · 0m16s · 5 turns

@github-actions

Copy link
Copy Markdown

🚀 Latest commit deployed to https://source-data-proxy-pr-260.source-coop.workers.dev

  • Date: 2026-10-20T17:17:37Z
  • Commit: c874f16

@alukach
alukach marked this pull request as ready for review October 20, 2026 19:08
@alukach
alukach merged commit a12589a into main Oct 20, 2026
23 checks passed
@alukach
alukach deleted the fix/log-stream-overdue-alarm branch October 20, 2026 19:08
alukach pushed a commit that referenced this pull request Oct 20, 2026
🤖 I have created a release *beep* *boop*
---


##
[2.5.1](v2.5.0...v2.5.1)
(2026-10-20)


### Bug Fixes

* **log-stream:** replace an overdue alarm instead of trusting it
([#260](#260))
([a12589a](a12589a))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: source-release-bot[bot] <265100246+source-release-bot[bot]@users.noreply.github.com>

This branch was successfully deployed

1 active deployment
preview — ae2fc882 Deployed Oct 20, 2026 by alukach via Deploy & Test / Deploy #437
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant