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

Authored by • max on Aug 24 2022, 6:27 AM.

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)

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

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

Path

Size

services/

tunnelbroker/

src/

Amqp/

AmqpManager.cpp

1 line

Diff 15925

View Options

services/tunnelbroker/src/Amqp/AmqpManager.cpp

[services] Tunnelbroker - Add wait until amqp ready in a first initialization callAbandonedPublicActions