#18910: distributing descriptors accross CollecTor instances
 Reporter:  iwakeh             |          Owner:  iwakeh
     Type:  enhancement        |         Status:  needs_review
 Priority:  High               |      Milestone:  CollecTor 1.1.0
Component:  Metrics/CollecTor  |        Version:
 Severity:  Normal             |     Resolution:
 Keywords:  ctip               |  Actual Points:
Parent ID:                     |         Points:
 Reviewer:                     |        Sponsor:

Comment (by karsten):

 Replying to [comment:21 iwakeh]:
 > Replying to [comment:20 karsten]:
 > > [...] Shouldn't we instead remove those config options entirely, [...]
 > This is only a warning that these will be deprecated.  They're still
 used, but already adressed in #20162.  But they should be removed here.
 That wouldn't be a small commit, though, and I'd rather have it separate.
 I'll add a new branch just up to that commit later today.

 Okay.  I was confused by the comment saying "only OutputPath is used".
 Looking forward to the new branch you're mentioning.

 > >  - Speaking of, can you include a change log entry for this commit?
 > Sure!


 > > [...] I wonder if we could simplify the configuration by avoiding that
 tri-state SyncType option and turning it into a boolean. [...]
 > Sync-Options (this description should be added to package-info, when we
 > There are currently three modules that can be synced: relaydescs,
 bridgedescs, and exitlists.
 > A module can be turned on or off via its `*Activated` option, which is
 only configurable at start-up and determines if anything is run from this
 > The runtime configurable tri-state `Sync*` option values are
 > * `NoSync`:  for simply running the module w/o fetching additional data.
 Useful when just the directly (from the Tor network) accessible data is of
 interest, or when having access to bridgedescs etc.
 > * `Sync` performs the module run and then fetches data from other
 > * `SyncOnly` just fetches data from other instances.  Useful for mirrors
 that don't have access to bridgedescs, or that fetch from a main instance
 they trust and don't have net access otherwise, etc.
 > `Sync*` options can be adapted during runtime.  It is possible to switch
 to syncing and then turn it off during runtime.
 > So, reducing the tri-state would complicate the combination of
 'activated' and sync-settings, which is separated from sync, i.e.
 `Scheduler` doesn't and shouldn't now about syncing.

 Hmm, I don't understand how the change I suggested would affect the
 ability to reconfigure at runtime.  I admit I didn't read that code yet
 (which comes in later commits, I think), but in theory it should still be
 possible to enable or disable syncing if it's yet another descriptor

 Here's how I'd explain configuring the relaydescs module to a new
 operator: "Want to collect relay descriptors?  Sure, just activate that
 module and also enable one or more descriptor sources: download from the
 directory authorities, read from a local directory, read from local cached
 descriptors, synchronize from other CollecTor instances.  If you don't
 enable any descriptor sources, you won't get any descriptors."

 This feels easier than: "Want to collect relay descriptors?  Sure, just
 activate that module and also enable between zero and three descriptors
 sources, and also consider synchronizing from other CollecTor instances."

 The three modules could still use the same code for synchronizing, so I'm
 not asking to change much code here.  But I'm worried that the tri-state
 config option will be harder to grasp for operators than it has to be.

 > >  - I found at least one long line that checkstyle should complain
 about, though I didn't run it myself.
 > Oh, I usually run this at the last commit.  Will check again.

 Okay, no rush. :)

 > > In case you want to start working on any of these comments, can you
 please write `--fixup` commits that resolve those issues in this
 particular commit?  In any case, please don't modify commits 2 to 7 in
 that branch at this point, because I already started reviewing those.
 > As said above, I'll add another branch with commits we agree on and in a
 way that the commits make sense in the final master version.


 > > More tomorrow.  Thanks!
 > Thanks, for reading your way through all that un-commented code!

 Still reviewing the other other commits...

Ticket URL: <https://trac.torproject.org/projects/tor/ticket/18910#comment:22>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online
tor-bugs mailing list

Reply via email to