From 80b3de8753225f169e4269503d63c273562345e7 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 3 Sep 2010 12:06:04 -0400 Subject: [PATCH 1/2] Tolerate a little skew in accounting intervals. This will make changes for DST still work, and avoid double-spending bytes when there are slight changes to configurations. Fixes bug 1511; the DST issue is a bugfix on 0.0.9pre5. --- changes/bug1511 | 9 +++++++++ src/or/hibernate.c | 29 +++++++++++++++++++++++++---- 2 files changed, 34 insertions(+), 4 deletions(-) create mode 100644 changes/bug1511 diff --git a/changes/bug1511 b/changes/bug1511 new file mode 100644 index 0000000000..8e9652b90e --- /dev/null +++ b/changes/bug1511 @@ -0,0 +1,9 @@ + o Minor bugfixes: + - Tolerate skew in stored vs computed interval starts for bandwidth + accounting. Now, if we change our configuration so that the start + of the period changes by no more than 50% of the period's duration, + we remember bytes that we transferred in the old one. The upshot + of this is that daylight savings time should no longer mess up + bandwidth accounting and make each period potentially happen + twice. Fixes bug 1511; bugfix on 0.0.9pre5. + diff --git a/src/or/hibernate.c b/src/or/hibernate.c index d50d05ed5e..994b7ea932 100644 --- a/src/or/hibernate.c +++ b/src/or/hibernate.c @@ -341,6 +341,15 @@ start_of_accounting_period_after(time_t now) return edge_of_accounting_period_containing(now, 1); } +/** Return the length of the accounting period containing the time + * now. */ +static long +length_of_accounting_period_containing(time_t now) +{ + return edge_of_accounting_period_containing(now, 1) - + edge_of_accounting_period_containing(now, 0); +} + /** Initialize the accounting subsystem. */ void configure_accounting(time_t now) @@ -356,14 +365,26 @@ configure_accounting(time_t now) log_info(LD_ACCT, "Starting new accounting interval."); reset_accounting(now); } else if (interval_start_time == - start_of_accounting_period_containing(interval_start_time)) { + start_of_accounting_period_containing(interval_start_time)) { log_info(LD_ACCT, "Continuing accounting interval."); /* We are in the interval we thought we were in. Do nothing.*/ interval_end_time = start_of_accounting_period_after(interval_start_time); } else { - log_warn(LD_ACCT, - "Mismatched accounting interval; starting a fresh one."); - reset_accounting(now); + time_t s_now = start_of_accounting_period_containing(interval_start_time); + long duration = length_of_accounting_period_containing(interval_start_time); + double delta = ((double)(s_now - interval_start_time)) / duration; + if (-0.50 <= delta && delta <= 0.50) { + /* The start of the period is now a little later than we remembered. + * That's fine; we might lose a little time, but that's ok. */ + log_info(LD_ACCT, "Accounting interval moved by %.02f%%; " + "that's fine.", delta*100); + interval_end_time = start_of_accounting_period_after(interval_start_time); + } else { + log_warn(LD_ACCT, + "Mismatched accounting interval: moved by %.02f%%. " + "Starting a fresh one.", delta*100); + reset_accounting(now); + } } accounting_set_wakeup_time(); } From 52db5c2539413aded9a7fb855030c6a3a7a25798 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Tue, 21 Sep 2010 14:59:43 -0400 Subject: [PATCH 2/2] Even more accurate handling for shifting accounting intervals Roger correctly pointed out that my code was broken for accounting periods that shifted forwards, since start_of_accounting_period_containing(interval_start_time) would not be equal to interval_start_time, but potentially much earlier. --- src/or/hibernate.c | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/src/or/hibernate.c b/src/or/hibernate.c index 994b7ea932..eb9c1cf6d6 100644 --- a/src/or/hibernate.c +++ b/src/or/hibernate.c @@ -354,31 +354,38 @@ length_of_accounting_period_containing(time_t now) void configure_accounting(time_t now) { + time_t s_now; /* Try to remember our recorded usage. */ if (!interval_start_time) read_bandwidth_usage(); /* If we fail, we'll leave values at zero, and * reset below.*/ - if (!interval_start_time || - start_of_accounting_period_after(interval_start_time) <= now) { - /* We didn't have recorded usage, or we don't have recorded usage - * for this interval. Start a new interval. */ + + s_now = start_of_accounting_period_containing(now); + + if (!interval_start_time) { + /* We didn't have recorded usage; Start a new interval. */ log_info(LD_ACCT, "Starting new accounting interval."); reset_accounting(now); - } else if (interval_start_time == - start_of_accounting_period_containing(interval_start_time)) { + } else if (s_now == interval_start_time) { log_info(LD_ACCT, "Continuing accounting interval."); /* We are in the interval we thought we were in. Do nothing.*/ interval_end_time = start_of_accounting_period_after(interval_start_time); } else { - time_t s_now = start_of_accounting_period_containing(interval_start_time); - long duration = length_of_accounting_period_containing(interval_start_time); + long duration = length_of_accounting_period_containing(now); double delta = ((double)(s_now - interval_start_time)) / duration; if (-0.50 <= delta && delta <= 0.50) { - /* The start of the period is now a little later than we remembered. - * That's fine; we might lose a little time, but that's ok. */ + /* The start of the period is now a little later or earlier than we + * remembered. That's fine; we might lose some bytes we could otherwise + * have written, but better to err on the side of obeying people's + * accounting settings. */ log_info(LD_ACCT, "Accounting interval moved by %.02f%%; " "that's fine.", delta*100); - interval_end_time = start_of_accounting_period_after(interval_start_time); + interval_end_time = start_of_accounting_period_after(now); + } else if (delta >= 0.99) { + /* This is the regular time-moved-forward case; don't be too noisy + * about it or people will complain */ + log_info(LD_ACCT, "Accounting interval elapsed; starting a new one"); + reset_accounting(now); } else { log_warn(LD_ACCT, "Mismatched accounting interval: moved by %.02f%%. "