Skip to content

Commit 06edd19

Browse files
dnephindnephin
committed
Small refactor of gc/scheduler to remove import of metadata
Replace metadata.GCStats with an interface for exposing elapsed time Signed-off-by: Daniel Nephin <dnephin@gmail.com>
1 parent e479165 commit 06edd19

File tree

5 files changed

+52
-29
lines changed

5 files changed

+52
-29
lines changed

‎gc/gc.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ package gc
88
import (
99
"context"
1010
"sync"
11+
"time"
1112
)
1213

1314
// ResourceType represents type of resource at a node
@@ -21,6 +22,11 @@ type Node struct {
2122
Key string
2223
}
2324

25+
// Stats about a garbage collection run
26+
type Stats interface {
27+
Elapsed() time.Duration
28+
}
29+
2430
// Tricolor implements basic, single-thread tri-color GC. Given the roots, the
2531
// complete set and a refs function, this function returns a map of all
2632
// reachable objects.

‎gc/scheduler/scheduler.go‎

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,14 @@ package scheduler
22

33
import (
44
"context"
5-
"errors"
65
"fmt"
76
"sync"
87
"time"
98

9+
"github.com/containerd/containerd/gc"
1010
"github.com/containerd/containerd/log"
11-
"github.com/containerd/containerd/metadata"
1211
"github.com/containerd/containerd/plugin"
12+
"github.com/pkg/errors"
1313
)
1414

1515
// config configures the garbage collection policies.
@@ -95,7 +95,12 @@ func init() {
9595
return nil, err
9696
}
9797

98-
m := newScheduler(md.(*metadata.DB), ic.Config.(*config))
98+
mdCollector, ok := md.(collector)
99+
if !ok {
100+
return nil, errors.Errorf("%s %T must implement collector", plugin.MetadataPlugin, md)
101+
}
102+
103+
m := newScheduler(mdCollector, ic.Config.(*config))
99104

100105
ic.Meta.Exports = map[string]string{
101106
"PauseThreshold": fmt.Sprint(m.pauseThreshold),
@@ -119,7 +124,7 @@ type mutationEvent struct {
119124

120125
type collector interface {
121126
RegisterMutationCallback(func(bool))
122-
GarbageCollect(context.Context) (metadata.GCStats, error)
127+
GarbageCollect(context.Context) (gc.Stats, error)
123128
}
124129

125130
type gcScheduler struct {
@@ -128,7 +133,7 @@ type gcScheduler struct {
128133
eventC chan mutationEvent
129134

130135
waiterL sync.Mutex
131-
waiters []chan metadata.GCStats
136+
waiters []chan gc.Stats
132137

133138
pauseThreshold float64
134139
deletionThreshold int
@@ -171,12 +176,12 @@ func newScheduler(c collector, cfg *config) *gcScheduler {
171176
return s
172177
}
173178

174-
func (s *gcScheduler) ScheduleAndWait(ctx context.Context) (metadata.GCStats, error) {
179+
func (s *gcScheduler) ScheduleAndWait(ctx context.Context) (gc.Stats, error) {
175180
return s.wait(ctx, true)
176181
}
177182

178-
func (s *gcScheduler) wait(ctx context.Context, trigger bool) (metadata.GCStats, error) {
179-
wc := make(chan metadata.GCStats, 1)
183+
func (s *gcScheduler) wait(ctx context.Context, trigger bool) (gc.Stats, error) {
184+
wc := make(chan gc.Stats, 1)
180185
s.waiterL.Lock()
181186
s.waiters = append(s.waiters, wc)
182187
s.waiterL.Unlock()
@@ -190,15 +195,15 @@ func (s *gcScheduler) wait(ctx context.Context, trigger bool) (metadata.GCStats,
190195
}()
191196
}
192197

193-
var gcStats metadata.GCStats
198+
var gcStats gc.Stats
194199
select {
195200
case stats, ok := <-wc:
196201
if !ok {
197-
return metadata.GCStats{}, errors.New("gc failed")
202+
return gcStats, errors.New("gc failed")
198203
}
199204
gcStats = stats
200205
case <-ctx.Done():
201-
return metadata.GCStats{}, ctx.Err()
206+
return gcStats, ctx.Err()
202207
}
203208

204209
return gcStats, nil
@@ -301,9 +306,9 @@ func (s *gcScheduler) run(ctx context.Context) {
301306
continue
302307
}
303308

304-
log.G(ctx).WithField("d", stats.MetaD).Debug("garbage collected")
309+
log.G(ctx).WithField("d", stats.Elapsed()).Debug("garbage collected")
305310

306-
gcTime += stats.MetaD
311+
gcTime += stats.Elapsed()
307312
collections++
308313
triggered = false
309314
deletions = 0

‎gc/scheduler/scheduler_test.go‎

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,11 @@ import (
66
"testing"
77
"time"
88

9-
"github.com/containerd/containerd/metadata"
9+
"github.com/containerd/containerd/gc"
10+
"github.com/stretchr/testify/require"
1011
)
1112

1213
func TestPauseThreshold(t *testing.T) {
13-
1414
cfg := &config{
1515
// With 100μs, gc should run about every 5ms
1616
PauseThreshold: 0.02,
@@ -99,7 +99,7 @@ func TestTrigger(t *testing.T) {
9999
}
100100
ctx, cancel = context.WithCancel(context.Background())
101101
scheduler = newScheduler(tc, cfg)
102-
stats metadata.GCStats
102+
stats gc.Stats
103103
err error
104104
)
105105

@@ -123,9 +123,7 @@ func TestTrigger(t *testing.T) {
123123
t.Fatalf("GC failed: %#v", err)
124124
}
125125

126-
if stats.MetaD != tc.d {
127-
t.Fatalf("unexpected gc duration: %s, expected %d", stats.MetaD, tc.d)
128-
}
126+
require.Equal(t, tc.d, stats.Elapsed())
129127

130128
if c := tc.runCount(); c != 1 {
131129
t.Fatalf("unexpected gc run count %d, expected 1", c)
@@ -180,11 +178,18 @@ func (tc *testCollector) RegisterMutationCallback(f func(bool)) {
180178
tc.callbacks = append(tc.callbacks, f)
181179
}
182180

183-
func (tc *testCollector) GarbageCollect(context.Context) (metadata.GCStats, error) {
181+
func (tc *testCollector) GarbageCollect(context.Context) (gc.Stats, error) {
184182
tc.m.Lock()
185183
tc.gc++
186184
tc.m.Unlock()
187-
return metadata.GCStats{
188-
MetaD: tc.d,
189-
}, nil
185+
return gcStats{elapsed: tc.d}, nil
186+
}
187+
188+
type gcStats struct {
189+
elapsed time.Duration
190+
}
191+
192+
// Elapsed returns the duration which elapsed during a collection
193+
func (s gcStats) Elapsed() time.Duration {
194+
return s.elapsed
190195
}

‎metadata/db.go‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,7 @@ func (m *DB) Update(fn func(*bolt.Tx) error) error {
204204
// RegisterMutationCallback registers a function to be called after a metadata
205205
// mutations has been performed.
206206
//
207-
// The callback function in an argument for whether a deletion has occurred
207+
// The callback function is an argument for whether a deletion has occurred
208208
// since the last garbage collection.
209209
func (m *DB) RegisterMutationCallback(fn func(bool)) {
210210
m.dirtyL.Lock()
@@ -219,15 +219,20 @@ type GCStats struct {
219219
SnapshotD map[string]time.Duration
220220
}
221221

222+
// Elapsed returns the duration which elapsed during a collection
223+
func (s GCStats) Elapsed() time.Duration {
224+
return s.MetaD
225+
}
226+
222227
// GarbageCollect starts garbage collection
223-
func (m *DB) GarbageCollect(ctx context.Context) (stats GCStats, err error) {
228+
func (m *DB) GarbageCollect(ctx context.Context) (gc.Stats, error) {
224229
m.wlock.Lock()
225230
t1 := time.Now()
226231

227232
marked, err := m.getMarked(ctx)
228233
if err != nil {
229234
m.wlock.Unlock()
230-
return GCStats{}, err
235+
return nil, err
231236
}
232237

233238
m.dirtyL.Lock()
@@ -259,9 +264,10 @@ func (m *DB) GarbageCollect(ctx context.Context) (stats GCStats, err error) {
259264
}); err != nil {
260265
m.dirtyL.Unlock()
261266
m.wlock.Unlock()
262-
return GCStats{}, err
267+
return nil, err
263268
}
264269

270+
var stats GCStats
265271
var wg sync.WaitGroup
266272

267273
if len(m.dirtySS) > 0 {
@@ -303,7 +309,7 @@ func (m *DB) GarbageCollect(ctx context.Context) (stats GCStats, err error) {
303309

304310
wg.Wait()
305311

306-
return
312+
return stats, err
307313
}
308314

309315
func (m *DB) getMarked(ctx context.Context) (map[gc.Node]struct{}, error) {

‎services/images/service.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
imagesapi "github.com/containerd/containerd/api/services/images/v1"
99
"github.com/containerd/containerd/errdefs"
1010
"github.com/containerd/containerd/events"
11+
"github.com/containerd/containerd/gc"
1112
"github.com/containerd/containerd/images"
1213
"github.com/containerd/containerd/log"
1314
"github.com/containerd/containerd/metadata"
@@ -43,7 +44,7 @@ func init() {
4344
}
4445

4546
type gcScheduler interface {
46-
ScheduleAndWait(gocontext.Context) (metadata.GCStats, error)
47+
ScheduleAndWait(gocontext.Context) (gc.Stats, error)
4748
}
4849

4950
type service struct {

0 commit comments

Comments
 (0)