Re: [RFC/WIP] Pluggable reference backends

Karsten,

Thanks for your feedback!

On 03/11/2014 11:56 AM, Karsten Blees wrote:

I am mostly thinking about long-running processes, like "gc" and "prune-refs", which need to be made race-free without blocking other processes for the whole time they are running (whereas it might be quite tolerable to have them fail or only complete part of their work in any given invocation). Also, I work at GitHub, where we have quite a few repositories, some of which are quite active :-)

Remember that I'm not yet proposing anything like hard-core ACID reference transactions. I'm just clearing the way for various possible changes in reference handling. I listed the ideas only to whet people's appetites and motivate the refactoring, which will take a while before it bears any real fruit.

Gmane is down at the moment but I assume you are referring to my patch series and the ref_transaction implementation therein.

No explicit rollback is necessary at this stage, because the "commit" function first locks all of the references that it wants to change (first verifying that they have the expected values), and then modifies them all. By the time the references are locked, the whole transaction is guaranteed to succeed [1]. If the locks can't all be acquired, then any locks that were obtained are released.

If a caller wants to rollback a transaction, it only needs to free the transaction instead of committing. I should probably make that clearer by renaming free_ref_transaction() to rollback_ref_transaction(). By the time we start implementing other reference backends, that function will of course have to do more. For that matter, maybe create_ref_transaction() should be renamed to begin_ref_transaction(). Now would be a good time for concrete bikeshedding suggestions about function names or other details of the API :-)

Yes, the queue_*() methods should probably later make a preliminary check of the reference's old value and return an error if the expected value is already incorrect. This would allow callers to fail fast if the transaction is doomed to failure. But that wasn't needed yet for the one existing caller, which builds up a transaction and commits it immediately, so I didn't implement it yet. And the early checks would add overhead for this caller, so maybe they should be optional anyway. Maybe these functions should already be declared to return an error status, but there should be an option passed to create_ref_transaction() that selects whether fast checks should be performed or not for that transaction.

Really, all that this first patch series does is put a different API around the mechanism that was already there, in update_refs(). There will be a lot more steps before we see anything approaching real reference transactions. But I think your (implied) suggestion, to make the API more reminiscent of something like database transactions, is a good one and I will work on it.

Cheers, Michael

[1] "Guaranteed" here is of course relative. The commit could still fail due to the process being killed, disk errors, etc. But it can't fail due to lock contention with another git process.