#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! Thanks! > > [...] 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 agree): > 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 module. > > 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 instances. > * `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 source. 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. Thanks! > > 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 tor-bugs@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs