SynchronizePrimitive class marked private instead of abstract

[bjeanes]

Currently, the public interface for the synchronization primitives is the #synchronize method, which is blocking.

If using the private interface, one can use enter(timeout: x, wait: y) to put an upper bound on lock wait time, or use try_enter to avoid blocking altogether.

Ideally, these would be exposed in the public interface, such as:

sync = Moneta::Mutex.new(...) # or Moneta::Semaphore.new(...)

acquired = sync.synchronize(block: false) do
  # Don't run but don't block either
end

acquired = sync.synchronize(timeout: 5) do
  # Block for at most 5 seconds
end

Alternatively, making parts of the internal API part of the public interface would be sufficient (for my purposes). I'm using the private API anyway but it'd be nice to have a stronger compatibility guarantee for my use case, where I want to execute an alternate codepath if the lock is not immediately available.

I don't actually love adding a block parameter to synchronize, so maybe this is a different method altogether, like try_synchronize?

[asppsa]

I just want to flag that #enter and friends are not private.

However, I think this sounds like a good idea -- it should be possible to set both the timeout and wait params via #synchronize. I think simplest for now would just be to change #synchronize to have the same signature as #enter though, so I don't favour the block parameter. I like your idea of having try_synchronize for this though.

[bjeanes]

I just want to flag that #enter and friends are not private.

Hmmm, I see now. The Moneta::SynchronizePrimitive class is marked private https://github.com/minad/moneta/blob/53518f6e1eb4eb22826b0514f47275228c4b0d39/lib/moneta/synchronize.rb#L1-L4 but because the documentation for the inherited methods is only on that page in the rubydoc if the warning isn't taken literally it can imply the actual methods are private. But of course, they are literally public on the public classes, so that puts my mind at ease.

I think simplest for now would just be to change #synchronize to have the same signature as #enter though

I thought about this too. I think the actual implementation of #enter would have to change to support a "never block" use case, though, as it doesn't look like #try_enter would run if the timeout param were set to 0 https://github.com/minad/moneta/blob/53518f6e1eb4eb22826b0514f47275228c4b0d39/lib/moneta/synchronize.rb#L33-L40

[asppsa]

Yep, so I think that @api private line should be removed. It was probably (incorrectly) marked private because that class is not meant to be used independently. It should be marked @abstract instead, if I remember my schooling ...

Concerning #synchronize, I've now changed my mind, and I think the current behaviour is correct: the return value of calling #synchronize is at present guaranteed to be the return value of the block that is passed in. If there are cases where the block doesn't get run (i.e. timeout or non-blocking), this guarantee is lost (unless we were to throw an exception perhaps). To avoid significantly altering the semantics of #synchronize, I think for now it would be best to leave it with no timeout & blocking semantics (the wait param could be added though perhaps). So people who want to do non-blocking should use try_enter & leave instead of synchronize.

On the other hand, I think there is merit in considering a complete overhaul of the mutex API for some future Moneta 2.0.0 release. One thing that I find problematic is apps that crash while holding a lock. In such cases, there is no way other than calling the private leave_primitive (or a manual op on the underlying store) to release it.

I also think there should be a facility to have a mutex that uses Moneta's expiry functionality, with the ability to extend the lock, like is done in redlock. The first part can be done at present with the a store that has a a default expiry, but the extending part is not available.

[bjeanes]

I also think there should be a facility to have a mutex that uses Moneta's expiry functionality, with the ability to extend the lock, like is done in redlock. The first part can be done at present with the a store that has a a default expiry, but the extending part is not available.

💯. Suo also does this. I haven't used it but I am currently implementing some "flow control" for background jobs (a la activejob-traffic_control and activejob-locking) based on Moneta, as I don't want to add Suo as a dependency when I already have Moneta.

[asppsa]

I've changed the description of this issue to reflect the documentation problem. We can continue the discussion of overhauling the Mutex API in #156.