[tor-bugs] #13718 [Tor]: Reachability Tests aren't conducted if there are no exit nodes

[Tor Bug Tracker & Wiki]

#13718: Reachability Tests aren't conducted if there are no exit nodes
-------------------------+-------------------------------------------------
     Reporter:  tom      |      Owner:  teor
         Type:  defect   |     Status:  needs_review
     Priority:  normal   |  Milestone:  Tor: 0.2.6.x-final
    Component:  Tor      |    Version:  Tor: 0.2.6.1-alpha
   Resolution:           |   Keywords:  tor-relay test-network lorax
Actual Points:           |  chutney 026-deferrable
       Points:           |  Parent ID:  #14034
-------------------------+-------------------------------------------------

Comment (by teor):

 Replying to [comment:46 nickm]:
 > Hi!  Everything below is something I can do when merging; I'd just like
 your feedback before I do so.
 >
 > 4a41976753c9e916acf12d6156be5fab7f534f07
 >   * The parenthesis bug me a little on the conditional. Maybe put the
 whole inequality on a single line.  (minor)

 Oh yes, it looks ugly, doesn't it?
 (It's funny how you look at your own work differently after a few days...)

 Please rearrange as you see fit.

 >
 > fc92ba6b9c9d85c546b94096e6d9bf76a3f38e47
 >   * the message should probably be a notice again (minor)

 Happy to keep it as a notice. I bumped it down to info for being spammy.
 Then I implemented the suppression, which I tried to adjust to keep it
 from being too annoying on the public tor network. What do you think of
 the 60 second suppression interval?
 (It applies to each kind of message separately, so the user can get a "we
 don't have enough descriptors" message, followed shortly by a message as
 soon as we do have enough descriptors.)

 Also, the message tends to appear more often on test or private networks -
 where users are less likely to be alarmed by it anyway.

 >
 > f93b46adac531406e1cc13668ceecd3e5fca650a
 >   * The expression for setting have_path is pretty complex; I'd like to
 turn that into a function that uses a set of ifs. (minor)

 Yes, this makes sense since I used exactly the same code twice. And
 boolean expressions embedded within a ternary operator just look confusing
 to me now, even though I'm ''sure'' I understood what they did at the
 time.

 >
 > Do you agree with these changes?  If so I think I can go ahead and merge
 and tweak.

 These sound like good changes - happy to make the code more readable. (At
 the time, I was more focused on chopping away at it until it did what
 dgoulet, tom and I wanted.)

 I am looking forward to a working, quick-bootstrapping tor/chutney
 combination.

 Thanks for your work reviewing all these commits and suggesting changes -
 I understand it's a sizeable set of changes that you've reviewed multiple
 times.

 I'm really happy we managed to get such a dramatic improvement in the load
 times (and in time for 2.6.1, as long as nothing seriously breaks during
 alpha testing).

 I also think it's interesting the bootstrap time now matches the original
 implementation of src/test/test-network.sh, which gives chutney 18 seconds
 to configure and bootstrap. Makes me suspect there was a version of tor
 that bootstrapped within about 10 seconds, and then something broke.

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/13718#comment:47>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online