Page MenuHomePhabricator

[tunnelbroker] Recover AMQP consumers
ClosedPublic

Authored by bartek on Oct 3 2024, 1:37 PM.
Tags
None
Referenced Files
F3337873: D13606.diff
Thu, Nov 21, 5:19 PM
Unknown Object (File)
Sun, Nov 10, 7:39 AM
Unknown Object (File)
Fri, Nov 8, 10:37 PM
Unknown Object (File)
Fri, Nov 8, 1:48 PM
Unknown Object (File)
Wed, Oct 30, 1:58 PM
Unknown Object (File)
Wed, Oct 30, 1:07 PM
Unknown Object (File)
Wed, Oct 30, 3:04 AM
Unknown Object (File)
Tue, Oct 29, 1:51 AM
Subscribers

Details

Summary

When a consumer returns a confirmation error, we can try restoring it without closing the WebSocket session

Depends on D13605

Test Plan

Test plan from D13595 (commtest) sometimes failed on this confirmation. Now it's no longer happening

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

bartek held this revision as a draft.
bartek published this revision for review.Oct 4 2024, 3:58 AM
bartek edited the summary of this revision. (Show Details)
bartek edited the test plan for this revision. (Show Details)
bartek added inline comments.
services/tunnelbroker/src/websockets/mod.rs
254 ↗(On Diff #44886)

Should it be warn or error?

services/tunnelbroker/src/amqp.rs
137–143

is it needed? should we use it here?

services/tunnelbroker/src/websockets/mod.rs
248

do we always want to reset or only on connection errors?

254 ↗(On Diff #44886)

maybe error to first have a better view of when and how often this is happening, and later we can change to warn?

This revision is now accepted and ready to land.Oct 7 2024, 2:37 AM
services/tunnelbroker/src/amqp.rs
137–143

Should be in the next diff

services/tunnelbroker/src/websockets/mod.rs
248

On all channel errors. Generally all errors here will need Consumer reset, but another condition inside reset_failed_amqp is not a bad idea