Skip to content

Fix and simplify alarm logic#14

Merged
lukevalenta merged 4 commits intomainfrom
lvalenta/fix-batch
Apr 9, 2025
Merged

Fix and simplify alarm logic#14
lukevalenta merged 4 commits intomainfrom
lvalenta/fix-batch

Conversation

@lukevalenta
Copy link
Copy Markdown
Contributor

@lukevalenta lukevalenta self-assigned this Apr 9, 2025
Rather than track when the alarm was last fired, set it when the first
entry is added to a batch, and delete the alarm if the batch is
submitted before the timeout. Previously, there was a bug where we could
get unflushed batches as a new alarm wasn't being set in the
already-initialized DO.

fixes #9, fixes #11
@thibmeu thibmeu self-requested a review April 9, 2025 07:27
Copy link
Copy Markdown
Contributor

@thibmeu thibmeu left a comment

Choose a reason for hiding this comment

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

this is simpler indeed.

It's possible that when we call the async 'submit_batch' function
execution yields to the runtime immediately before the 'take' occurs to
replace the batch. Then, two concurrent 'submit_batch' calls could
result in a batch that is never submitted.

I'm not sure at which point execution of the async 'submit_batch' yields
to the runtime, but this change shouldn't hurt, at least.
This is closer to the example at
https://developers.cloudflare.com/durable-objects/examples/alarms-api/,
and will handle cases where we fail to reset the batch when the alarm is
reset.
@lukevalenta
Copy link
Copy Markdown
Contributor Author

After running for some time with the first two commits in this PR, I saw the below errors. It looks like the the Batchers were running into a memory limit, perhaps if the alarm wasn't correctly flushing batches.

POST https://static-ct.cloudflareresearch.com/logs/cftest2025h2a/ct/v1/add-chain - Ok @ 4/9/2025, 9:17:21 AM
  (warn) Internal error: Error: CompileError: memory access out of bounds - Cause: CompileError: memory access out of bounds
POST http://fake_url.com/add_leaf?name=cftest2025h2a - Exception Thrown @ 4/9/2025, 9:17:23 AM
✘ [ERROR]   Error: memory access out of bounds

The two commits I just pushed seem to resolve this issue, but I'll keep monitoring.

@lukevalenta lukevalenta merged commit f249068 into main Apr 9, 2025
@lukevalenta lukevalenta deleted the lvalenta/fix-batch branch April 9, 2025 14:34
rozbb pushed a commit to rozbb/azul that referenced this pull request Jun 3, 2025
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.

Unicode not displayed correctly in checkpoints served from Worker Client timeouts and internal server errors 500 internal server errors

2 participants