Add contexts to pubsub.Subscribe to allow early cancelation #1756
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
/ok-to-test |
ItalyPaleAle
force-pushed
the
pubsub-context
branch
from
e5b3625
to
0c0dcdd
Compare
|
/ok-to-test |
ItalyPaleAle
added a commit
to ItalyPaleAle/dapr
that referenced
this pull request
Jun 2, 2022
With dapr/components-contrib#1756, it's now possible to stop receiving messages from pubsub components via context cancelation. This makes it so we won't receive more work while the sidecar is shutting down, but we can still publish messages in pubsub. Signed-off-by: ItalyPaleAle <43508+ItalyPaleAle@users.noreply.github.com>
This commit is related to dapr/dapr#4624. As noted there, we have an issue in the runtime where all components are shut down after the grace period, when the app is likely already stopped. Because of that, certain input components (the subscribe part of pubsub and the input part of bindings - the latter not in scope of this PR), can continue bringing new work when it's known to fail. In order to fix the issue linked above properly, we need to implement a way for PubSub components to have the "publish" part closed before the "subscribe" one (and in the future that will need to be done for input bindings too). This commit achieves precisely that by adding a context in the Subscribe method. When that context is canceled (which can be at any time), the subscription is removed. PS: This API change was implemented so it can one day be used for dapr/dapr#814 too, as it allows canceling individual subscriptions by using a different context. Although that's not possible today because it requires more work on the runtime, it does implement everything that's needed in the pubsub components already. Signed-off-by: ItalyPaleAle <43508+ItalyPaleAle@users.noreply.github.com>
ItalyPaleAle
force-pushed
the
pubsub-context
branch
from
0c0dcdd
to
c9f5c32
Compare
ItalyPaleAle
commented
Jun 2, 2022
| @@ -163,6 +162,7 @@ require ( | |||
| github.com/huaweicloud/huaweicloud-sdk-go-v3 v0.0.87 | |||
| github.com/labd/commercetools-go-sdk v0.3.2 | |||
| github.com/nacos-group/nacos-sdk-go/v2 v2.0.1 | |||
| github.com/rabbitmq/amqp091-go v1.3.4 | |||
There was a problem hiding this comment.
github.com/rabbitmq/amqp091-go is the new version of github.com/streadway/amqp and it's maintained by RabbitMQ now
Codecov Report
@@ Coverage Diff @@
## master #1756 +/- ##
==========================================
- Coverage 36.74% 36.67% -0.08%
==========================================
Files 176 177 +1
Lines 15955 16200 +245
==========================================
+ Hits 5863 5941 +78
- Misses 9434 9590 +156
- Partials 658 669 +11
Continue to review full report at Codecov.
|
|
Note that all certification tests will fail until the corresponding PR is merged in dapr/dapr, due to a change in the interface of the pubsub components |
berndverst
approved these changes
Jun 2, 2022
ItalyPaleAle
added a commit
to ItalyPaleAle/dapr
that referenced
this pull request
Jun 3, 2022
With dapr/components-contrib#1756, it's now possible to stop receiving messages from pubsub components via context cancelation. This makes it so we won't receive more work while the sidecar is shutting down, but we can still publish messages in pubsub. Signed-off-by: ItalyPaleAle <43508+ItalyPaleAle@users.noreply.github.com>
artursouza
pushed a commit
to dapr/dapr
that referenced
this pull request
Jun 3, 2022
* Updated pinned components-contrib Signed-off-by: ItalyPaleAle <43508+ItalyPaleAle@users.noreply.github.com> * Stop pubsub subscriptions before shutdown grace period With dapr/components-contrib#1756, it's now possible to stop receiving messages from pubsub components via context cancelation. This makes it so we won't receive more work while the sidecar is shutting down, but we can still publish messages in pubsub. Signed-off-by: ItalyPaleAle <43508+ItalyPaleAle@users.noreply.github.com>
beiwei30
pushed a commit
to beiwei30/components-contrib
that referenced
this pull request
Jun 7, 2022
This commit is related to dapr/dapr#4624. As noted there, we have an issue in the runtime where all components are shut down after the grace period, when the app is likely already stopped. Because of that, certain input components (the subscribe part of pubsub and the input part of bindings - the latter not in scope of this PR), can continue bringing new work when it's known to fail. In order to fix the issue linked above properly, we need to implement a way for PubSub components to have the "publish" part closed before the "subscribe" one (and in the future that will need to be done for input bindings too). This commit achieves precisely that by adding a context in the Subscribe method. When that context is canceled (which can be at any time), the subscription is removed. PS: This API change was implemented so it can one day be used for dapr/dapr#814 too, as it allows canceling individual subscriptions by using a different context. Although that's not possible today because it requires more work on the runtime, it does implement everything that's needed in the pubsub components already. Signed-off-by: ItalyPaleAle <43508+ItalyPaleAle@users.noreply.github.com>
ItalyPaleAle
added a commit
to ItalyPaleAle/dapr
that referenced
this pull request
Jun 8, 2022
* Updated pinned components-contrib Signed-off-by: ItalyPaleAle <43508+ItalyPaleAle@users.noreply.github.com> * Stop pubsub subscriptions before shutdown grace period With dapr/components-contrib#1756, it's now possible to stop receiving messages from pubsub components via context cancelation. This makes it so we won't receive more work while the sidecar is shutting down, but we can still publish messages in pubsub. Signed-off-by: ItalyPaleAle <43508+ItalyPaleAle@users.noreply.github.com>
7 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR is related to dapr/dapr#4624. As noted there, we have an issue in the runtime where all components are shut down after the grace period, when the app is likely already stopped. Because of that, certain input components (the subscribe part of pubsub and the input part of bindings - the latter not in scope of this PR), can continue bringing new work when it's known to fail.
In order to fix the issue linked above properly, we need to implement a way for PubSub components to have the "publish" part closed before the "subscribe" one (and in the future that will need to be done for input bindings too).
This PR achieves precisely that by adding a context in the
Subscribemethod. When that context is canceled (which can be at any time), the subscription is removed.PS: This API change was implemented so it can one day be used for dapr/dapr#814 too, as it allows canceling individual subscriptions by using a different context. Although that's not possible today because it requires more work on the runtime, it does implement everything that's needed in the pubsub components already.
Checklist
Please make sure you've completed the relevant tasks for this PR, out of the following list:
TODO
Marking this PR as draft until the following are done (in order):