From a3f1b5b1b8ec4080164d22746376e395145a097b Mon Sep 17 00:00:00 2001 From: loqimean Date: Tue, 7 Apr 2026 14:06:15 +0200 Subject: [PATCH] fix(cache): remove runtime.GC() workaround resolved in Go 1.12 The manual runtime.GC() call after atlas.SeedMapTile was a workaround for golang/go#14045 (large arrays not being garbage collected). That issue was fixed in Go 1.12; the module now requires Go 1.26.1, so the hack and its TODO comment are no longer needed. Also adds TestSeedWorkerSucceeds to cover the happy path of seedWorker. --- cmd/tegola/cmd/cache/worker.go | 5 -- cmd/tegola/cmd/cache/worker_test.go | 130 ++++++++++++++++++++++++++++ 2 files changed, 130 insertions(+), 5 deletions(-) create mode 100644 cmd/tegola/cmd/cache/worker_test.go diff --git a/cmd/tegola/cmd/cache/worker.go b/cmd/tegola/cmd/cache/worker.go index c34b0442d..c0d040077 100644 --- a/cmd/tegola/cmd/cache/worker.go +++ b/cmd/tegola/cmd/cache/worker.go @@ -4,7 +4,6 @@ import ( "context" "errors" "fmt" - "runtime" "time" "github.com/go-spatial/geom/slippy" @@ -86,10 +85,6 @@ func seedWorker(overwrite bool, logThresholdMs int64) func(ctx context.Context, } } - // TODO: this is a hack to get around large arrays not being garbage collected - // https://github.com/golang/go/issues/14045 - should be addressed in Go 1.11 - runtime.GC() - durationMs := time.Now().Sub(t).Nanoseconds() / 1000000 if durationMs >= logThresholdMs { log.Infof("seeding map (%v) tile (%v/%v/%v) took: %dms", mt.MapName, z, x, y, durationMs) diff --git a/cmd/tegola/cmd/cache/worker_test.go b/cmd/tegola/cmd/cache/worker_test.go new file mode 100644 index 000000000..fdeb6e690 --- /dev/null +++ b/cmd/tegola/cmd/cache/worker_test.go @@ -0,0 +1,130 @@ +package cache + +import ( + "bytes" + "context" + "testing" + + "github.com/go-spatial/geom" + "github.com/go-spatial/geom/slippy" + "github.com/go-spatial/tegola/atlas" + "github.com/go-spatial/tegola/cache" + "github.com/go-spatial/tegola/cache/memory" + "github.com/go-spatial/tegola/internal/env" + "github.com/go-spatial/tegola/provider/test" +) + +const testMapName = "test-map" + +func TestMain(m *testing.M) { + testMap := atlas.NewWebMercatorMap(testMapName) + testMap.Layers = append(testMap.Layers, atlas.Layer{ + Name: "test-layer", + ProviderLayerName: "test-layer", + MinZoom: 0, + MaxZoom: 20, + Provider: &test.TileProvider{}, + GeomType: geom.Point{}, + DefaultTags: env.Dict{}, + }) + atlas.AddMap(testMap) + + cacher, err := memory.New(nil) + if err != nil { + panic(err) + } + atlas.SetCache(cacher) + + m.Run() +} + +// TestSeedWorkerSucceeds verifies the happy path of seedWorker — the exact +// code path where runtime.GC() used to be called — completes without error. +func TestSeedWorkerSucceeds(t *testing.T) { + mt := MapTile{ + MapName: testMapName, + Tile: slippy.Tile{Z: 1, X: 1, Y: 1}, + } + + worker := seedWorker(true, 0) + if err := worker(context.Background(), mt); err != nil { + t.Fatalf("seedWorker returned unexpected error: %v", err) + } +} + +func TestSeedWorkerNoOverwriteSkipsExistingTile(t *testing.T) { + mt := MapTile{ + MapName: testMapName, + Tile: slippy.Tile{Z: 1, X: 2, Y: 3}, + } + z, x, y := mt.Tile.ZXY() + key := cache.Key{ + MapName: mt.MapName, + Z: uint(z), + X: x, + Y: y, + } + + expected := []byte("already-cached") + c := atlas.GetCache() + if c == nil { + t.Fatal("expected cache to be configured") + } + if err := c.Set(context.Background(), &key, expected); err != nil { + t.Fatalf("failed to prefill cache: %v", err) + } + + worker := seedWorker(false, 0) + if err := worker(context.Background(), mt); err != nil { + t.Fatalf("seedWorker returned unexpected error: %v", err) + } + + got, hit, err := c.Get(context.Background(), &key) + if err != nil { + t.Fatalf("failed reading cache after worker run: %v", err) + } + if !hit { + t.Fatalf("expected cache hit for key %+v", key) + } + if !bytes.Equal(got, expected) { + t.Fatalf("cache value changed for existing tile: got %q want %q", got, expected) + } +} + +func TestSeedWorkerNoOverwriteSeedsOnCacheMiss(t *testing.T) { + mt := MapTile{ + MapName: testMapName, + Tile: slippy.Tile{Z: 1, X: 3, Y: 4}, + } + z, x, y := mt.Tile.ZXY() + key := cache.Key{ + MapName: mt.MapName, + Z: uint(z), + X: x, + Y: y, + } + + c := atlas.GetCache() + if c == nil { + t.Fatal("expected cache to be configured") + } + if err := c.Purge(context.Background(), &key); err != nil { + t.Fatalf("failed to purge cache key before test: %v", err) + } + + worker := seedWorker(false, 0) + if err := worker(context.Background(), mt); err != nil { + t.Fatalf("seedWorker returned unexpected error: %v", err) + } + + got, hit, err := c.Get(context.Background(), &key) + if err != nil { + t.Fatalf("failed reading cache after worker run: %v", err) + } + if !hit { + t.Fatalf("expected cache miss to be seeded for key %+v", key) + } + if len(got) == 0 { + t.Fatal("expected seeded cache value to be non-empty") + } +}