From 7930ef7cf46b2d861b7e47dbe38db9c15db5b754 Mon Sep 17 00:00:00 2001 From: carla Date: Tue, 8 Sep 2020 13:47:16 +0200 Subject: [PATCH] chanfitness: make online period calculation tolerant of duplicates To get our uptime, we first filter our event log to get online periods. This change updates this code to be tolerant of consecutive online or offline events in the log. This will be required for rate limiting, because we will not record every event for anti-dos reasons, so we could record an online event, ignore an offline event and then record another offline event. We could just ignore this duplicate event, but we will also need this tolerance for when we persist uptime and our peers can have their last event before restart as an online event and record another online event when we come back up. --- chanfitness/chanevent.go | 89 +++++++++++++++++---------- chanfitness/chanevent_test.go | 111 ++++++++++++++++++++++++++++++++-- 2 files changed, 164 insertions(+), 36 deletions(-) diff --git a/chanfitness/chanevent.go b/chanfitness/chanevent.go index 1125f373..6a3af2aa 100644 --- a/chanfitness/chanevent.go +++ b/chanfitness/chanevent.go @@ -107,8 +107,11 @@ type onlinePeriod struct { // getOnlinePeriods returns a list of all the periods that the event log has // recorded the remote peer as being online. In the unexpected case where there // are no events, the function returns early. Online periods are defined as a -// peer online event which is terminated by a peer offline event. This function -// expects the event log provided to be ordered by ascending timestamp. +// peer online event which is terminated by a peer offline event. If the event +// log ends on a peer online event, it appends a final period which is +// calculated until the present. This function expects the event log provided +// to be ordered by ascending timestamp, and can tolerate multiple consecutive +// online or offline events. func (e *chanEventLog) getOnlinePeriods() []*onlinePeriod { // Return early if there are no events, there are no online periods. if len(e.events) == 0 { @@ -116,8 +119,11 @@ func (e *chanEventLog) getOnlinePeriods() []*onlinePeriod { } var ( - lastOnline time.Time - offline bool + // lastEvent tracks the last event that we had that was of + // a different type to our own. It is used to determine the + // start time of our online periods when we experience an + // offline event, and to track our last recorded state. + lastEvent *channelEvent onlinePeriods []*onlinePeriod ) @@ -128,51 +134,72 @@ func (e *chanEventLog) getOnlinePeriods() []*onlinePeriod { // recent event is tracked using the offline bool so that we can add a // final online period if necessary. for _, event := range e.events { - switch event.eventType { case peerOnlineEvent: - lastOnline = event.timestamp - offline = false - - case peerOfflineEvent: - offline = true - - // Do not add to uptime if there is no previous online - // timestamp, the event log has started with an offline - // event - if lastOnline.IsZero() { - continue + // If our previous event is nil, we just set it and + // break out of the switch. + if lastEvent == nil { + lastEvent = event + break } - // The eventLog has recorded an offline event, having - // previously been online so we add an online period to - // set of online periods. - onlinePeriods = append(onlinePeriods, &onlinePeriod{ - start: lastOnline, - end: event.timestamp, - }) + // If our previous event was an offline event, we update + // it to this event. We do not do this if it was an + // online event because duplicate online events would + // progress our online timestamp forward (rather than + // keep it at our earliest online event timestamp). + if lastEvent.eventType == peerOfflineEvent { + lastEvent = event + } + + case peerOfflineEvent: + // If our previous event is nil, we just set it and + // break out of the switch since we cannot record an + // online period from this single event. + if lastEvent == nil { + lastEvent = event + break + } + + // If the last event we saw was an online event, we + // add an online period to our set and progress our + // previous event to this offline event. We do not + // do this if we have had duplicate offline events + // because we would be tracking the most recent offline + // event (rather than keep it at our earliest offline + // event timestamp). + if lastEvent.eventType == peerOnlineEvent { + onlinePeriods = append( + onlinePeriods, &onlinePeriod{ + start: lastEvent.timestamp, + end: event.timestamp, + }, + ) + + lastEvent = event + } } } // If the last event was an peer offline event, we do not need to // calculate a final online period and can return online periods as is. - if offline { + if lastEvent.eventType == peerOfflineEvent { return onlinePeriods } // The log ended on an online event, so we need to add a final online // event. If the channel is closed, this period is until channel // closure. It it is still open, we calculate it until the present. - endTime := e.closedAt - if endTime.IsZero() { - endTime = e.clock.Now() + finalEvent := &onlinePeriod{ + start: lastEvent.timestamp, + end: e.closedAt, + } + if finalEvent.end.IsZero() { + finalEvent.end = e.clock.Now() } // Add the final online period to the set and return. - return append(onlinePeriods, &onlinePeriod{ - start: lastOnline, - end: endTime, - }) + return append(onlinePeriods, finalEvent) } // uptime calculates the total uptime we have recorded for a channel over the diff --git a/chanfitness/chanevent_test.go b/chanfitness/chanevent_test.go index 4086a40f..637e3627 100644 --- a/chanfitness/chanevent_test.go +++ b/chanfitness/chanevent_test.go @@ -72,10 +72,10 @@ func TestGetOnlinePeriod(t *testing.T) { closedAt time.Time }{ { - name: "No events", + name: "no events", }, { - name: "Start on online period", + name: "start on online period", events: []*channelEvent{ { timestamp: threeHoursAgo, @@ -94,7 +94,7 @@ func TestGetOnlinePeriod(t *testing.T) { }, }, { - name: "Start on offline period", + name: "start on offline period", events: []*channelEvent{ { timestamp: fourHoursAgo, @@ -103,7 +103,7 @@ func TestGetOnlinePeriod(t *testing.T) { }, }, { - name: "End on an online period, channel not closed", + name: "end on an online period, channel not closed", events: []*channelEvent{ { timestamp: fourHoursAgo, @@ -118,7 +118,7 @@ func TestGetOnlinePeriod(t *testing.T) { }, }, { - name: "End on an online period, channel closed", + name: "end on an online period, channel closed", events: []*channelEvent{ { timestamp: fourHoursAgo, @@ -133,12 +133,113 @@ func TestGetOnlinePeriod(t *testing.T) { }, closedAt: oneHourAgo, }, + { + name: "duplicate online events, channel not closed", + events: []*channelEvent{ + { + timestamp: fourHoursAgo, + eventType: peerOnlineEvent, + }, + { + timestamp: threeHoursAgo, + eventType: peerOnlineEvent, + }, + }, + expectedOnline: []*onlinePeriod{ + { + start: fourHoursAgo, + end: testNow, + }, + }, + }, + { + name: "duplicate online events, channel closed", + events: []*channelEvent{ + { + timestamp: fourHoursAgo, + eventType: peerOnlineEvent, + }, + { + timestamp: twoHoursAgo, + eventType: peerOnlineEvent, + }, + }, + expectedOnline: []*onlinePeriod{ + { + start: fourHoursAgo, + end: threeHoursAgo, + }, + }, + closedAt: threeHoursAgo, + }, + { + name: "duplicate offline events, channel not closed", + events: []*channelEvent{ + { + timestamp: fourHoursAgo, + eventType: peerOfflineEvent, + }, + { + timestamp: threeHoursAgo, + eventType: peerOfflineEvent, + }, + }, + expectedOnline: nil, + }, + { + name: "duplicate online then offline", + events: []*channelEvent{ + { + timestamp: fourHoursAgo, + eventType: peerOnlineEvent, + }, + { + timestamp: threeHoursAgo, + eventType: peerOnlineEvent, + }, + { + timestamp: twoHoursAgo, + eventType: peerOfflineEvent, + }, + }, + expectedOnline: []*onlinePeriod{ + { + start: fourHoursAgo, + end: twoHoursAgo, + }, + }, + }, + { + name: "duplicate offline then online", + events: []*channelEvent{ + { + timestamp: fourHoursAgo, + eventType: peerOfflineEvent, + }, + { + timestamp: threeHoursAgo, + eventType: peerOfflineEvent, + }, + { + timestamp: twoHoursAgo, + eventType: peerOnlineEvent, + }, + }, + expectedOnline: []*onlinePeriod{ + { + start: twoHoursAgo, + end: testNow, + }, + }, + }, } for _, test := range tests { test := test t.Run(test.name, func(t *testing.T) { + t.Parallel() + score := &chanEventLog{ events: test.events, clock: clock.NewTestClock(testNow),