[FEATURE] Investigate GetXAttr performance

[Kern--]

Description

Looking at CPU profiles for a couple of containers, it seems that GetXAttr is consistently showing up as a non-trivial amount of the soci-snapshotter's runtime. Specifically it is n.IsOpaque() which checks for the existance of a whiteout in the metadata db:

soci-snapshotter/fs/layer/node.go

Lines 288 to 293 in 6dcfca7

	func (n *node) isOpaque() bool {
	if _, _, err := n.fs.r.Metadata().GetChild(n.id, whiteoutOpaqueDir); err == nil {
	return true
	}
	return false
	}

example profiles:

It's possible that this is a red herring and what we're actually seeing is just that GetXAttr is a very common, so we should be careful how much effort we put into this before validating that we're seeing benefit.

Describe the solution you'd like

We should investigate whether there is a way we can reduce the cost of n.IsOpaque().

We could possibly move the opaqueness to a property of the node itself rather than pushing it down into the metadata db. A good first step might be to try this and compare benchmarks to see if it has an effect.

Describe any alternative solutions/features you've considered

No response

Any additional context or information about the feature request

No response

[Kern--]

Copying some context

Ok, I finally got around to testing how this interacts with OverlayFS. The answer is that OverlayFS handles deleted files by creating "whiteouts" in higher layers. For example, if a lower layer has dir/file then a higher layer can delete it by containing a file named dir/.wh.file. This works with the change.

For directories, overlayfs has an "opaque" xattr (trusted.overlay.opaque=y) which treats all files as deleted rather than having to explicitly create a whiteout file for each. In containers, this is encoded as a special .wh..wh..opq file in a higher layer (.e.g. to delete all files in /bin a higher layer would contain /bin/.wh..wh..opq). So it's possible for the container image to contain an opaque directory with no xattrs. This change will disable xattrs and the directory will be populated at runtime.

An example:

FROM alpine:latest

RUN mkdir /dir && touch /dir/file1 /dir/file2 /dir/file3
RUN touch /dir/.wh..wh..opq

Without Soci:

# ls /dir
#

With SOCI:

# ls /dir
file1 file2 file3
#

The image spec says that whiteouts should be explicit, but to be spec compliant we must accept opaque directories. At a minimum, we should look for .wh..wh..opq files when determining if a layer has xattrs.

#718 (comment)

Taking a step back and considering #971, I think the real issue we have is that checking opaqueness is expensive. Each check goes through bbolt to see if a file exists. Since the lower layers are immutable, I think we could probably just push opaqueness up into the in-memory nodes.

I'm going to run some quick hard-coded benchmarks (normal, disableXattrs hard coded to true, isOpaque hard coded to false) to see how that compares.

#718 (comment)

[sondavidb]

TL;DR GetXAttrs is an expensive operation and should be avoided unless necessary. Full testing and results on changes can be found here: #1021 (comment)

Operations that normally made significant calls to GetXAttrs would take up a nontrivial amount of CPU seconds due to checking if isOpaque is true. #1021 fixed this by not making these calls on a layer-level unless the directory had opaque directories.

While we may want further iteration of this (i.e. see if we can make it more granular to avoid the call more often), the question and immediate problem at hand have been resolved, so this can be closed.