Skip to content

Commit

Permalink
Fix log server not exiting properly on SIGINT (#2805)
Browse files Browse the repository at this point in the history
* Fix log server not exiting properly on SIGINT

Make the tree garbage collection coroutine exit immediately when the
context is cancelled.
  • Loading branch information
gregoire-mullvad authored Sep 6, 2022
1 parent 5fa4cf6 commit 25255fa
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 13 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@

## HEAD

### Misc
* Fix log server not exiting properly on SIGINT

## v.1.5.0

### Storage
Expand Down
15 changes: 7 additions & 8 deletions server/admin/tree_gc.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ const (

var (
timeNow = time.Now
timeSleep = time.Sleep
timeAfter = time.After

hardDeleteCounter monitoring.Counter
metricsOnce sync.Once
Expand Down Expand Up @@ -85,12 +85,6 @@ func NewDeletedTreeGC(admin storage.AdminStorage, threshold, minRunInterval time
// Run starts the tree garbage collection process. It runs until ctx is cancelled.
func (gc *DeletedTreeGC) Run(ctx context.Context) {
for {
select {
case <-ctx.Done():
return
default:
}

count, err := gc.RunOnce(ctx)
if err != nil {
klog.Errorf("DeletedTreeGC.Run: %v", err)
Expand All @@ -100,7 +94,12 @@ func (gc *DeletedTreeGC) Run(ctx context.Context) {
}

d := gc.minRunInterval + time.Duration(rand.Int63n(gc.minRunInterval.Nanoseconds()))
timeSleep(d)
select {
case <-ctx.Done():
return
case <-timeAfter(d):
}

}
}

Expand Down
16 changes: 11 additions & 5 deletions server/admin/tree_gc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ func TestDeletedTreeGC_Run(t *testing.T) {
// * Sleep (ctx cancelled)
//
// DeletedTreeGC.Run() until ctx in cancelled. Since it always sleeps between iterations, we
// make "timeSleep" cancel ctx the second time around to break the loop.
// make "timeAfter" cancel ctx the second time around to break the loop.

tree1 := proto.Clone(testonly.LogTree).(*trillian.Tree)
tree1.TreeId = 1
Expand Down Expand Up @@ -74,10 +74,10 @@ func TestDeletedTreeGC_Run(t *testing.T) {
listTX2.EXPECT().Close().Return(nil)
listTX2.EXPECT().Commit().Return(nil)

defer func(now func() time.Time, sleep func(time.Duration)) {
defer func(now func() time.Time, after func(time.Duration) <-chan time.Time) {
timeNow = now
timeSleep = sleep
}(timeNow, timeSleep)
timeAfter = after
}(timeNow, timeAfter)

const deleteThreshold = 1 * time.Hour
const runInterval = 3 * time.Second
Expand All @@ -87,14 +87,20 @@ func TestDeletedTreeGC_Run(t *testing.T) {
timeNow = func() time.Time { return now }

calls := 0
timeSleep = func(d time.Duration) {
timeAfter = func(d time.Duration) <-chan time.Time {
calls++
if d < runInterval || d >= 2*runInterval {
t.Errorf("Called time.Sleep(%v), want %v", d, runInterval)
}
if calls >= 2 {
cancel()
// Block to make sure we're exiting because of the cancelled context and
// not because the timer elapsed.
return make(chan time.Time)
}
ch := make(chan time.Time, 1)
ch <- now
return ch
}

NewDeletedTreeGC(as, deleteThreshold, runInterval, nil /* mf */).Run(ctx)
Expand Down

0 comments on commit 25255fa

Please sign in to comment.