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

feat: Handle stdin closure and shut down the tailer. #868

Merged
merged 25 commits into from
May 26, 2024
Merged

Conversation

jaqx0r
Copy link
Contributor

@jaqx0r jaqx0r commented May 26, 2024

With this change the tailer does a better job of tracking current and future work, shutting down when all streams are completed. If there are no filesystem patterns to watch and all streams close, (e.g. stdin closes) then mtail will also stop.

This fixes a longstanding issue where mtail would remain running if stdin was the only log, and was closed.

Fixes: #331

jaqx0r added 21 commits May 25, 2024 16:06
Save some work by only tailing it once at startup; if we glob it, then we have
to check every time and we then end up having to also do the work in the
logstream of handling the "already closed".
The work of globbing a single pattern is separate from iterating over all
patterns, setting us up to start a goroutine per pattern instead of an
iterator.

Issue: #331
We would like to use this in per-pattern goroutines.

Issue: #331
Instead of collecting sockets and tailing them after, just tail them straight
away.

This makes the "early exit" check for "no things" broken, so replace that with
a check in the final shutdown handler that there were no active globs.  This
will be replaced in the next few changes once migrated to goroutines and we can
use only the `WaitGroup` for counting globs, too, but in the meantime this is
more correct than the previous code.

Issue: #331
We get greater control of what's being polled for by doing so.  This also
removes noise so in a few changes time I can clean up the log pattern poller.
…eams.

This gives us greater control in test.
This has no effect on test behaviour.
…tter.

We now can explicitly control these two events, making the tests faster.

Update the test tail comment explaining what the waker count parameters do; I
keep forgetting, this hint helps!
This lets us await a goroutine per pattern, and if there are none, no waiting
need be done, making shutdown cleaner.

The completion poller is still a bit of a mess though.
This is not a routine we wait for completion of to shut down the tailer, it's
shut down when the tailer is exiting.

Move the stdin check to logstream so it can use it too.
Rename the waker option and the goroutine launcher as it's not just for stale
logs anymore.

Fix tests that were relying on the pattern poller for log completion.
Copy link
Contributor

github-actions bot commented May 26, 2024

Unit Test Results

    1 files     27 suites   8m 50s ⏱️
  648 tests   646 ✅ 2 💤 0 ❌
1 917 runs  1 911 ✅ 6 💤 0 ❌

Results for commit e99f7e5.

♻️ This comment has been updated with latest results.

github-actions[bot]
github-actions bot previously approved these changes May 26, 2024
github-actions[bot]
github-actions bot previously approved these changes May 26, 2024
@jaqx0r jaqx0r added this pull request to the merge queue May 26, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 26, 2024
We pass the context to the functions so we don't need to keep it in here.
@jaqx0r jaqx0r added this pull request to the merge queue May 26, 2024
Merged via the queue into main with commit 291f5a1 May 26, 2024
23 checks passed
@jaqx0r jaqx0r deleted the tailer-shutdown branch May 26, 2024 16:13
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.

mtail keeps on running if producer crashes when reading from stdin
1 participant