kvs commit handling should propagate content cache errors back to user

[garlick]

If an error occurs while making an RPC to the content-cache service, it is not handled properly in the following cases:

• store() handles synchronous errors, but async errors are only logged, not propagated to the caller
• load() errors (sync and async) are only logged, not propagated to the caller

Further, after an async cache fill or cache flush error occur, the cache entry can be left dirty or invalid forever.

[garlick]

The description of store() above assumes #788 gets merged.

[garlick]

With PR #801 it becomes possible to put a dangling blobref to the KVS. In addition to managing the errors above, the commit logic should ensure that any new references are loaded into the rank 0 cache to verify that they work. Alternatively we might want to add a content store operation that simply checks if a blobref has been stored without returning it, since one use case for creating such a reference outside the KVS would be to avoid running a large value through the KVS commit logic.

[chu11]

Update here given all the recent changes to the KVS:

store() handles synchronous errors, but async errors are only logged, not propagated to the caller

Still true, but now is in the area of content_store_get().

load() errors (sync and async) are only logged, not propagated to the caller

Synchronous errors are handled now.

Further, after an async cache fill or cache flush error occur, the cache entry can be left dirty or invalid forever.

I believe these error paths have now been handled properly.

[chu11]

Per discussion in #1227 - a good test to add once this is complete is to have a valref with multiple blobs in it contain an illegal blobref.

[garlick]

Do we still have any content errors that are not handled properly? I'm going to tentatively close this as it seems like we've handled them all. @chu11 please reopen if I'm wrong.

[chu11]

Yeah, the asynchronous cases still exist. Basically the replay of a stalled request is still triggered when a value is updated in the local kvs cache. If an error occurs such that the cache is never updated, then the replay of the original request won't ever be triggered to complete.

[chu11]

As noted in #1799, it's not isolated to "content cache" errors that are part of this issue. If a blobref happens to be missing, this is equivalent to a "content cache" error.

With the completion of #1696 / #1318, I think an easy way to fix this is via msg aux_set/aux_get. If an error occurs in asynchronous communications (such as in content_store_get or content_load_completion), use msg aux_set to set some error field of some sort, then on replay check if that error field has been set.

I'll need some new function in the cache API to iterate through waitqueue entries and set the msg aux field, but this seems like a not so horrible path to finally fix this.

[chu11]

doh, the above assumed that all waiters had messages, but that's not always the case. What I really need is to set some type of "error callback" on a wait_t so that when the waiter is replayed it knows to call that callback and do something (call lookup_set_aux_errnum()) before a replay occurs.