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

fix: Use context cancellation instead of Stop to stop streams. #879

Merged
merged 11 commits into from
Jun 11, 2024

Conversation

jaqx0r
Copy link
Contributor

@jaqx0r jaqx0r commented Jun 11, 2024

This improves the interface for stream shutdown, though we can't remove Stop completely yet.

Also fixes oneShot handling in various streams.

Issue: #199

We handle their shutdown with fs.Stop().
This means a stream will shut itself down and does not need the tailer to
instruct it to do so.  It thus avoids a race when the tailer doesn't know if
the stream has performed enough reads.

We need to separate the meaning of oneShot mode and streamFromStart as the
former tells us how to exit a stream, and the latter tells us how to start
reading a new file after log rotation.
In this mode we merely tell datagrams to exit when a zero byte read occurs in
oneshot mode.  This is the same as before except we must also explicitly tell
the stream to do it in this mode.
Remove the stop channel, use a local context.

Fix up one test.
Fix up the test to not need cancellation for oneshot mode.
Copy link
Contributor

github-actions bot commented Jun 11, 2024

Unit Test Results

    1 files     27 suites   8m 29s ⏱️
  649 tests   647 ✅ 2 💤 0 ❌
1 920 runs  1 914 ✅ 6 💤 0 ❌

Results for commit 3d4fdba.

♻️ This comment has been updated with latest results.

We don't use the contexts in struct anyway, so this is neater.

Fixes a lint warning.
@jaqx0r jaqx0r force-pushed the logstream-context-cancellation branch from e2f24b1 to 3d4fdba Compare June 11, 2024 06:13
@jaqx0r jaqx0r added this pull request to the merge queue Jun 11, 2024
Merged via the queue into main with commit bb5920e Jun 11, 2024
23 checks passed
@jaqx0r jaqx0r deleted the logstream-context-cancellation branch June 11, 2024 06:31
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