electric-sql / electric

Sync little subsets of your Postgres data into local apps and services.
https://electric-sql.com
Apache License 2.0
6.48k stars 156 forks source link

Alco/acquire lock in replication client #1850

Closed alco closed 1 month ago

alco commented 1 month ago

PR chain: #1842 ← #1841 ← #1845 ← #1846 ← #​1850

Fixes #1792.

netlify[bot] commented 1 month ago

Deploy Preview for electric-next ready!

Name Link
Latest commit 15d90a567b0e3775c04702513f2005a3a54ee067
Latest deploy log https://app.netlify.com/sites/electric-next/deploys/670da0004e321500087182a4
Deploy Preview https://deploy-preview-1850--electric-next.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

alco commented 1 month ago

After working a bit more on connection process life cycle management in #1841 I've decided to scratch this change. Even though moving the advisory lock acquisition to the replication connection would help us save one idle PG connection, it would rob us of the ability to restart the replication client independently of the other connections, namely, the lock connection and the DB pool.

If we merged this PR, restarting the replication client process (which we do to ensure consistent storaged of streamed transactions by shape consumers) would always necessitate restarting the whole Electric.Connection.Supervisor tree.