fix: detect cache write failure for MemcacheCache

[chrisrossi]

Fixes #656

[justinkwaugh]

These changes look good to me, apart from the typo

[justinkwaugh]

typo: propogate->propagate

[justinkwaugh]

@chrisrossi I think I found another issue

if you change the futures property of _GlobalCacheSetBatch to a dict, then if there are duplicates (i.e. a set for the same key gets batched more than once) then you will end up with an unfinished future, as it will never get set in the done_callback. Probably that needs to be a list of tuples of (key, future) instead

[chrisrossi]

@chrisrossi I think I found another issue

if you change the futures property of _GlobalCacheSetBatch to a dict, then if there are duplicates (i.e. a set for the same key gets batched more than once) then you will end up with an unfinished future, as it will never get set in the done_callback. Probably that needs to be a list of tuples of (key, future) instead

Yeah, that's a good point. I'm trying to think through why the same key would get set twice and what the behavior should be if the values differ. Does the last one "win"? Do we raise an error? When/if not raising an error, we can probably hand the same future back to each caller. So it's maybe less that we need to change data structure, and more we need to add code to handle duplicates in add.

[justinkwaugh]

@chrisrossi I think I found another issue
if you change the futures property of _GlobalCacheSetBatch to a dict, then if there are duplicates (i.e. a set for the same key gets batched more than once) then you will end up with an unfinished future, as it will never get set in the done_callback. Probably that needs to be a list of tuples of (key, future) instead

Yeah, that's a good point. I'm trying to think through why the same key would get set twice and what the behavior should be if the values differ. Does the last one "win"? Do we raise an error? When/if not raising an error, we can probably hand the same future back to each caller. So it's maybe less that we need to change data structure, and more we need to add code to handle duplicates in add.

Sure, that is true. No problem with returning the same future to multiple callers, the problem is in creating new futures for existing keys. Regarding who wins etc, I don't have a strong sense for the lifecycle of a batch. The main problem would be if a set of a model and a set of a lock value were attempted in the same batch, but I'm hoping that can't happen. If not, then multiple callers trying to set for the same key should be satisfied by a single success or failure. If it can happen, then there are definitely bigger problems. Now that I think about it though, I'm remembering that read threads use cas to set model values so they would not be a problem. Their setting of the lock value is problematic though, and I think should be changed per #651 to an add operation for read threads.

The reason I suggested the tuple is because for my own fix for #651, which involves adding a memcache add operation for read thread locking. That one does care about having only one winner for duplicates, with the rest returning false. For those I have a list of tuples of the form (key, future, is_duplicate) and any that have is_duplicate == True get assigned a result of False in done_callback for that batch operation.