introducer client uses cache even if it doesn't have to #2859

New Issue

tahoe-lafs · 2016-12-29T19:44:42Z

dawuud commented

2016-12-29 19:44:42 +00:00

Hello, I'm at 33c3 with str4d discussing the introducer client. He's report a connectivity ordering problem in #2858 however this also brought our attention to the currenct introducer client implementation: the introducer client connects to the cached storage servers even if the introducer is available. Is this a bug?

Shouldn't the introducer client try and fail to connect to the introducer before using the introducer cache and connecting to the cache storage node addresses?

Hello, I'm at 33c3 with str4d discussing the introducer client. He's report a connectivity ordering problem in #2858 however this also brought our attention to the currenct introducer client implementation: the introducer client connects to the cached storage servers even if the introducer is available. Is this a bug? Shouldn't the introducer client try and fail to connect to the introducer before using the introducer cache and connecting to the cache storage node addresses?

tahoe-lafs added the

labels 2016-12-29 19:44:42 +00:00

tahoe-lafs added this to the undecided milestone 2016-12-29 19:44:42 +00:00

dawuud commented

2016-12-29 19:55:38 +00:00

the introducer client's startService calls self._load_announcements() which loads the cache AND then calls _deliver_announcements which starts eventually connecting to all the storage nodes.

i could write a patch to fix this but i want to make sure it's the right thing to do.
thoughts?

the introducer client's startService calls self._load_announcements() which loads the cache AND then calls _deliver_announcements which starts eventually connecting to all the storage nodes. i could write a patch to fix this but i want to make sure it's the right thing to do. thoughts?

warner commented

2016-12-29 20:10:07 +00:00

Huh. Looking at the code (IntroducerClient.startService), it looks like it will only read from the cache (_load_announcements) if a single getReference fails (via the errback to connect_failed). If that initial connection succeeds, there's no addCallback, so nothing will ever read from the cache.

Specifically, it looks like we start a Reconnector (tub.connectTo()), then immediately start a normal tub.getReference() to the same FURL. If/when the Reconnector succeeds, we'll eventually overwrite the cache file. If/when the normal connector fails, we'll load the cache.

Both of those are supposed to use the same connection attempt. I'm not sure which service will be startServiced first. If the Tub is running before IntroducerClient.startService is called, then the connectTo should create the outbound connection, and the getReference should wait for it to complete (success or failure). If the Tub isn't running by that point, then both connectTo and getReference will be queued, and then maybe the order in which they trigger outbound connections will be unpredictable. But the second one is still supposed to not start a second independent connection: it should see that a connection is already in progress, and attach itself to wait for the results of that one.

Huh. Looking at the code (`IntroducerClient.startService`), it looks like it will only read from the cache (`_load_announcements`) if a single `getReference` fails (via the errback to `connect_failed`). If that initial connection succeeds, there's no addCallback, so nothing will ever read from the cache. Specifically, it looks like we start a Reconnector (`tub.connectTo()`), then immediately start a normal `tub.getReference()` to the same FURL. If/when the Reconnector succeeds, we'll eventually overwrite the cache file. If/when the normal connector *fails*, we'll load the cache. Both of those are supposed to use the same connection attempt. I'm not sure which service will be `startService`d first. If the Tub is running before `IntroducerClient.startService` is called, then the `connectTo` should create the outbound connection, and the `getReference` should wait for it to complete (success or failure). If the Tub isn't running by that point, then both `connectTo` and `getReference` will be queued, and then maybe the order in which they trigger outbound connections will be unpredictable. But the second one is still supposed to *not* start a second independent connection: it should see that a connection is already in progress, and attach itself to wait for the results of that one.

warner added

code-network

and removed

unknown

labels 2016-12-29 20:17:36 +00:00

warner modified the milestone from undecided to 1.12.1

2016-12-29 20:17:36 +00:00

tahoe-lafs added

minor

and removed

normal

labels 2017-01-10 17:31:12 +00:00