mirror of
https://github.com/facebook/sapling.git
synced 2024-10-10 00:45:18 +03:00
c77969df2c
Summary: At the moment our memcache lease implementation has surprising side effect - releasing the lease with `put_success = true` sets a memcache key, which means that until this key is evicted from memcache it's not possible to take this lease again (`try_add_put_lease()` fails with because it uses memcache.add_with_ttl() method, which returns false if a key already exists in memcache). That caused a deadlock in our derived data generation: 1) `derive_may_panic()` function does a few things - it actually derives the data, then releases the lease but it DOES NOT update derived data mapping (mapping is updated later in a batch for performance reasons). 2) So the following situation is possible - data was successfully derived, memcache lease key was updated (hence taking the lease one more time is not possible) but mapping update failed. This is an invalid state - any attempt to get the lease fails, but mapping doesn't exist either, and workers just loop forever. There seems to be a few ways to fix the deadlock, but changing release_lease() logic seems to cause less surprises in future - we expect memcache leases to be a "soft lock", and when a lock was unlocked it should be possible to lock it again. So currently release_lease() checks the value that's set in the memcache key. If this value is LockState::uploaded(...) (which can be set only by MemcacheLease::put()`), then nothing is done to this key, otherwise the key is deleted. this ensure that 1) No changes to the current behaviour of the CachingBlobstore - lease is invalidated on inserting a new value in the cache 2) Deadlock issue is fixed, because release_lease() now deletes the key. Reviewed By: krallin Differential Revision: D19468234 fbshipit-source-id: 85525bc11802a0fd4d94a692bf7c6dd4569a6f46 |
||
---|---|---|
.. | ||
blobsync/src | ||
changeset_fetcher/src | ||
errors/src | ||
factory/src | ||
repo_blobstore/src | ||
src | ||
test |