Opened 3 years ago

Closed 2 years ago

#20162 closed enhancement (implemented)

reduce configuration parameters in collector.properties

Reported by: iwakeh Owned by:
Priority: Medium Milestone: CollecTor 1.1.0
Component: Metrics/CollecTor Version:
Severity: Normal Keywords:
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

The number of parameters in collector.properties will very soon surpass fifty. It should be reduced where it makes sense.

This ticket collects possible ways to achieve the reduction.

Child Tickets

Change History (11)

comment:1 Changed 3 years ago by karsten

Sounds good. Some suggestions to start this process:

  • Remove the following six config options that could be used to disable downloading certain descriptor types in the relaydescs module: DownloadCurrentConsensus, DownloadCurrentMicrodescConsensus, DownloadCurrentVotes, DownloadMissingServerDescriptors, DownloadMissingExtraInfoDescriptors, and DownloadMissingMicrodescriptors. These can all be hard-coded to true, which is also their current default value. I'd argue that whoever sets up a CollecTor instance to download relay descriptors from the directory authorities will likely want to download all available descriptor types.
  • Remove the ReplaceIpAddressesWithHashes option but hard-code it as true rather than the current default value false. There shouldn't be sanitized bridge descriptors with IP addresses 127.0.0.1, but those should all be replaced with IP address hashes.
  • While we're at it, replace the default value of BridgeDescriptorMappingsLimit with 90, because that's a much more reasonable default for a production setting than inf. (And while we're at this, there's also a bug that makes inf do something different than one would expect, but I didn't file that one yet.) However, let's keep this config option in case we ever want/have to reprocess past bridge descriptors.

comment:2 Changed 3 years ago by iwakeh

Great! Seven properties less. A good start.

I would suggest replacing the following:

  • DirectoryArchivesOutputDirectory
  • SanitizedBridgesWriteDirectory
  • ExitlistOutputDirectory
  • TorperfOutputDirectory

by a simple OutputDirectory with the default value out.
The paths below should not be changed and can be hard-coded.

Please, file the bug for the mappings-limit value!

comment:3 Changed 3 years ago by karsten

Agreed, simplifying directory options as you suggest sounds like a fine plan. Filed #20224 for the mapping-limit thing.

comment:4 in reply to:  2 ; Changed 3 years ago by iwakeh

Reminder:

...
replace

  • DirectoryArchivesOutputDirectory
  • SanitizedBridgesWriteDirectory
  • ExitlistOutputDirectory
  • TorperfOutputDirectory

by a simple OutputPath with the default value out.

This is part of #18910.
So, any changes for the other properties should be based on the commits of #18910.

comment:5 in reply to:  4 Changed 3 years ago by iwakeh

The *OutpuDirectoryProperties are replaced cf. master 5003567.

comment:6 Changed 3 years ago by iwakeh

Streamline the naming of all properties, i.e. Relay* instead of Relaydescs* and similar.

As there are many obsolete properties by now there should be a check informing operators of redundant properties in their config file.

comment:7 Changed 3 years ago by iwakeh

Milestone: CollecTor 2.0.0CollecTor 1.2.0
Summary: reduce configuration paramaters in collector.propertiesreduce configuration parameters in collector.properties

This is far enough progressed to be part of 1.2.0 already.
Corrected typo in title.

comment:8 Changed 2 years ago by iwakeh

Status: newneeds_review

Removed the three DownloadCurrent* and DownloadMissing* properties in this branch.

comment:9 Changed 2 years ago by iwakeh

Milestone: CollecTor 1.2.0CollecTor 1.1.0

comment:10 Changed 2 years ago by karsten

Merged together with a change log entry. Please close if that concludes the ticket. Thanks!

comment:11 Changed 2 years ago by iwakeh

Resolution: implemented
Status: needs_reviewclosed

All done for now. Removed 10 configuration parameter and introduced one.

Closing.

Thanks!

Note: See TracTickets for help on using tickets.