Accounting Resets when System Timezone Changes
If you have one timezone set and burn through your bandwidth, then change the system timezone and restart Tor, it will reset and burn through the bandwidth a second time. This causes double the intended bandwidth to get used.
Trac:
Username: BarkerJr
Activity
Hm. This is probably our fault for having the accounting code use local time instead of UTC. This will probably need some thinking to get right...
Are there any interesting messages in the log when Tor starts up in the new timezone? In particular, I'm wondering if you're seeing the warning, "Mismatched accounting interval; starting a fresh one."
-
On further investigation, the culprit code is probably the logic in configure_accounting() that resets the accounting period if the start of the current accounting period is not the same as what it was before.
It's not totally clear what to do in this case. If the start of the accounting period moves back in time, then it's probably okay to remember the bytes we already wrote. But what should we do if it moves forward in time, yet still overlaps with the previous accounting period? (Ignore for now the fact that we don't record when the old period was supposed to end.)
One easy fix would be to add some way to specify that accounting periods are to be taken relative to UTC rather than local time, but that leaves the major issue unaddressed.
The only real interesting log entry I got was this: May 17 03:29:10.041 [warn] Mismatched accounting interval; starting a fresh one.
The state file had changed. Old: AccountingIntervalStart 2010-05-01 04:00:00 New: AccountingIntervalStart 2010-05-01 00:00:00
I have a snapshot of the old /usr/local/var/lib/tor if needed.
Trac:
Username: BarkerJr-
So, here's the best revised behavior I can think of on changing an accounting period in progress:
- If the period start moves backwards, remember the old bytes.
- If the period start moves forwards, and there's more than 2% overlap, remember the old bytes.
- Otherwise, if the period start moves forward and the old period overlaps the new one less than 2%, then don't remember the old bytes.
But before we implement this, we should figure out bug #1789 (moved), which might mean that accounting is broken in other ways.
Triage: if you're in a timezone that does daylight saving time, does this bug hit you? If so, it's a bug that should be resolved, perhaps in 0.2.1.x also.
Nick says "It's not too hard, either, if we do the hack suggested in my last comment."
Would be good to get solved for the rc.
Trac:
Priority: normal to major-
Looks fine to me. Green light to merge when you like.
(That being said, I hate time comparison calculations. :)
I will probably revise this to a 'major bugfix' when I'm making the changelog, on the assumption that this really is a bug that happens when a machine moves into or out of daylight saving time. If that's true, we should probably merge this into 0.2.1.x and then on to master. It's a serious bug.
All of that said, one question: you say "if it changes by up to 50% either way", but then your comment says "The start of the period is now a little later than we remembered." If the period has shifted forward, isn't the old time now in the previous interval, meaning
start_of_accounting_period_containing(interval_start_time)would be like a month earlier?
-
arg, I think you're right. In the case where the period shifts forward, we'll hit the first check in configure_accounting, which checks for
start_of_accounting_period_after(interval_start_time) <= nowand advances the accounting period.
Not quite sure what the right way to express the fix is. I'll take a crack at it... have a look at the latest patch on my bug1511 branch? If you like it, I'll backport to 0.2.1.x and do the merging
hrm. BarkerJr reported #2151 (closed) and #2146 (moved), might they be related? Or is it all #2003 (moved)
Replying to Sebastian:
hrm. BarkerJr reported #2151 (closed) and #2146 (moved), might they be related? Or is it all #2003 (moved) I think you mean #2152 (moved), not #2151 (closed).
-
Haven't reviewed patch yet, but since we think this triggers on DST, it is probably a good idea to backport in for 0.2.1.28.
That said, I think we should block the backport until we resolve #2146 (moved).
-
Right now I'm thinking the best we can do for users here is not to backport this (or #2146 (moved)) to 0.2.1. Our chances of getting it wrong are high enough, and the workaround ("upgrade to 0.2.2.x") is simple enough that we should call this "won't fix" in the 0.2.1. series.
Trac:
Actualpoints: N/A to N/A
Points: N/A to N/A -
Closing this one since we decided to abandon the backport to 0.2.1.
I opened #3020 (moved) for the new concern on 0.2.2 (but we need more details before we have a prayer of fixing it).
Trac:
Resolution: N/A to fixed
Status: needs_review to closed mentioned in issue #2146 (moved)
mentioned in issue #3020 (moved)
moved to tpo/core/tor#1511 (closed)
mentioned in issue tpo/core/tor#3020 (closed)