grain.social/grain

Switch repository

Loading…

What changes

Notifications are pushed. A record a live path puts new in the index is pushed to whoever it notifies, on their devices. The stream (on Applied::Created), indexRecords and the web's writes call Index::arrived(uri), which hands the URI to the sender's queue (bounded at 10 000; past that, and once it stops, a record is logged and not pushed). Backfill, resync and the import do not call it, nor does an edit, a redelivery (Unchanged/Stale) or a delete.

The sender (push::send), a task started at boot (Settings::push, when GRAIN_APNS_KEY_FILE or GRAIN_FCM_KEY_FILE is set, which needs GRAIN_INDEX), takes each URI in turn:

• leaves it be unless its createdAt is within the hour, or later (a clock set ahead); one with none, or not a time, is not pushed;
• INSERT OR IGNOREs it into push_record (V9), and leaves it be if it was there, so a record is pushed at most once; rows go after a day;
• finds whom it notifies with notification::arising, which already holds the moderation: the author shown (no admin takedown, host not saying it is gone), neither blocking the other, not muted by the recipient, no hiding label on the record or by its author, and the recipient's push preference for the kind (off, or followed accounts only);
• makes a Message for each: the title and body as before, the author's profile name or "Someone", type, the uri (or did for a follow) a tap goes to, and the recipient's unread count as the badge;
• pushes it to each of the recipient's devices on a task of its own, by APNs or FCM (previous pull), trying one that may go again up to four times in all, waiting 2, 4 then 8 seconds; a device the service says is gone is dropped for every account that had it.

Why

The last of the push steps on feature/rust, stacked on registerPush and the transports. Pushes are found and sent off the path that indexed the record: the stream and the requests never wait on Apple or Google.

push_record makes "at most once" hold across restarts and across the three paths: a web write indexes the record, and Jetstream's copy of it is Unchanged; a record deleted and made again at the same key is Created twice, and pushed once.

Differences from the old server:

• Mentions are pushed: gallery-mention ("Bob mentioned you in a gallery", uri the gallery) and gallery-comment-mention ("Bob mentioned you in a comment", uri the gallery or story commented on), as arising finds them and the mentions push preference governs. The old server only pushed favorites, follows, comments and replies. A comment on your own gallery that also mentions you is pushed as the mention.
• Its push types are as before: gallery-favorite, story-favorite, comment-favorite (uri the comment), follow (did the follower), comment-reply (uri the gallery), gallery-comment, story-comment.
• Backfill is told apart by path, not only by age; the hour's recency still applies to every path, as before, so an account moving its repo to a new host does not push its history.
• The badge is the unread count with the new record in it; the old server added one to a count that, by then, already held it.
• Moderation is arising's, which also leaves out an author whose host says they are deactivated or gone, and records a label hides; the old hooks checked admin takedowns, blocks and mutes only.
• Retries, and a queue; the old server sent once from the hook.

How it was checked

Unit tests in src/push/tests.rs: what each kind of record pushes, to whom, with what words, type, uri/did and badge; once only; old and future records; records that notify no one, or that the index does not keep; a push preference off, a mute, a block and a takedown each pushing nothing; the sender end to end against stand-ins for APNs and FCM: one record to an iOS and an Android device, a web device sent nothing, a 503 tried again and a 410 dropping the device, three 500s tried three times and no more, a 400 not tried again; the stream saying a created record and not a redelivered one. tests/index_records.rs and tests/write.rs now check that indexRecords and the web's writes say what they made new and not what they edited. Settings::push refuses push without an index or with a key it cannot read. The sender's queries are in the plan check. The rest is left to the runner.

Shortcomings

• The queue is in memory: what is waiting when the server stops is not pushed after it starts (it is in push_record only once it is being sent).
• A push to a device whose retries run out is dropped, logged.
• Not tried against Apple's or Google's real services.
• The apps call the old method names until they move to registerPush (first pull), so no device is registered with grain-serve before then.
• No web push, as before.

Review

Start at push/send.rs (pushes, then start/deliver), then the three arrived calls. This pull's own change is its last commit; the ones before are the registerPush and transport pulls it stacks on.

🤖 Generated with Claude Code

This pull adds the push sender. Live paths call Index::arrived(uri), which queues the URI. A task then checks the record is recent, records it in push_record so it is pushed at most once, finds recipients with notification::arising, and sends to each device with retries.

I read push/send.rs and Index::arrived. I did not read the diff in full, because the stacked pulls make it very large. I did not check the three arrived call sites or the tests, and I did not look at the checks.

I found nothing that must change.

• The recency check handles a missing or unparseable createdAt correctly, because NULL leads to no push.
• The INSERT OR IGNORE guard comes before recipient lookup, so redelivery and delete-then-recreate push once.
• The retry schedule is 4 tries with waits of 2, 4 and 8 seconds, as described.
• Gone drops the token, and Failed is not retried.

One minor point, not blocking: the sender task holds the index's std::sync::Mutex while it runs pushes, which writes to push_record and runs several queries. This is brief, but it runs on a tokio worker and blocks writers meanwhile. Moving it to spawn_blocking would be tidier if the queue gets deep.

The new commit does what I suggested last round. The sender now runs pushes inside spawn_blocking, so the index's mutex is no longer held on a runtime thread. The closure owns clones of the Arc<Index> and the URI, so it type-checks and nothing else changes. Error handling after the call is the same as before.

I read only this commit's diff and did not re-read the rest. The earlier findings still stand: nothing else needs to change.

One small point, not blocking: .expect("finding pushes does not panic") ends the sender task if the closure ever panics, for example on a poisoned lock. Pushes would then stop silently until the queue filled. A panic inside the old inline call would also have ended the task, so this is no worse than before. Logging the JoinError and continuing to the next URI would be more robust.

Taken up in c3d387d: the sender finds what to push in spawn_blocking, so the index lock is held off the runtime's worker threads. On the parts not read: the three arrived call sites are jetstream::apply (Applied::Created only), sync::read_back and write::keep; tests/index_records.rs and tests/write.rs assert each says a created record and not an edited or redelivered one, and push::tests::the_stream_says_what_is_new covers the stream. This diff will shrink to its own commits once the registerPush and transport pulls land.

Since my last round, the head gained only a merge of push/transports (itself merged with feature/rust) into push/send. The sender code I reviewed is unchanged, and the spawn_blocking change I asked for earlier is still in. I did not re-read the merged-in changes, which belong to the pulls this one stacks on. I found nothing new that must change.

The earlier non-blocking suggestion still stands: log a JoinError from the pushes task and continue, rather than ending the sender with .expect.

The new commit does what I suggested earlier. A panic while finding a record's pushes is now caught inside the index.read closure, logged, and the sender moves on to the next URI. Catching it inside the closure means the unwind never passes through the index's lock guard, so the lock is not poisoned. A JoinError from the blocking task is logged and skipped as well. The .expect is gone.

start_finding takes the lookup as a fn pointer so a test can inject a panic. start passes the real pushes, so production behavior is unchanged. The new test panics on the first record and checks that the next record is pushed. It also checks that the panicking record is not pushed and that the index is still usable afterwards.

One note, not blocking. If the panic comes after INSERT OR IGNORE into push_record, that record is marked sent and never retried. That fits "logged and left" and the at-most-once guarantee.

I found nothing that must change.