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

Consume clean exit fixes #161

Merged
merged 3 commits into from
Oct 20, 2023
Merged

Consume clean exit fixes #161

merged 3 commits into from
Oct 20, 2023

Conversation

mtmk
Copy link
Collaborator

@mtmk mtmk commented Oct 19, 2023

  • Enlarge subscription channel to avoid blocking other socket reads on the same connection.

  • Pass cancellation token to Consume() so it can be gracefully stopped.

  • 'Consume' can also be gracefully stopped when Stop() is called.

  • There is no graceful stop for 'Fetch' since we have no option but wait for the single pull request to complete.

  • Disposing the 'Consume' or 'Fetch' would exit loops a.s.a.p.

* Enlarge subscription channel to avoid blocking other socket reads on the
  same connection.

* Pass cancellation token to Consume() so it can be gracefully stopped.

* 'Consume' can also be gracefully stopped when Stop() is called.

* There is no graceful stop for 'Fetch' since we have no option
  but wait for the single pull request to complete.

* Disposing the 'Consume' or 'Fetch' would exit loops a.s.a.p.
@mtmk mtmk requested a review from caleblloyd October 19, 2023 23:17
Copy link
Collaborator

@caleblloyd caleblloyd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

{
while (!cts.Token.IsCancellationRequested)
{
try
{
Console.WriteLine($"___\nFETCH {maxMsgs}");
await consumer.RefreshAsync(cts.Token);
await using var sub = await consumer.FetchAsync<RawData>(fetchOpts, cts.Token);
await using var sub = await consumer.FetchAsync<IMemoryOwner<byte>>(fetchOpts, cts.Token);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you are a fan of #162 you may want to convert these examples!

@mtmk mtmk merged commit 0347d6c into main Oct 20, 2023
9 checks passed
@mtmk mtmk deleted the consumer-clean-exit branch October 20, 2023 05:19
mtmk added a commit that referenced this pull request Oct 20, 2023
* Initial Object Store implementation (#150)
* TLS first connection (#156)
* Consume clean exit fixes (#161)
* Add NatsMemoryOwner (#162)
@mtmk mtmk mentioned this pull request Oct 20, 2023
mtmk added a commit that referenced this pull request Oct 20, 2023
* Release prep 2.0.0-alpha.5

* Initial Object Store implementation (#150)
* TLS first connection (#156)
* Consume clean exit fixes (#161)
* Add NatsMemoryOwner (#162)

* Pack object store
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.

2 participants