[rfc] cas: cmpxchg interface?

[cyphar]

I have a feeling that issues like #146 (which feel quite fundamental) would be solved much better if we had a more transaction-like interface.

[wking]

On Thu, Nov 09, 2017 at 04:43:16AM +0000, Aleksa Sarai wrote: I have a feeling that issues like #146 (which feel quite fundamental) would be solved much better if we had a more transaction-like interface.

If you're considering a separate publication step (you mentioned ‘parcel export’ earlier [1]), then you can return to per-name ref files (backing out opencontainers/image-spec#533 locally) or use SQLite, or whatever inside an umoci image. That gives you something that's much easier to handle in the face of parallel edits, because you can set things up to only lock the resource you're editing (e.g. what “1.0” resolves to) without locking the entire index. On publication, you can collect up whichever refs are getting published into a new index.json. And I think this parallel, semantically-orthogonal edits issue is really just a problem for the mutable refs. The actual content-addressable blob store does not suffer from this. [1]: https://github.com/openSUSE/umoci/pull/198#issuecomment-337937858

[cyphar]

@wking That's not really what I'm referring to or suggesting. #146 has a more lengthy explanation of why index.json needs a better way of updating it than "just overwrite the current index.json with what I tell you". I don't think that moving away from the OCI format is a good idea -- if there isn't a way to solve the problem within OCI then OCI should be fixed, and if there is a way to solve it within the current spec then we should do it that way. One interface would be something like

var ErrReplaceConflict = errors.New("could not replace index as the 'old' index had changed")
func ReplaceIndex(ctx context.Context, old, new ispec.Index) (err error)

Effectively a cmpxchg-style interface -- if the old contents weren't identical to the previous value then the replacement fails. This would be a bit hard to implement using filesystem operations, but not impossible.

I think transactions would make this sort of style even nicer (and it would also benefit from making cleanup if an operation fails in the middle much simpler). But I think that this would break oci/cas far too much (maybe this is something that could be done as part of oci/casext).

[wking]

...if there isn't a way to solve the problem within OCI then OCI should be fixed...

The racing-updates issue was a known weakness when image-spec switched to index.json. The mitigation was that it's a publication-time format, and you can use something friendlier to parallel writes in development.

func ReplaceIndex(ctx context.Context, old, new ispec.Index) (err error)

This is a good idea. And even with per-ref locks, something like that would still be a good idea. Using per-ref files or SQLite or whatever doesn't uniquely allow you to do that, it just makes the implementation easier.

[wking]

On Thu, Nov 09, 2017 at 02:40:21PM +0000, W. Trevor King wrote: … or SQLite…

And thinking this through a bit further, SQLite is probably not going to help (no concurrent-writer support [1]). Using a different database engine (e.g. PostgreSQL) would, but that's a heavy requirement for this use case. I think flocked (or old/new), per-ref files would be the easiest way to implement this if you want to allow parallel updates to separate refs. A single flocked (or old/new), per-index update (like you're considering) would be the easiest was to implement this if you are comfortable blocking (or erroring) when one client tries to change ${REF1} and another wants to simultaneously change ${REF2}. [1]: https://www.sqlite.org/whentouse.html#dbcklst

[cyphar]

I think blocking for a while in a retry loop (with some sane defaults for amount-of-times-to-retry) would work fine. And personally since there are several "global" properties of index.json I feel that making it a "replace" operation for the entire struct rather than thinking of it as "modifying a single reference in the index" will probably lead to less confusion.

[wking]

On Fri, Nov 10, 2017 at 12:05:52AM +0000, Aleksa Sarai wrote: And personally since there are several "global" properties of `index.json`…

schemaVersion is fixed [1] and mediaType is reserved [2], so I expect you mean annotations [3]. Grepping for ‘\.Annotations’ didn't turn up anything that seemed to be setting that in umoci, but I guess folks using it as a library (and somehow working around the library-vendor-dir issue, e.g. opencontainers/image-spec#527) could be setting those attributes. And if folks complain about a global index lock, we can always revisit this then ;). [1]: https://github.com/opencontainers/image-spec/blame/v1.0.1/image-index.md#L14 [2]: https://github.com/opencontainers/image-spec/blame/v1.0.1/image-index.md#L20 [3]: https://github.com/opencontainers/image-spec/blame/v1.0.1/image-index.md#L88