Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[Messenger] Adding final routing key to delay queue name#31355
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
e8f7ba6 tocb757e2CompareUh oh!
There was an error while loading.Please reload this page.
src/Symfony/Component/Messenger/Tests/Transport/AmqpExt/AmqpExtIntegrationTest.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
cb757e2 to9940e71Comparefabpot commentedMay 7, 2019
Thank you@weaverryan. |
…weaverryan)This PR was merged into the 4.3-dev branch.Discussion----------[Messenger] Adding final routing key to delay queue name| Q | A| ------------- | ---| Branch? | master| Bug fix? | yes| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets |#31241| License | MIT| Doc PR | n/aFixes#31241.When we delay, we create a queue whose `x-message-ttl` matches the delay length and `x-dead-letter-routing-key` matches the original routing key used for the message. However, before this PR, the original routing key was not part of that queue's name. The result is that if two messages were delayed by the same length, but with different routing keys, the second would try to "redeclare" the existing delay queue with a new `x-dead-letter-routing-key`, resulting in an error similar to:> Server channel error: 406, message: PRECONDITION_FAILED - inequivalent arg 'x-dead-letter-routing-key' for queue 'delay_queue_1000'Integration test was improved to catch this.Cheers!Commits-------9940e71 fixing a bug where the delay queue name did not contain the final routing key
| $actualDuration =microtime(true) -$startTime; | ||
| if (\method_exists([$this,'assertEqualsWithDelta'])) { | ||
| $this->assertEqualsWithDelta($expectedDuration,$actualDuration,'Duration was not within expected range',.5); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
the order of arguments is wrong.assertEqualsWithDelta($expected, $actual, float $delta, string $message = '')
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I'm on it
This PR was merged into the 4.3-dev branch.Discussion----------[Messenger] Fixing missed tests from#31355| Q | A| ------------- | ---| Branch? | master| Bug fix? | yes| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets | none| License | MIT| Doc PR | not neededTests I missed from#31355 - sorry about that!Commits-------ab37449 Fixing missed tests from#31355
…obion)This PR was merged into the 4.3 branch.Discussion----------[Messenger] fix delay delivery for non-fanout exchanges| Q | A| ------------- | ---| Branch? | 4.3| Bug fix? | yes| New feature? | no <!-- please update src/**/CHANGELOG.md files -->| BC breaks? | no <!-- seehttps://symfony.com/bc -->| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->| Tests pass? | yes <!-- please add some, will be required by reviewers -->| Fixed tickets || License | MIT| Doc PR | symfony/symfony-docs#... <!-- required for new features -->Fixes two bugs and outdated phpdoc:1. When your exchange is not of type fanout, but direct for example, then delivery of delayed (retrying) messages does not work. This is because the delay logics adds a routing key to message. It was fixed if you have a custom routing key in#31355. But if you have no routing key, it still changed the routing key which means the message will not be delivery from your direct exchange to your queue anymore after being in the delay exchange. For fanout, which is the default, it does not matter because the routing key is ignored.2. also fix dsn parsing of plain `amqp://` which is a valid URI that parse_url cannot handle when you want to pass all parameters as optionsCommits-------0f15306 [Messenger] fix delay delivery for non-fanout exchanges
Fixes#31241.
When we delay, we create a queue whose
x-message-ttlmatches the delay length andx-dead-letter-routing-keymatches the original routing key used for the message. However, before this PR, the original routing key was not part of that queue's name. The result is that if two messages were delayed by the same length, but with different routing keys, the second would try to "redeclare" the existing delay queue with a newx-dead-letter-routing-key, resulting in an error similar to:Integration test was improved to catch this.
Cheers!