Skip to content

fix: acknowledge accepted updates and guard persistence leases - #32

Open
stanley2058 wants to merge 1 commit into
mainfrom
fix/dev-3194-transport-persistence-correctness
Open

stanley2058 wants to merge 1 commit into
mainfrom
fix/dev-3194-transport-persistence-correctness

Conversation

@stanley2058

Copy link
Copy Markdown
Collaborator

This PR fixes two transport correctness issues:

  • sync-update retries could receive an ACK after the original Redis append failed or while it was still pending. Every attempt now appends to Redis and receives an ACK only after successful acceptance. Yjs safely merges duplicate updates.
  • Namespace cleanup could delete another server's persistence lease, and heartbeat renewal used a non-atomic ownership check. Acquisitions now have unique tokens, and Redis scripts atomically check ownership before renewal or release. Concurrent acquisition calls share one pending request; cleanup invalidates late acquisition results. Reconnects cancel cleanup without dropping the current lease, including cleanup callbacks already waiting for persistence.

Adds 15 regression tests covering append failures, concurrent retries, lost ACKs, three server instances sharing Redis, lease expiry and reacquisition, delayed renewal, cleanup races, and a real Socket.IO reconnect. The tests run in CI with Redis.

Validated with the regression suite on Node 18, npm run lint (Standard and TypeScript), and npm run dist.

The storage and worker APIs are unchanged. Lease ownership does not prevent an already-dispatched worker from committing stale state after lease expiry; storage-enforced commit ordering remains separate work. All servers must be upgraded before relying on owner-checked leases because older servers still release leases unconditionally.

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.

1 participant