Add dead letter support in RabbitMQ pubsub #883

Taction · 2021-05-25T16:23:13Z

Description

Add dead letter support in RabbitMQ pubsub.
Add maxLen and maxLenBytes option, queue can has a maximum capacity in order to prevent using too many resources

Issue reference

We strive to have all PR being opened based on an issue, where the problem or feature have been discussed prior to implementation.

Please reference the issue this PR will close: #882

Checklist

Please make sure you've completed the relevant tasks for this PR, out of the following list:

Code compiles correctly
Created/updated tests
Extended the documentation / Created issue in the https://github.com/dapr/docs/ repo: Doc complement for dapr/components-contrib#883 docs#1834

yaron2 · 2021-06-04T02:56:47Z

@skyao can you please review?

skyao

LGTM

skyao · 2021-06-08T07:47:39Z

@Taction "This branch is out-of-date with the base branch", please help to update the branch.

@yaron2 I reviewed, but I don't have the write access for this repo, you can help to do the merge.

pkedy

Curious about the format of the DLQ and DLX strings

pkedy · 2021-06-11T23:16:05Z

pubsub/rabbitmq/rabbitmq.go

+	var args amqp.Table
+	if r.metadata.enableDeadLetter {
+		// declare dead letter exchange
+		dlxName := fmt.Sprintf("%s-%s", defaultDeadLetterExchangeNamePrefix, queueName)


Why not let the DLX and DLQ be free form? Is this convention mandated by RabbitMQ?

This is not convention mandated by RabbitMQ. Prefixes（DLX and DLQ） can be customized by metadata. Is this OK? In this case, the queueName is consumerID-Topic, the dlx name will be dlxPrefixFromMetadata-consumerID-Topic, the dlq name will be dlqPrefixFromMetadata-consumerID-Topic.

@Taction sorry for the delayed response. What do you think about having the names be formats (e.g. dlq-%s -> usage: dlxName := fmt.Sprintf(defaultDeadLetterExchangeFormat, queueName)).

This would like you specify a constant or plug the queue name in anywhere you want. Thoughts?

Ping @Taction, we'd like to get this merged.

@pkedy fixed.

@yaron2 this is the last review comment needs to be fixed.

pkedy · 2021-06-11T23:18:03Z

@Taction pls resolve conflicts

…nto feat_add_rabbitmq_dead_letter # Conflicts: # pubsub/rabbitmq/metadata_test.go

yaron2 · 2021-07-14T16:10:16Z

@Taction have you addressed all review comments?

daixiang0 · 2021-07-27T05:29:26Z

/cc @yaron2 @skyao

artursouza · 2021-08-10T00:54:04Z

@halspang Please, check this with Matt.

daixiang0 · 2021-09-22T07:06:38Z

@Taction please fix CI failure.

pkedy · 2021-09-22T18:00:19Z

@daixiang0 The MQTT tests can be flakey at times. I re-ran the jobs. Hoping some of the integration tests efforts in 1.5 will resolve this.

cvictory · 2021-10-01T03:32:51Z

@Taction CI Issue.

beiwei30

@Taction I resolved conflicts today. This change looks good to me now.

beiwei30 · 2021-10-03T08:38:11Z

@Taction Thanks for your proposed change. Would you mind to follow up dapr/docs#1834?

Taction · 2021-10-06T09:51:39Z

@beiwei30 I'll complete the doc asap.

codecov · 2021-10-08T14:02:34Z

Codecov Report

Merging #883 (b2aad28) into master (310b4fd) will increase coverage by 0.48%.
The diff coverage is 66.20%.

@@            Coverage Diff             @@
##           master     #883      +/-   ##
==========================================
+ Coverage   34.34%   34.83%   +0.48%     
==========================================
  Files         140      141       +1     
  Lines       11993    12147     +154     
==========================================
+ Hits         4119     4231     +112     
- Misses       7450     7486      +36     
- Partials      424      430       +6

Impacted Files	Coverage Δ
bindings/azure/storagequeues/storagequeues.go	`36.93% <0.00%> (-0.68%)`	⬇️
internal/component/redis/redis.go	`0.00% <0.00%> (ø)`
internal/component/redis/settings.go	`71.42% <ø> (ø)`
pubsub/azure/servicebus/subscription.go	`0.00% <0.00%> (ø)`
tests/conformance/common.go	`14.04% <0.00%> (-0.13%)`	⬇️
pubsub/azure/servicebus/servicebus.go	`30.29% <12.50%> (+0.09%)`	⬆️
nameresolution/mdns/mdns.go	`60.68% <53.33%> (ø)`
pubsub/rabbitmq/rabbitmq.go	`63.34% <55.55%> (-4.66%)`	⬇️
pubsub/rabbitmq/metadata.go	`92.72% <80.95%> (-7.28%)`	⬇️
pubsub/azure/servicebus/message.go	`85.84% <85.84%> (ø)`
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4795f75...b2aad28. Read the comment docs.

yaron2 · 2021-10-11T16:50:05Z

This is merged, thanks @Taction for your contribution, and thanks @beiwei30 @pkedy and all for reviewing.

* Add dead letter support * fix lint * add maxLen and maxLenBytes support * Let the DLX and DLQ be free form. Co-authored-by: Yaron Schneider <yaronsc@microsoft.com> Co-authored-by: Artur Souza <artursouza.ms@outlook.com> Co-authored-by: Simon Leet <31784195+CodeMonkeyLeet@users.noreply.github.com> Co-authored-by: Long Dai <long0dai@foxmail.com> Co-authored-by: Phil Kedy <phil.kedy@gmail.com> Co-authored-by: Long Dai <long.dai@intel.com> Co-authored-by: Ian Luo <ian.luo@gmail.com> Co-authored-by: cvictory <shenglicao2@gmail.com>

Taction and others added 4 commits May 26, 2021 00:14

Add dead letter support

f32ad4b

fix lint

d4e546d

add maxLen and maxLenBytes support

3c62f69

Merge branch 'master' into feat_add_rabbitmq_dead_letter

acf4bd1

skyao approved these changes Jun 8, 2021

View reviewed changes

skyao previously approved these changes Jun 8, 2021

View reviewed changes

Merge branch 'master' into feat_add_rabbitmq_dead_letter

7d026d7

pkedy reviewed Jun 11, 2021

View reviewed changes

Merge branch 'master' of https://github.com/dapr/components-contrib i…

82f4043

…nto feat_add_rabbitmq_dead_letter # Conflicts: # pubsub/rabbitmq/metadata_test.go

Taction mentioned this pull request Jun 25, 2021

Scaling down a pod in kubernetes, causes daprd sidecar to fail all messages in queue with 500 status #946

Closed

yaron2 added 2 commits July 8, 2021 10:35

Merge branch 'master' into feat_add_rabbitmq_dead_letter

d489e88

Merge branch 'master' into feat_add_rabbitmq_dead_letter

6098d56

yaron2 requested review from a team as code owners July 12, 2021 18:39

Merge branch 'master' into feat_add_rabbitmq_dead_letter

76bb4fe

Let the DLX and DLQ be free form.

0bb52c0

Taction dismissed skyao’s stale review via 0bb52c0 July 15, 2021 00:54

Taction requested a review from pkedy July 15, 2021 01:10

Merge branch 'master' into feat_add_rabbitmq_dead_letter

13c699e

CodeMonkeyLeet and others added 3 commits September 1, 2021 10:39

Merge branch 'master' into feat_add_rabbitmq_dead_letter

9a01483

Merge branch 'master' into feat_add_rabbitmq_dead_letter

11e676d

Merge branch 'master' into feat_add_rabbitmq_dead_letter

11f9186

daixiang0 and others added 2 commits September 23, 2021 09:08

Merge branch 'master' into feat_add_rabbitmq_dead_letter

0e5b46a

Merge branch 'master' into feat_add_rabbitmq_dead_letter

e3bd1e7

beiwei30 self-requested a review September 27, 2021 10:11

Merge branch 'master' into feat_add_rabbitmq_dead_letter

5f930e5

Merge branch 'master' into feat_add_rabbitmq_dead_letter

4b6f161

beiwei30 approved these changes Oct 3, 2021

View reviewed changes

beiwei30 mentioned this pull request Oct 3, 2021

Doc complement for dapr/components-contrib#883 dapr/docs#1834

Closed

CodeMonkeyLeet and others added 3 commits October 7, 2021 10:17

Merge branch 'master' into feat_add_rabbitmq_dead_letter

fce4c7f

Merge branch 'master' into feat_add_rabbitmq_dead_letter

45b0bf6

Merge branch 'master' into feat_add_rabbitmq_dead_letter

b2aad28

greenie-msft mentioned this pull request Oct 8, 2021

Add dead letter support for RabbitMQ pubsub dapr/docs#1844

Merged

6 tasks

yaron2 merged commit ca8cf5c into dapr:master Oct 11, 2021

Taction deleted the feat_add_rabbitmq_dead_letter branch October 26, 2021 14:05

Taction mentioned this pull request Nov 23, 2021

REQUEST: New approver status for @Taction dapr/components-contrib dapr/community#117

Closed

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add dead letter support in RabbitMQ pubsub #883

Add dead letter support in RabbitMQ pubsub #883

Taction commented May 25, 2021 •

edited

yaron2 commented Jun 4, 2021

skyao left a comment

skyao commented Jun 8, 2021 •

edited

pkedy left a comment

pkedy Jun 11, 2021

Taction Jun 17, 2021

pkedy Jun 28, 2021

yaron2 Jul 8, 2021

Taction Jul 15, 2021

pkedy commented Jun 11, 2021

yaron2 commented Jul 14, 2021

daixiang0 commented Jul 27, 2021

artursouza commented Aug 10, 2021

daixiang0 commented Sep 22, 2021 •

edited

pkedy commented Sep 22, 2021

cvictory commented Oct 1, 2021

beiwei30 left a comment

beiwei30 commented Oct 3, 2021

Taction commented Oct 6, 2021

codecov bot commented Oct 8, 2021

yaron2 commented Oct 11, 2021

Add dead letter support in RabbitMQ pubsub #883

Add dead letter support in RabbitMQ pubsub #883

Conversation

Taction commented May 25, 2021 • edited

Description

Issue reference

Checklist

yaron2 commented Jun 4, 2021

skyao left a comment

Choose a reason for hiding this comment

skyao commented Jun 8, 2021 • edited

pkedy left a comment

Choose a reason for hiding this comment

pkedy Jun 11, 2021

Choose a reason for hiding this comment

Taction Jun 17, 2021

Choose a reason for hiding this comment

pkedy Jun 28, 2021

Choose a reason for hiding this comment

yaron2 Jul 8, 2021

Choose a reason for hiding this comment

Taction Jul 15, 2021

Choose a reason for hiding this comment

pkedy commented Jun 11, 2021

yaron2 commented Jul 14, 2021

daixiang0 commented Jul 27, 2021

artursouza commented Aug 10, 2021

daixiang0 commented Sep 22, 2021 • edited

pkedy commented Sep 22, 2021

cvictory commented Oct 1, 2021

beiwei30 left a comment

Choose a reason for hiding this comment

beiwei30 commented Oct 3, 2021

Taction commented Oct 6, 2021

codecov bot commented Oct 8, 2021

Codecov Report

yaron2 commented Oct 11, 2021

Taction commented May 25, 2021 •

edited

skyao commented Jun 8, 2021 •

edited

daixiang0 commented Sep 22, 2021 •

edited