Page MenuHomePhabricator
Differential D4947

[services] Tunnelbroker - Add wait until amqp ready in a first initialization call
AbandonedPublic
Actions

Authored by • max on Aug 24 2022, 6:27 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Jun 29, 11:45 PM
Unknown Object (File)
Sat, Jun 29, 5:23 AM
Unknown Object (File)
Wed, Jun 26, 5:22 AM
Unknown Object (File)
Sun, Jun 23, 4:41 PM
Unknown Object (File)
Sun, Jun 23, 4:41 PM
Unknown Object (File)
Sun, Jun 23, 4:29 PM
Unknown Object (File)
Wed, Jun 12, 3:50 PM
Unknown Object (File)
May 24 2024, 8:04 AM
View All 31 Files
Subscribers
• abosh
ashoat
atul
• karol
tomek

Details

Reviewers
• karol
tomek
Summary

This diff introduces small changes to the AMQP client initialization function.

The good point is to wait until the client is ready in the initialization call using waitUntilready from D4741. In case the AMQP server is not available we shouldn't defer connection/reconnection waiting to the first call of the send or ack and stop the execution of the app right after the maximum reconnection failures is reached.

The init() is called from the main and in tests in the SetUp section. That's why is a good point to wait for a successful connection here.

Test Plan

Running of yarn run-unit-tests with the unavailable amqp server will cause a reconnection and the app exit after the N failures right on the initialization phase.

Diff Detail

Repository
rCOMM Comm
Branch
amqp-add-wait-on-init
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

• max created this revision.Aug 24 2022, 6:27 AM
• max held this revision as a draft.
Herald added subscribers: • abosh, • karol, atul and 2 others. · View Herald TranscriptAug 24 2022, 6:27 AM
• max edited the summary of this revision. (Show Details)Aug 24 2022, 6:36 AM
• max edited the test plan for this revision. (Show Details)
• max added reviewers: • karol, tomek.
Harbormaster completed remote builds in B11583: Diff 15925.Aug 24 2022, 6:38 AM
• max published this revision for review.Aug 24 2022, 6:38 AM
• max edited the summary of this revision. (Show Details)
tomek requested changes to this revision.Aug 25 2022, 1:42 AM

It doesn't make too much sense for me in context of the previous diffs. We wanted to have the initialization in another thread and we were afraid that a short lived thread shouldn't be the one that initializes, and now we're blocking until the initialization is done. I guess this whole approach needs some rethinking, because it doesn't feel consistent.
A quick guess is that maybe we should call waitUntilReady inside connect. But still, we need to find a design that is more consistent.

This revision now requires changes to proceed.Aug 25 2022, 1:42 AM
• max abandoned this revision.Aug 25 2022, 2:16 AM
In D4947#143312, @tomek wrote:

It doesn't make too much sense for me in context of the previous diffs. We wanted to have the initialization in another thread and we were afraid that a short lived thread shouldn't be the one that initializes, and now we're blocking until the initialization is done. I guess this whole approach needs some rethinking, because it doesn't feel consistent.
A quick guess is that maybe we should call waitUntilReady inside connect. But still, we need to find a design that is more consistent.

Ok, let's abandon this diff in a favor of possible future updates for 0.5.

Revision Contents
Changeset List

PathSize
services/
tunnelbroker/
src/
Amqp/
1 line

Diff 15925

View Options

services/tunnelbroker/src/Amqp/AmqpManager.cpp

Loading...