Remove deterministic download schedule code and configs
Under exponential backoff, download schedules contain the maximum time we will wait, even if the random amount is larger. initial time we will wait, and everything else is random exponential from that point onwards.
But in most cases, the random amount is much smaller than the maximum, so we could replace the item with the actual maximum, or delete it from the schedule altogether. (On the public network, the maximum is 4x the last entry, on test networks, it's 3x.)
So once we're sure that we will never revert to deterministic schedules, we should make each schedule into a single initial value, and remove the deterministic code.
We should make these changes based on the schedules in #23347 (moved).
Activity
changed milestone to %Tor: 0.3.4.x-final
Trac:
Description: Under exponential backoff, download schedules contain the maximum time we will wait, even if the random amount is larger.But in most cases, the random amount is much smaller than the maximum, so we could replace the item with the actual maximum, or delete it from the schedule altogether. (On the public network, the maximum is 4x the last entry, on test networks, it's 3x.)
I'll make these changes based on the schedules in #23347 (moved).
to
Under exponential backoff, download schedules contain the maximum time we will wait, even if the random amount is larger.
But in most cases, the random amount is much smaller than the maximum, so we could replace the item with the actual maximum, or delete it from the schedule altogether. (On the public network, the maximum is 4x the last entry, on test networks, it's 3x.)
We should make these changes based on the schedules in #23347 (moved).
Trac:
Summary: Remove redundant items from download schedules to Remove deterministic download schedule code and configs
Description: Under exponential backoff, download schedules contain the maximum time we will wait, even if the random amount is larger.But in most cases, the random amount is much smaller than the maximum, so we could replace the item with the actual maximum, or delete it from the schedule altogether. (On the public network, the maximum is 4x the last entry, on test networks, it's 3x.)
We should make these changes based on the schedules in #23347 (moved).
to
Under exponential backoff, download schedules contain the
maximum time we will wait, even if the random amount is larger.initial time we will wait, and everything else is random exponential from that point onwards.But in most cases, the random amount is much smaller than the maximum, so we could replace the item with the actual maximum, or delete it from the schedule altogether. (On the public network, the maximum is 4x the last entry, on test networks, it's 3x.)So once we're sure that we will never revert to deterministic schedules, we should make each schedule into a single initial value, and remove the deterministic code.
We should make these changes based on the schedules in #23347 (moved).
#24151 (moved) is a duplicate; copied text from that ticket is:
It looks like the only code that uses DL_SCHED_DETERMINISTIC is testing code. We should rip out all code related to it. This will make it easier to refactor in the future and reduce the binary size by eliminating dead code.
Trac:
Keywords: N/A deleted, technical-debt, refactor, s8-perf added
Milestone: Tor: 0.3.4.x-final to Tor: 0.3.3.x-final
Cc: N/A to catalyst
Sponsor: N/A to Sponsor8
Whoops; look like we had another duplicate as #23814 (moved). But let's keep this ticket around for just removing the configs.
Here's how we can remove these configs:
- turn each download schedule config option into a single non-negative integer
- set the default config value to the first item in each list
- most schedules will be 0, so merge those schedules together, removing conditional code
- maybe rename *DownloadSchedule to *DownloadInitialDelay
-
As requested, this is done in
ticket23354. Pull request at https://github.com/torproject/tor/pull/54Trac:
Status: accepted to needs_review There is an extra commit c9c26d0 at the start of this branch.
I wonder if it is ok that our coverage dropped slightly. But I think that's ok, because we removed 50 lines of code.
When did we stop using deterministic schedules? I think it was 0.2.9. Do we want to say 0.2.9 in the changes file?
The rest seems sensible to me, but I would like someone else to review this too.
I opened #25844 (moved) for the relevant chutney changes.
-
There is an extra commit c9c26d0 at the start of this branch.
Okay; I've rebased the branch to remove this commit.
I think it was 0.2.9. Do we want to say 0.2.9 in the changes file?
Makes sense; doing that too.
I've re-pushed the branch: there are no other changes.
The rest seems sensible to me, but I would like someone else to review this too.
Okay -- into needs_review it goes!
Trac:
Status: needs_revision to needs_review -
You suggested, wrt
getinfo_helper_config():we might want to change this to something that does NOT say "comma list"
So, these values still accept the same format as before (ignoring everything except the first entry), and they still emit a subset of the format that they emitted before.
On the other hand, they are closer to TimeInterval now than they were before. So I'l change them to that. Done in commit b205061eb13abd68a5335f8008c5121ef230c34e.
Also, merged.
Trac:
Status: merge_ready to closed
Resolution: N/A to implemented mentioned in issue #23814 (moved)
mentioned in issue #24151 (moved)
mentioned in issue #24152 (moved)
mentioned in issue #25844 (moved)
mentioned in issue tpo/core/chutney#25844 (closed)
moved to tpo/core/tor#23354 (closed)
mentioned in issue tpo/core/tor#24152 (closed)
