Skip to content

Conversation

@borsboom
Copy link
Member

@borsboom borsboom commented Dec 6, 2025

This uses a PostgreSQL trigger on the blocks table to send a pg_notify notification whenever a record is added. A listener in kolme-store receives these notifications and triggers a Trigger. In kolme, a subscriber to that Trigger calls resync

@borsboom
Copy link
Member Author

borsboom commented Dec 6, 2025

I had to do some contortions to get kolme-store to be able to trigger a Trigger, since Trigger is defined in the kolme crate, so kolme-store doesn't have direct access to it. The code could be simplified if Trigger were moved somewhere that kolme-store can use it directly. Would it make sense to move it to the shared crate?

@borsboom
Copy link
Member Author

borsboom commented Dec 6, 2025

Currently there can be spurious remote data notifications (and therefore resyncs) due to own-writes or if the pg_notify listener has to reconnect. This could be prevented in kolme-store by keeping track of the latest local write height and skipping the notification if it matches the new record's height payload in the pg_notify event.

@borsboom
Copy link
Member Author

borsboom commented Dec 6, 2025

For testing, I modified the multiple_processors test to remove the explicit resync and instead delay briefly to give the auto-resync a chance to happen. The modified test fails if I disable the auto-resync. Is this good enough, or would you prefer a separate test for this?

@borsboom borsboom requested a review from snoyberg December 6, 2025 14:41
@snoyberg
Copy link
Member

snoyberg commented Dec 7, 2025

The modified test fails if I disable the auto-resync. Is this good enough, or would you prefer a separate test for this?

Oh, that's perfect! Let me rereview with an approval, I'd just commented before because I hadn't realized you'd already demonstrated this is working as expected. Nice!

@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Dec 11, 2025

Deploying kolme with  Cloudflare Pages  Cloudflare Pages

Latest commit: 3a45135
Status: ✅  Deploy successful!
Preview URL: https://90476af2.kolme.pages.dev
Branch Preview URL: https://kol-67-postgres-resync-on-de.kolme.pages.dev

View logs

@borsboom
Copy link
Member Author

Along with the other changes, I also put in place some logic to prevent most spurious new remote data notification by having the Postgres store keep track of the height of the last block it inserted, therefore being able to detect when it receives a pg_notify event for a local block and skipping the notification.

}
Err(err) => {
tracing::error!("PostgreSQL insert block notification listener error: {err:?}");
tokio::time::sleep(tokio::time::Duration::from_secs(2)).await;
Copy link
Member

Choose a reason for hiding this comment

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

The choice to sleep here could use a comment explaining motivation. It's a bit surprising to me to see it here TBH.

Copy link
Member Author

Choose a reason for hiding this comment

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

If we were in a situation where recv() immediately returns with an error repeatedly, we don't want to be in a situtation where resync is repeated in a tight loop. I picked two seconds because that's what you had in six-sigma-kolme's resync_loop.

Taking another look at this does make me realize that this logic should move to kolme::init_remote_data_listener, both because this kind of decision should be closer to where the resync is called, and because the one second timeout there means this two second sleep here will never finish anyway.

@borsboom borsboom force-pushed the kol-67-postgres-resync-on-demand branch from 25fbaac to 97f9a74 Compare December 12, 2025 16:58
@borsboom borsboom merged commit ebd1af2 into main Dec 22, 2025
3 checks passed
@borsboom borsboom deleted the kol-67-postgres-resync-on-demand branch December 22, 2025 15:36
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.

3 participants