envato / event_sourcery

A library for building event sourced applications in Ruby
MIT License
84 stars 10 forks source link

Switch to using a pg_try_advisory_lock in the write events function #104

Closed twe4ked closed 7 years ago

stevehodgkiss commented 7 years ago

Background: The effect (synchronise writes to ensure linear growth of event store sequence IDs) & performance is the same. The benefit of using advisory locks is, unlike a full table lock, they don't conflict with VACUUM which is something RDS postgres runs automatically.

orien commented 7 years ago

Do we have any tests to ensure this behaves as expected?

stevehodgkiss commented 7 years ago

@orien I imagine there aren't any tests for this because it's hard to effectively test. If you have any ideas about how to effectively test that these locks are working as expected I'm all ears :)

The only way I can think of to test that the lock is in place is to run some brute force inserts in parallel, have one thread/process continually selecting the latest 2-3 events of the stream while this is happening and verifying that there are no gaps in the sequence ID's selected (where a gap would indicate an in-flight transaction has decremented the global sequence counter but not yet committed to the table). It's hardly ideal to be running something like that in a test suite.

orien commented 7 years ago

@stevehodgkiss That's somewhat what I was thinking. I think such a script would provide confidence that the lock works as intended. I understand if you'd prefer not to execute on each test run but we could run it manually when updating this code.

This SQL function is rather long and funky especially for developers not accustomed to working in this language/environment, some specific tests for it would be nice.

stevehodgkiss commented 7 years ago

Yeah I'm actually planning to write that script soon to demonstrate the non-linear sequence ID growth issue. When I've done that I'll add it as a separate script that can be invoked when we change this function. I don't want to make it a blocker to this PR though and I'm confident this does what we expect it to.