RFC: Create an “incomplete” layer record prior to invoking the graph driver

[mtrmac]

Currently, layerStore.Put first invokes the graph driver to create a layer, and only afterwards it creates a layers.json entry (possibly an “incomplete” one if the layer is created from a tarball).

So, crashing/aborting layerStore.Put before a layers.json entry is committed to disk can result in a (possibly incomplete) graph driver layer object that we don’t have any record of, and we might well tell the graph driver to create a layer with the same ID (which can then result in unexpected situations).

With Kubernetes-colored glasses on, it seems elegant to me to have a “single source of truth”, so that if layers.json and the graph driver state get out of sync, we only ever do corrections in one direction. Considering the existing incompleteFlag logic that treats layers.json as authoritative (and also the fact that layers.json writing is taking more care to be atomic than the overlay graph driver, at least), it seems natural to do the same in layerStore.Put. so:

Proposal: In layerStore.Put, always create a Layer object with incompleteFlag, save it to layers.json, and only afterwards invoke the graph driver to create the layer.

This would be costless for layers created from a tarball (we would only change the timing of the save of the incomplete data), and add one more layerStore.Save (and a fdatasync) for read/write layers.

If creating the layer is aborted for any reason, the incompleteFlag set would cause the layer to be cleaned up in the future (but note #1136 ).

Driver.Remove implementations would still need to be fairly resilient against incomplete/corrupt layers, but that’s, to an extent, already the case today.

---

Alternatively, we could require Driver.Create* to be idempotent / resilient against pre-existing/partial layers with the same ID.

[mtrmac]

Alternatively, we could require Driver.Create* to be idempotent / resilient against pre-existing/partial layers with the same ID.

#1140 is a step in that direction.

[mtrmac]

… and #1148 implements the suggestion from this issue.

I tend to think that that #1140 is a short-term backportable workaround, and #1148 is closer to a long-term fix, but I really don’t know what I don’t know, so I’d love the experts to review and decide.

[mtrmac]

#1148 was merged.