Be explicit about how we ignore errors using trucindex.TruncIndex

This does not change behavior, it just makes it clearer
what is going on.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
This commit is contained in:
Miloslav Trmač 2022-09-27 20:24:55 +02:00
parent fb9a8eaf77
commit d1de06c839
4 changed files with 28 additions and 12 deletions

View File

@ -221,7 +221,7 @@ func (r *containerStore) Load() error {
}
}
r.containers = containers
r.idindex = truncindex.NewTruncIndex(idlist)
r.idindex = truncindex.NewTruncIndex(idlist) // Invalid values in idlist are ignored: they are not a reason to refuse processing the whole store.
r.byid = ids
r.bylayer = layers
r.byname = names
@ -354,7 +354,9 @@ func (r *containerStore) Create(id string, names []string, image, layer, metadat
}
r.containers = append(r.containers, container)
r.byid[id] = container
r.idindex.Add(id)
// This can only fail on duplicate IDs, which shouldnt happen — and in that case the index is already in the desired state anyway.
// Implementing recovery from an unlikely and unimportant failure here would be too risky.
_ = r.idindex.Add(id)
r.bylayer[layer] = container
for _, name := range names {
r.byname[name] = container
@ -434,7 +436,9 @@ func (r *containerStore) Delete(id string) error {
}
}
delete(r.byid, id)
r.idindex.Delete(id)
// This can only fail if the ID is already missing, which shouldnt happen — and in that case the index is already in the desired state anyway.
// The stores Delete method is used on various paths to recover from failures, so this should be robust against partially missing data.
_ = r.idindex.Delete(id)
delete(r.bylayer, container.LayerID)
for _, name := range container.Names {
delete(r.byname, name)

View File

@ -299,7 +299,7 @@ func (r *imageStore) Load() error {
return ErrDuplicateImageNames
}
r.images = images
r.idindex = truncindex.NewTruncIndex(idlist)
r.idindex = truncindex.NewTruncIndex(idlist) // Invalid values in idlist are ignored: they are not a reason to refuse processing the whole store.
r.byid = ids
r.byname = names
r.bydigest = digests
@ -455,7 +455,9 @@ func (r *imageStore) Create(id string, names []string, layer, metadata string, c
return nil, fmt.Errorf("validating digests for new image: %w", err)
}
r.images = append(r.images, image)
r.idindex.Add(id)
// This can only fail on duplicate IDs, which shouldnt happen — and in that case the index is already in the desired state anyway.
// Implementing recovery from an unlikely and unimportant failure here would be too risky.
_ = r.idindex.Add(id)
r.byid[id] = image
for _, name := range names {
r.byname[name] = image
@ -572,7 +574,9 @@ func (r *imageStore) Delete(id string) error {
}
}
delete(r.byid, id)
r.idindex.Delete(id)
// This can only fail if the ID is already missing, which shouldnt happen — and in that case the index is already in the desired state anyway.
// The stores Delete method is used on various paths to recover from failures, so this should be robust against partially missing data.
_ = r.idindex.Delete(id)
for _, name := range image.Names {
delete(r.byname, name)
}

View File

@ -393,7 +393,7 @@ func (r *layerStore) Load() error {
return ErrDuplicateLayerNames
}
r.layers = layers
r.idindex = truncindex.NewTruncIndex(idlist)
r.idindex = truncindex.NewTruncIndex(idlist) // Invalid values in idlist are ignored: they are not a reason to refuse processing the whole store.
r.byid = ids
r.byname = names
r.bycompressedsum = compressedsums
@ -685,7 +685,9 @@ func (r *layerStore) PutAdditionalLayer(id string, parentLayer *Layer, names []s
// TODO: check if necessary fields are filled
r.layers = append(r.layers, layer)
r.idindex.Add(id)
// This can only fail on duplicate IDs, which shouldnt happen — and in that case the index is already in the desired state anyway.
// Implementing recovery from an unlikely and unimportant failure here would be too risky.
_ = r.idindex.Add(id)
r.byid[id] = layer
for _, name := range names { // names got from the additional layer store won't be used
r.byname[name] = layer
@ -795,7 +797,9 @@ func (r *layerStore) Put(id string, parentLayer *Layer, names []string, mountLab
BigDataNames: []string{},
}
r.layers = append(r.layers, layer)
r.idindex.Add(id)
// This can only fail if the ID is already missing, which shouldnt happen — and in that case the index is already in the desired state anyway.
// This is on various paths to recover from failures, so this should be robust against partially missing data.
_ = r.idindex.Add(id)
r.byid[id] = layer
for _, name := range names {
r.byname[name] = layer
@ -1279,7 +1283,9 @@ func (r *layerStore) deleteInternal(id string) error {
for _, name := range layer.Names {
delete(r.byname, name)
}
r.idindex.Delete(id)
// This can only fail if the ID is already missing, which shouldnt happen — and in that case the index is already in the desired state anyway.
// The stores Delete method is used on various paths to recover from failures, so this should be robust against partially missing data.
_ = r.idindex.Delete(id)
mountLabel := layer.MountLabel
if layer.MountPoint != "" {
delete(r.bymount, layer.MountPoint)

View File

@ -42,6 +42,7 @@ type TruncIndex struct {
}
// NewTruncIndex creates a new TruncIndex and initializes with a list of IDs.
// Invalid IDs are _silently_ ignored.
func NewTruncIndex(ids []string) (idx *TruncIndex) {
idx = &TruncIndex{
ids: make(map[string]struct{}),
@ -51,7 +52,7 @@ func NewTruncIndex(ids []string) (idx *TruncIndex) {
trie: patricia.NewTrie(patricia.MaxPrefixPerNode(64)),
}
for _, id := range ids {
idx.addID(id)
_ = idx.addID(id) // Ignore invalid IDs. Duplicate IDs are not a problem.
}
return
}
@ -132,7 +133,8 @@ func (idx *TruncIndex) Get(s string) (string, error) {
func (idx *TruncIndex) Iterate(handler func(id string)) {
idx.Lock()
defer idx.Unlock()
idx.trie.Visit(func(prefix patricia.Prefix, item patricia.Item) error {
// Ignore the error from Visit: it can only fail if the provided visitor fails, and ours never does.
_ = idx.trie.Visit(func(prefix patricia.Prefix, item patricia.Item) error {
handler(string(prefix))
return nil
})