[tor-bugs] #21846 [Metrics/Torflow]: BwAuthority can't be run out of the box without manual work

Tor Bug Tracker & Wiki blackhole at torproject.org
Thu May 4 02:17:28 UTC 2017


#21846: BwAuthority can't be run out of the box without manual work
-------------------------------+--------------------------------
 Reporter:  davidwf            |          Owner:  aagbsn
     Type:  enhancement        |         Status:  needs_revision
 Priority:  Medium             |      Milestone:
Component:  Metrics/Torflow    |        Version:
 Severity:  Normal             |     Resolution:
 Keywords:                     |  Actual Points:
Parent ID:                     |         Points:
 Reviewer:  chelseakomlo, tom  |        Sponsor:
-------------------------------+--------------------------------
Changes (by teor):

 * status:  merge_ready => needs_revision


Comment:

 davidwf: thanks for this patch!

 In general:

 Why did you downgrade from SQLAlchemy 0.7.10 to 0.7.8?
 Whichever way you choose to go, please find all instances of
 SQLAlchemy-0.7.* and make them consistent.

 For future patches, it is ok to provide a branch and use multiple commits
 - in fact, we prefer it!

 In particular, it is nice if whitespace changes are in a separate commit
 to functional changes.

 In particular:

 In setup.sh and setup_cron.sh:

 readlink -f works with gnu readlink, but not with BSD readlink. It is
 possible to install BSD readlink on Linux, and GNU readlink on BSD or
 macOS. So we can't depend on uname, we should just try basename if
 readlink doesn't work here:
 {{{
 +# readlink -f does not work on Mac
 +if [ `uname` != "Darwin" ]
 +then
 +  SCANNER_DIR=$(readlink -f "$SCANNER_DIR")
 +fi
 }}}
 Also, let's use the same code in all the scripts to do this, for
 consistency.

 Some OSs only have a non-versioned python (e.g. macOS), so we should fall
 back to "which python" here:
 {{{
 +PYTHON=$(which python2.7 || which python2.6)
 }}}

 In stop_scan.sh:

 This is a dangerous strategy if the process has already died, and its PID
 is being used by something else. Can we check the process has the right
 name in a cross-platform way?
 {{{
 kill -9 `head -1 $PIDFILE` && rm $PIDFILE
 }}}

 You may also be better making the parent scanner script Tor's
 __OwningControllerProcess, so tor exits automatically when the script
 exits. But you'll just have to kill the bwauthority child processes.

 Please don't copy environmental variables like this:
 {{{
 +SCANNERS_PER_TOR_COUNT=4
 +TOR_COUNT=2
 +SCANNER_COUNT=$(($SCANNERS_PER_TOR_COUNT * $TOR_COUNT + 1))
 }}}

 It makes it easy for them to get out of sync with the launch script.
 Also, if someone changes the variables, then stops the scanners, some will
 be left over.
 Instead, why not do:
 {{{
 for PIDFILE in ./data/scanner.*/bwauthority.pid
 }}}
 and the same for the tor instances.

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


More information about the tor-bugs mailing list