Paths

Table of Contentst

Differential D13606

[tunnelbroker] Recover AMQP consumers
ClosedPublic
Actions

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

View All 51 Files

Subscribers

Details

Reviewers

kamil
varun
will

Commits

rCOMM7649ca4931b2: [tunnelbroker] Recover AMQP consumers

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

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

bartek created this revision.Oct 3 2024, 1:37 PM

bartek held this revision as a draft.

Herald added subscribers: tomek, ashoat. · View Herald TranscriptOct 3 2024, 1:37 PM

bartek added a child revision: D13607: [tunnelbroker] Trigger AMQP reconnect when closing session.Oct 3 2024, 1:37 PM

Harbormaster completed remote builds in B32057: Diff 44886.Oct 3 2024, 2:11 PM

bartek mentioned this in D13611: [tunnelbroker] Recover AMQP before publishing messages.Oct 4 2024, 3:38 AM

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?

Remove connection pool

Harbormaster completed remote builds in B32071: Diff 44903.Oct 4 2024, 8:46 AM

kamil added inline comments.Oct 7 2024, 2:15 AM

services/tunnelbroker/src/amqp.rs
137–143 ↗	(On Diff #44903)	is it needed? should we use it here?
services/tunnelbroker/src/websockets/mod.rs
248 ↗	(On Diff #44903)	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?

kamil accepted this revision.Oct 7 2024, 2:37 AM

This revision is now accepted and ready to land.Oct 7 2024, 2:37 AM

bartek added inline comments.Oct 7 2024, 4:38 AM

services/tunnelbroker/src/amqp.rs
137–143 ↗	(On Diff #44903)	Should be in the next diff
services/tunnelbroker/src/websockets/mod.rs
248 ↗	(On Diff #44903)	On all channel errors. Generally all errors here will need Consumer reset, but another condition inside `reset_failed_amqp` is not a bad idea

Address feedback

Harbormaster completed remote builds in B32088: Diff 44924.Oct 7 2024, 7:25 AM

Closed by commit rCOMM7649ca4931b2: [tunnelbroker] Recover AMQP consumers (authored by bartek). · Explain WhyOct 7 2024, 7:28 AM

This revision was automatically updated to reflect the committed changes.

bartek added a commit: rCOMM7649ca4931b2: [tunnelbroker] Recover AMQP consumers.

bartek mentioned this in rCOMM51a32d2cff2a: [tunnelbroker] Recover AMQP before publishing messages.

Revision Contents
Changeset List

Path

Size

services/

tunnelbroker/

src/

websockets/

32 lines

22 lines

Diff 44924

services/tunnelbroker/src/websockets/mod.rs

Loading...

services/tunnelbroker/src/websockets/session.rs

Loading...