Skip to content

A few river_notification improvements for SQLite - #1381

Merged
brandur merged 1 commit into
masterfrom
brandur-river-notification-improvements
Sep 25, 2026
Merged

brandur merged 1 commit into
masterfrom
brandur-river-notification-improvements

Conversation

@brandur

@brandur brandur commented Sep 24, 2026

Copy link
Copy Markdown
Contributor

Here, a few minor improvements for river_notification inspired
somewhat by looking at Active Cable's notifications handling:

  • Per-topic subscription cursors: Each topic gets its own cursor so that
    we avoid replaying old notifications in case a client unsubscribes
    from a topic and later resubscribes.

  • Notification fetches can read up to 256 notifications at a time
    (previously we'd only fetch them one at a time).

  • Cleanup now deletes 10,000 rows (same number as job cleaner + queue
    cleaner) per batch instead of trying to do all rows beyond a delete
    horizon. This protects against degenerate cases where some kind of
    enormous backlog has built up.

I honestly thought we were doing the second two already, but apparently
not so it's good that we revisited this.

Here, a few minor improvements for `river_notification` inspired
somewhat by looking at Active Cable's notifications handling:

* Per-topic subscription cursors: Each topic gets its own cursor so that
  we avoid replaying old notifications in case a client unsubscribes
  from a topic and later resubscribes.

* Notification fetches can read up to 256 notifications at a time
  (previously we'd only fetch them one at a time).

* Cleanup now deletes 10,000 rows (same number as job cleaner + queue
  cleaner) per batch instead of trying to do all rows beyond a delete
  horizon. This protects against degenerate cases where some kind of
  enormous backlog has built up.

I honestly thought we were doing the second two already, but apparently
not so it's good that we revisited this.
@brandur
brandur force-pushed the brandur-river-notification-improvements branch from 25ee80c to 139b11e Compare September 24, 2026 20:35
@brandur
brandur requested a review from bgentry September 24, 2026 20:44
@bgentry

bgentry commented Sep 25, 2026

Copy link
Copy Markdown
Contributor

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

To use Codex here, create a Codex account and connect to github.

@bgentry

bgentry commented Sep 25, 2026

Copy link
Copy Markdown
Contributor

@codex review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 25, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-25T02:56:50.026946Z 139b11e Manual request
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Already looking forward to the next diff.

Reviewed commit: 139b11ef03

ℹ️ 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".

@brandur

brandur commented Sep 25, 2026

Copy link
Copy Markdown
Contributor Author

thx.

@brandur
brandur merged commit 52c1763 into master Sep 25, 2026
15 checks passed
@brandur
brandur deleted the brandur-river-notification-improvements branch September 25, 2026 19:49
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.

2 participants