Support transient store mode #16371

alexlarsson · 2022-11-01T15:36:19Z

This adds a boolean option transient_store to storage.conf, or via the CLI --transient-store. If this is enabled then all containers state is considered transient and will not survived reboot, as discussed in #16245.

In particular the changes are these:

$rootdir/libpod/bolt_state.db moves to $runroot/libpod
$rootdir/overlay-containers/containers.json moves to $runroot/overlay-containers
There is a new set of storage directories overlay-containerlayers which is similar to overlay-layers (uses the same layer store code). This is used for layers that are part of containers (i,e. not image layers).
The layer store now has a transient mode where the layers.json is stored in $runroot. This is used for the overlay-containerlayers dir.

The end result of this is that all metadata that is stored related to containers and volumes (but not images) end up on tmpfs. This has two main advantages. First of all database writes much faster, in particular for podman run which now has no database writes outside tmpfs. And secondly, since the databases are not persisted we can't run into weird issues on unclean shutdowns or other corner case. Every boot always starts from a clean slate.

Note, while the databases (bolt_state.db, containers.json and layers.json) don't persist, we do use persistant storage for some of the content (such as the "upper" of the overlayfs). This shouldn't cause problems in practice because all such data is indexed by layer/container id which is unlikely to be reused across boots. However, ideally $rootdir/overlay-container and $rootdir/overlay-containerlayers should be cleaned on boot to avoid leftover data to waste space.

The performance improvement of this depends on system performance and characteristics. But here is an example on my desktop:

Without transient mode:

Benchmark 1: bin/podman run --transient-store=false --rm --pull=never --network=host --security-opt seccomp=unconfined fedora true
  Time (mean ± σ):     229.4 ms ±  31.3 ms    [User: 76.2 ms, System: 39.9 ms]
  Range (min … max):   196.0 ms … 298.3 ms    11 runs

With transient mode:

Benchmark 1: bin/podman run --transient-store=true --rm --pull=never --network=host --security-opt seccomp=unconfined fedora true
  Time (mean ± σ):      90.5 ms ±   3.2 ms    [User: 39.1 ms, System: 18.9 ms]
  Range (min … max):    86.4 ms … 103.6 ms    27 runs

NOTE: This commit touches both podman code and vendored containers/storage code, so will need to be split out. But I think reviewing is easier this way.

Does this PR introduce a user-facing change?

None

rhatdan · 2022-11-01T15:56:40Z

@mheon @vrothberg @nalind @baude PTAL

flouthoc · 2022-11-01T16:37:11Z

libpod/runtime.go

@@ -322,6 +322,14 @@ func makeRuntime(runtime *Runtime) (retErr error) {
 		}
 	}

+	// Create the tmpDir
+	if err := os.MkdirAll(runtime.config.Engine.TmpDir, 0751); err != nil {


Shouldn't this be 755 or if 751 is intentional maybe a comment just above could help understand why only execute is given without read to others.

Suggested change

if err := os.MkdirAll(runtime.config.Engine.TmpDir, 0751); err != nil {

if err := os.MkdirAll(runtime.config.Engine.TmpDir, 0755); err != nil {

0751 would give others the ability to reach files (search) under the tree, but not able to list it. I do not know which is correct.

I don't have an opinion on this, I just moved the existing mkdir code earlier in the function because I need to reference TmpDir. The existing code actually had both 751 and 755, so I deleted both, but the 751 is the one that would have "won" before, so I kept that to make it not change behaviour.

I'm fine with changing the behavior to 755`, but maybe that should be a separate PR with separate arguments.

0751 is what's being used at the moment, so I think we should keep it that way in this PR. Changing the default can be done in a separate change if needed.

libpod/runtime.go

pkg/specgenutil/util.go

vendor/github.com/containers/storage/pkg/config/config.go

mheon · 2022-11-01T19:27:48Z

Can you add this to podman info as well, to make it easy to discover?

rhatdan · 2022-11-01T19:39:11Z

Missing man page changes.

Can I run both --transient containers and non transient containers at the same time?

alexlarsson · 2022-11-02T08:39:33Z

Can I run both --transient containers and non transient containers at the same time?

This is more of a storage configuration similar to --runroot= or --root=, as it affects everything. Might it still work? I dunno? Maybe? You'll have double databases for some things (bolt_state.db, etc) but shared for some (images.json, etc). My guess is that it would look like it works, but there are some issues in the locking etc if you look at it more carefully.

vrothberg

I really like the numbers!

For the c/storage changes, I suggest to open a PR directly. This will allow for passing CI and get the storage experts' eyes on it.

Need more docs for both PRs.

cmd/podman/root.go

libpod/runtime.go

pkg/specgenutil/util.go

alexlarsson · 2022-11-02T10:26:27Z

New version with some feedback applied. Still lacks man page updates and splitting up into two PRs.

alexlarsson · 2022-11-07T15:51:37Z

I split out the containers/storage part here: containers/storage#1424
It is then vendored back in here, but that is a hack which I will redo properly when that PR lands.

alexlarsson · 2022-11-07T15:59:57Z

Note, this doesn't currently move container userdata files like config.json, secrets and shm to runroot. Maybe we should do that too for minor performance reasons. I assume only the artifacts subdir will really contain large files, right? Those should probably still go on the disk.

giuseppe · 2022-11-08T11:27:19Z

cmd/podman/root.go

@@ -457,6 +457,8 @@ func rootFlags(cmd *cobra.Command, podmanConfig *entities.PodmanConfig) {
 		pFlags.StringVar(&podmanConfig.Runroot, runrootFlagName, "", "Path to the 'run directory' where all state information is stored")
 		_ = cmd.RegisterFlagCompletionFunc(runrootFlagName, completion.AutocompleteDefault)

+		pFlags.BoolVar(&podmanConfig.TransientStore, "transient-store", false, "Enable transient container storage")


it seems to override the setting in storage.conf.

Shouldn't transient_store = true in storage.conf always set the transient flag?

We need to be able to pass this in the conmon cleanup commands to get the exact same setup as the initial podman command was run as, similar to the --rundir=... args, etc. That can't rely on the global setting.

agreed it makes sense to have the CLI option, but when I create a new container and the global setting is set I'd expect it to affect the newly created container unless I specify --transient=false

Ok, this was a bit busted before, but should work now.

However, if you specify transient_store=true in the config file that currently only affects the default for rootful containers. It seems that there is a bunch of special code that defines the default for rootless containers. This might make sense, but I'm not sure, opinions?

I don't feel strongly about either way. I think it's a fairly niche feature (for now) but it's worth pointing users to storage.conf.

rhatdan · 2022-12-02T18:39:59Z

@mheon @vrothberg PTAL

vrothberg

LGTM overall

vrothberg · 2022-12-05T09:47:00Z

libpod/runtime.go

@@ -322,6 +322,14 @@ func makeRuntime(runtime *Runtime) (retErr error) {
 		}
 	}

+	// Create the tmpDir
+	if err := os.MkdirAll(runtime.config.Engine.TmpDir, 0751); err != nil {


0751 is what's being used at the moment, so I think we should keep it that way in this PR. Changing the default can be done in a separate change if needed.

vrothberg · 2022-12-05T09:51:39Z

cmd/podman/root.go

@@ -457,6 +457,8 @@ func rootFlags(cmd *cobra.Command, podmanConfig *entities.PodmanConfig) {
 		pFlags.StringVar(&podmanConfig.Runroot, runrootFlagName, "", "Path to the 'run directory' where all state information is stored")
 		_ = cmd.RegisterFlagCompletionFunc(runrootFlagName, completion.AutocompleteDefault)

+		pFlags.BoolVar(&podmanConfig.TransientStore, "transient-store", false, "Enable transient container storage")


I don't feel strongly about either way. I think it's a fairly niche feature (for now) but it's worth pointing users to storage.conf.

docs/source/markdown/podman.1.md

vrothberg · 2022-12-05T09:53:27Z

docs/source/markdown/podman.1.md

+This moda mode allows starting containers faster, as well as guaranteeing a fresh state on boot in case of unclean shutdowns or other problems. However
+it is not compabible with a traditional model where containers persist across reboots.
+
+Default value for this is configured in `/etc/containers/storage.conf`.


/etc/containers/storage.conf -> containers-storage.conf(5) as there are various locations for storage.conf.

This is copied verbatim from other options like e.g. --root. Should I change all of them?

Yes lets move to man page links.

Later changes will need to access it earlier, so move its creation to just after the creation of StaticDir. Note: For whatever reason this we created twice before, but we now only do it once. Signed-off-by: Alexander Larsson <[email protected]>

This handles the transient store options from the container/storage configuration in the runtime/engine. Changes are: * Print transient store status in `podman info` * Print transient store status in runtime debug output * Add --transient-store argument to override config option * Propagate config state to conmon cleanup args so the callback podman gets the same config. Note: This doesn't really change any behaviour yet (other than the changes in containers/storage). Signed-off-by: Alexander Larsson <[email protected]>

This brings a performance improvement to `podman run` on top of the other transient_store improvements in containers/storage: Transient mode without transient bolt_db: Benchmark 1: bin/podman run --transient-store=true --rm --pull=never --network=host --security-opt seccomp=unconfined fedora true Time (mean ± σ): 130.6 ms ± 5.8 ms [User: 44.4 ms, System: 25.9 ms] Range (min … max): 122.6 ms … 143.7 ms 21 runs Transient mode with transient bolt_db: Benchmark 1: bin/podman run --transient-store=true --rm --pull=never --network=host --security-opt seccomp=unconfined fedora true Time (mean ± σ): 100.3 ms ± 5.3 ms [User: 40.5 ms, System: 24.9 ms] Range (min … max): 93.0 ms … 111.6 ms 29 runs Signed-off-by: Alexander Larsson <[email protected]>

Signed-off-by: Alexander Larsson <[email protected]>

This just calls GC on the local storage, which will remove any leftover directories from previous containers that are not in the podman db anymore. This is useful primarily for transient store mode, but can also help in the case of an unclean shutdown. Also adds some e2e test to ensure prune --external works. Signed-off-by: Alexander Larsson <[email protected]>

This changes references to `/etc/containers/storage.conf` (and similar) to links to `containers-storage.conf(5)`, as there are alternative locations for this file. Signed-off-by: Alexander Larsson <[email protected]>

vrothberg

LGTM
Nice work, @alexlarsson !

rhatdan · 2022-12-06T14:35:57Z

/approve
/lgtm

openshift-ci · 2022-12-06T14:36:06Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alexlarsson, rhatdan

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [rhatdan]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-ci bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. do-not-merge/release-note-label-needed Enforce release-note requirement, even if just None labels Nov 1, 2022

alexlarsson force-pushed the transient-store branch from 075fcee to 5c15e25 Compare November 1, 2022 16:11

flouthoc reviewed Nov 1, 2022

View reviewed changes

Luap99 reviewed Nov 1, 2022

View reviewed changes

libpod/runtime.go Outdated Show resolved Hide resolved

rhatdan reviewed Nov 1, 2022

View reviewed changes

pkg/specgenutil/util.go Outdated Show resolved Hide resolved

rhatdan reviewed Nov 1, 2022

View reviewed changes

vendor/github.com/containers/storage/pkg/config/config.go Outdated Show resolved Hide resolved

rhatdan approved these changes Nov 1, 2022

View reviewed changes

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 1, 2022

vrothberg reviewed Nov 2, 2022

View reviewed changes

cmd/podman/root.go Show resolved Hide resolved

libpod/runtime.go Outdated Show resolved Hide resolved

pkg/specgenutil/util.go Outdated Show resolved Hide resolved

alexlarsson force-pushed the transient-store branch from 5c15e25 to 70c90ba Compare November 2, 2022 10:24

alexlarsson mentioned this pull request Nov 7, 2022

Support transient storage mode containers/storage#1424

Merged

alexlarsson force-pushed the transient-store branch from 70c90ba to 0d1e64a Compare November 7, 2022 15:50

alexlarsson force-pushed the transient-store branch from 0d1e64a to 7b1e537 Compare November 7, 2022 16:11

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 7, 2022

giuseppe reviewed Nov 8, 2022

View reviewed changes

alexlarsson force-pushed the transient-store branch 4 times, most recently from e8f53dd to 7cb716d Compare November 10, 2022 12:33

openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 10, 2022

alexlarsson force-pushed the transient-store branch 2 times, most recently from b523c9b to b55fb1f Compare November 11, 2022 08:01

alexlarsson force-pushed the transient-store branch 2 times, most recently from 0823d99 to c917692 Compare December 2, 2022 09:37

github-actions bot added the kind/api-change Change to remote API; merits scrutiny label Dec 2, 2022

alexlarsson force-pushed the transient-store branch 4 times, most recently from 5bc4d64 to cf1d1d4 Compare December 2, 2022 13:47

alexlarsson changed the title ~~WIP: Support transient store mode~~ Support transient store mode Dec 2, 2022

openshift-ci bot added release-note-none and removed do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. do-not-merge/release-note-label-needed Enforce release-note requirement, even if just None labels Dec 2, 2022

vrothberg reviewed Dec 5, 2022

View reviewed changes

alexlarsson added 5 commits December 5, 2022 18:09

Add some tests for transient store

f1dbfda

Signed-off-by: Alexander Larsson <[email protected]>

alexlarsson force-pushed the transient-store branch from cf1d1d4 to 93d2ec1 Compare December 5, 2022 17:09

podman manpage: Use man-page links instead of file names

75f4215

This changes references to `/etc/containers/storage.conf` (and similar) to links to `containers-storage.conf(5)`, as there are alternative locations for this file. Signed-off-by: Alexander Larsson <[email protected]>

vrothberg reviewed Dec 6, 2022

View reviewed changes

openshift-ci bot assigned rhatdan Dec 6, 2022

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Dec 6, 2022

openshift-merge-robot merged commit 4a8d953 into containers:main Dec 6, 2022

edsantiago mentioned this pull request Dec 8, 2022

podman machine: flaky start #16789

Closed

socketpair mentioned this pull request Feb 24, 2023

podman run: add ability to store files written inside a container in host's tmpfs. #17599

Closed

github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 18, 2023

github-actions bot locked as resolved and limited conversation to collaborators Sep 18, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support transient store mode #16371

Support transient store mode #16371

alexlarsson commented Nov 1, 2022 •

edited

Loading

rhatdan commented Nov 1, 2022

flouthoc Nov 1, 2022 •

edited

Loading

rhatdan Nov 1, 2022

alexlarsson Nov 2, 2022

vrothberg Dec 5, 2022

mheon commented Nov 1, 2022

rhatdan commented Nov 1, 2022

alexlarsson commented Nov 2, 2022 •

edited

Loading

vrothberg left a comment

alexlarsson commented Nov 2, 2022

alexlarsson commented Nov 7, 2022

alexlarsson commented Nov 7, 2022

giuseppe Nov 8, 2022

alexlarsson Nov 8, 2022

giuseppe Nov 9, 2022

alexlarsson Nov 9, 2022

alexlarsson Nov 9, 2022

vrothberg Dec 5, 2022

rhatdan commented Dec 2, 2022

vrothberg left a comment

vrothberg Dec 5, 2022

vrothberg Dec 5, 2022

vrothberg Dec 5, 2022

alexlarsson Dec 5, 2022

rhatdan Dec 5, 2022

vrothberg left a comment

rhatdan commented Dec 6, 2022

openshift-ci bot commented Dec 6, 2022

	if err := os.MkdirAll(runtime.config.Engine.TmpDir, 0751); err != nil {
	if err := os.MkdirAll(runtime.config.Engine.TmpDir, 0755); err != nil {

Support transient store mode #16371

Support transient store mode #16371

Conversation

alexlarsson commented Nov 1, 2022 • edited Loading

Does this PR introduce a user-facing change?

rhatdan commented Nov 1, 2022

flouthoc Nov 1, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mheon commented Nov 1, 2022

rhatdan commented Nov 1, 2022

alexlarsson commented Nov 2, 2022 • edited Loading

vrothberg left a comment

Choose a reason for hiding this comment

alexlarsson commented Nov 2, 2022

alexlarsson commented Nov 7, 2022

alexlarsson commented Nov 7, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rhatdan commented Dec 2, 2022

vrothberg left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vrothberg left a comment

Choose a reason for hiding this comment

rhatdan commented Dec 6, 2022

openshift-ci bot commented Dec 6, 2022

alexlarsson commented Nov 1, 2022 •

edited

Loading

flouthoc Nov 1, 2022 •

edited

Loading

alexlarsson commented Nov 2, 2022 •

edited

Loading