Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimize 'enter' and 'run' for a container getting initialized #1495

Conversation

debarshiray
Copy link
Member

@debarshiray debarshiray commented May 20, 2024

Currently, the enter and run commands poll at one second intervals
to check if the Toolbx container's entry point has created the
initialization stamp file to indicate that the container has been
initialized. This came from the POSIX shell implementation [1], where
it was relatively easier to poll than to use inotify(7) to monitor the
file system.

The problem with polling is that the interval is always going to be
either too short and waste resources or too long and cause delays. The
current one second interval is sufficiently long to add a noticeable
delay to the enter and run commands.

It will be better to use inotify(7) to monitor the file system, which is
quite easy to do with the Go implementation, so that the commands can
proceed as soon as the initialization stamp file is available, instead
of waiting for the polling interval to pass.

There's a fallback to polling, as before, when the operating system is
suffering from a shortage of resources needed for inotify(7). This code
path can be forced through the TOOLBX_RUN_USE_POLLING environment
variable for testing. Setting this environment variable disables some
code to ensure that the polling ticker is actually used, because,
otherwise, the race between the creation and detection of the
initialization stamp file makes it difficult to test the fallback.

[1] Commit d3e0f3d
d3e0f3df06d3f5ac
#305

#1070

@debarshiray debarshiray requested a review from HarryMichal as a code owner May 20, 2024 10:15
@debarshiray debarshiray force-pushed the wip/rishi/cmd-run-prefer-fs-events-over-polling-to-ensure-initialization branch from d45d993 to b34979d Compare May 20, 2024 10:29
Copy link

Build succeeded.
https://softwarefactory-project.io/zuul/t/local/buildset/e071ec50213946ed8fa6e0099e6c5eed

✔️ unit-test SUCCESS in 6m 42s
✔️ unit-test-migration-path-for-coreos-toolbox SUCCESS in 3m 35s
✔️ unit-test-restricted SUCCESS in 5m 53s
✔️ system-test-fedora-rawhide SUCCESS in 35m 29s
✔️ system-test-fedora-40 SUCCESS in 32m 41s
✔️ system-test-fedora-39 SUCCESS in 33m 17s
✔️ system-test-fedora-38 SUCCESS in 32m 10s

There's no immediate desire to make Toolbx work on operating systems
that don't use forward slashes as path separators.  However, there's
also no reason not to use the standard library.

containers#1495
@debarshiray debarshiray force-pushed the wip/rishi/cmd-run-prefer-fs-events-over-polling-to-ensure-initialization branch from b34979d to c610860 Compare May 22, 2024 07:38
Copy link

Build succeeded.
https://softwarefactory-project.io/zuul/t/local/buildset/7e3d069613934350a8dc9a400e320f4c

✔️ unit-test SUCCESS in 6m 31s
✔️ unit-test-migration-path-for-coreos-toolbox SUCCESS in 3m 04s
✔️ unit-test-restricted SUCCESS in 5m 37s
✔️ system-test-fedora-rawhide SUCCESS in 35m 42s
✔️ system-test-fedora-40 SUCCESS in 34m 15s
✔️ system-test-fedora-39 SUCCESS in 33m 58s
✔️ system-test-fedora-38 SUCCESS in 33m 48s

@debarshiray debarshiray force-pushed the wip/rishi/cmd-run-prefer-fs-events-over-polling-to-ensure-initialization branch from e40d3a3 to d3bd5e9 Compare May 22, 2024 08:45
Copy link

Build failed.
https://softwarefactory-project.io/zuul/t/local/buildset/cd7d1c901d924802b7b19cb02ef9fdf3

✔️ unit-test SUCCESS in 6m 48s
✔️ unit-test-migration-path-for-coreos-toolbox SUCCESS in 3m 51s
✔️ unit-test-restricted SUCCESS in 6m 00s
system-test-fedora-rawhide FAILURE in 35m 58s
system-test-fedora-40 FAILURE in 34m 15s
system-test-fedora-39 FAILURE in 32m 39s
✔️ system-test-fedora-38 SUCCESS in 33m 18s

@debarshiray debarshiray force-pushed the wip/rishi/cmd-run-prefer-fs-events-over-polling-to-ensure-initialization branch from d3bd5e9 to ff6c146 Compare May 22, 2024 09:37
Copy link

Build succeeded.
https://softwarefactory-project.io/zuul/t/local/buildset/4e820e8872914a35be2e8e89384bb5f2

✔️ unit-test SUCCESS in 7m 19s
✔️ unit-test-migration-path-for-coreos-toolbox SUCCESS in 3m 57s
✔️ unit-test-restricted SUCCESS in 6m 11s
✔️ system-test-fedora-rawhide SUCCESS in 36m 29s
✔️ system-test-fedora-40 SUCCESS in 33m 46s
✔️ system-test-fedora-39 SUCCESS in 34m 10s
✔️ system-test-fedora-38 SUCCESS in 33m 25s

@debarshiray debarshiray force-pushed the wip/rishi/cmd-run-prefer-fs-events-over-polling-to-ensure-initialization branch from ff6c146 to 3c8f30a Compare May 22, 2024 10:21
Copy link

Build failed.
https://softwarefactory-project.io/zuul/t/local/buildset/31353efeaf6a40719897ddb35175c13e

✔️ unit-test SUCCESS in 7m 14s
unit-test-migration-path-for-coreos-toolbox POST_FAILURE in 3m 47s
✔️ unit-test-restricted SUCCESS in 6m 05s
✔️ system-test-fedora-rawhide SUCCESS in 38m 39s
✔️ system-test-fedora-40 SUCCESS in 36m 53s
✔️ system-test-fedora-39 SUCCESS in 35m 59s
✔️ system-test-fedora-38 SUCCESS in 36m 16s

@debarshiray
Copy link
Member Author

Build failed. https://softwarefactory-project.io/zuul/t/local/buildset/31353efeaf6a40719897ddb35175c13e

unit-test-migration-path-for-coreos-toolbox POST_FAILURE in 3m 47s

I will waive that POST_FAILURE because the only change between the last two runs was this addition to test/system/104-run/bats:

+  assert_output --partial "Handling polling tick"

Currently, the 'enter' and 'run' commands poll at one second intervals
to check if the Toolbx container's entry point has created the
initialization stamp file to indicate that the container has been
initialized.  This came from the POSIX shell implementation [1], where
it was relatively easier to poll than to use inotify(7) to monitor the
file system.

The problem with polling is that the interval is always going to be
either too short and waste resources or too long and cause delays.  The
current one second interval is sufficiently long to add a noticeable
delay to the 'enter' and 'run' commands.

It will be better to use inotify(7) to monitor the file system, which is
quite easy to do with the Go implementation, so that the commands can
proceed as soon as the initialization stamp file is available, instead
of waiting for the polling interval to pass.

There's a fallback to polling, as before, when the operating system is
suffering from a shortage of resources needed for inotify(7).  This code
path can be forced through the TOOLBX_RUN_USE_POLLING environment
variable for testing.  Setting this environment variable disables some
code to ensure that the polling ticker is actually used, because,
otherwise, the race between the creation and detection of the
initialization stamp file makes it difficult to test the fallback.

[1] Commit d3e0f3d
    containers@d3e0f3df06d3f5ac
    containers#305

containers#1070
@debarshiray debarshiray force-pushed the wip/rishi/cmd-run-prefer-fs-events-over-polling-to-ensure-initialization branch from 3c8f30a to d8289fb Compare May 22, 2024 11:07
@debarshiray debarshiray changed the title cmd/run: Optimize 'enter' and 'run' for a container getting initialized Optimize 'enter' and 'run' for a container getting initialized May 22, 2024
Copy link

Build succeeded.
https://softwarefactory-project.io/zuul/t/local/buildset/dce365c2b2d04ce691e49f973c0925bb

✔️ unit-test SUCCESS in 6m 16s
✔️ unit-test-migration-path-for-coreos-toolbox SUCCESS in 5m 35s
✔️ unit-test-restricted SUCCESS in 6m 13s
✔️ system-test-fedora-rawhide SUCCESS in 35m 51s
✔️ system-test-fedora-40 SUCCESS in 33m 25s
✔️ system-test-fedora-39 SUCCESS in 33m 55s
✔️ system-test-fedora-38 SUCCESS in 33m 42s

@debarshiray debarshiray merged commit d8289fb into containers:main May 22, 2024
3 checks passed
@debarshiray debarshiray deleted the wip/rishi/cmd-run-prefer-fs-events-over-polling-to-ensure-initialization branch May 22, 2024 11:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant