Commit 2fc48f15 authored by Steven Allen's avatar Steven Allen

correctly set the initial total

The previous attempt actually make the problem worse:

1. It didn't update the "last update" time.
2. It updated the accumulator but then didn't set the total. That meant that the
_next_ update would count the bandwidth from two time periods.

This change also reverts the changes to the test (the test was right, the code
was wrong).
parent ce5ebda5
......@@ -108,9 +108,6 @@ func TestUnregister(t *testing.T) {
m := new(Meter)
go func() {
defer wg.Done()
m.Mark(1)
time.Sleep(time.Second)
ticker := time.NewTicker(100 * time.Millisecond)
defer ticker.Stop()
for i := 0; i < 40; i++ {
......@@ -120,9 +117,6 @@ func TestUnregister(t *testing.T) {
time.Sleep(62 * time.Second)
m.Mark(2)
time.Sleep(time.Second)
for i := 0; i < 40; i++ {
m.Mark(2)
<-ticker.C
......@@ -130,7 +124,7 @@ func TestUnregister(t *testing.T) {
}()
go func() {
defer wg.Done()
time.Sleep(40*100*time.Millisecond + time.Second)
time.Sleep(40 * 100 * time.Millisecond)
actual := m.Snapshot()
if !approxEq(actual.Rate, 10, 1) {
......@@ -143,10 +137,10 @@ func TestUnregister(t *testing.T) {
}
actual = m.Snapshot()
if actual.Total != 41 {
t.Errorf("expected total 41, got %d", actual.Total)
if actual.Total != 40 {
t.Errorf("expected total 4000, got %d", actual.Total)
}
time.Sleep(3*time.Second + 40*100*time.Millisecond)
time.Sleep(2*time.Second + 40*100*time.Millisecond)
actual = m.Snapshot()
if !approxEq(actual.Rate, 20, 4) {
......@@ -154,8 +148,8 @@ func TestUnregister(t *testing.T) {
}
time.Sleep(2 * time.Second)
actual = m.Snapshot()
if actual.Total != 123 {
t.Errorf("expected total 123, got %d", actual.Total)
if actual.Total != 120 {
t.Errorf("expected total 120, got %d", actual.Total)
}
if atomic.LoadUint64(&m.accumulator) == 0 {
t.Error("expected meter to be active")
......
......@@ -15,7 +15,7 @@ func TestRegistry(t *testing.T) {
m1.Mark(10)
m2.Mark(30)
time.Sleep(2 * time.Second)
time.Sleep(2*time.Second + time.Millisecond)
if total := r.Get("first").Snapshot().Total; total != 10 {
t.Errorf("expected first total to be 10, got %d", total)
......
......@@ -151,11 +151,15 @@ func (sw *sweeper) update() {
sw.meters[i] = sw.meters[newLen]
}
// Re-add the total to all the newly active accumulators.
// Re-add the total to all the newly active accumulators and set the snapshot to the total.
// 1. We don't do this on register to avoid having to take the snapshot lock.
// 2. We skip calculating the bandwidth for this round so we get an _accurate_ bandwidth calculation.
for _, m := range sw.meters[sw.activeMeters:] {
atomic.AddUint64(&m.accumulator, m.snapshot.Total)
total := atomic.AddUint64(&m.accumulator, m.snapshot.Total)
if total > m.snapshot.Total {
m.snapshot.LastUpdate = now
}
m.snapshot.Total = total
}
// trim the meter list
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment