diff --git a/client/alloc_runner.go b/client/alloc_runner.go index 54ed06f98cb2..1a396ac058ea 100644 --- a/client/alloc_runner.go +++ b/client/alloc_runner.go @@ -921,7 +921,6 @@ func (r *AllocRunner) handleDestroy() { // Final state sync. We do this to ensure that the server has the correct // state as we wait for a destroy. alloc := r.Alloc() - r.updater(alloc) // Broadcast and persist state synchronously r.sendBroadcast(alloc) @@ -936,6 +935,11 @@ func (r *AllocRunner) handleDestroy() { r.logger.Printf("[ERR] client: alloc %q unable unmount task directories: %v", r.allocID, err) } + // Update the server with the alloc's status -- also marks the alloc as + // being eligible for GC, so from this point on the alloc can be gc'd + // at any time. + r.updater(alloc) + for { select { case <-r.ctx.Done(): diff --git a/client/gc.go b/client/gc.go index 79ff7cb1a122..7946d36d4db8 100644 --- a/client/gc.go +++ b/client/gc.go @@ -51,7 +51,7 @@ type AllocGarbageCollector struct { func NewAllocGarbageCollector(logger *log.Logger, statsCollector stats.NodeStatsCollector, ac AllocCounter, config *GCConfig) *AllocGarbageCollector { // Require at least 1 to make progress if config.ParallelDestroys <= 0 { - logger.Printf("[WARN] client: garbage collector defaulting parallism to 1 due to invalid input value of %d", config.ParallelDestroys) + logger.Printf("[WARN] client.gc: garbage collector defaulting parallism to 1 due to invalid input value of %d", config.ParallelDestroys) config.ParallelDestroys = 1 } @@ -75,7 +75,7 @@ func (a *AllocGarbageCollector) Run() { select { case <-ticker.C: if err := a.keepUsageBelowThreshold(); err != nil { - a.logger.Printf("[ERR] client: error garbage collecting allocation: %v", err) + a.logger.Printf("[ERR] client.gc: error garbage collecting allocation: %v", err) } case <-a.shutdownCh: ticker.Stop() @@ -124,7 +124,7 @@ func (a *AllocGarbageCollector) keepUsageBelowThreshold() error { // Collect an allocation gcAlloc := a.allocRunners.Pop() if gcAlloc == nil { - a.logger.Printf("[WARN] client: garbage collection due to %s skipped because no terminal allocations", reason) + a.logger.Printf("[WARN] client.gc: garbage collection due to %s skipped because no terminal allocations", reason) break } @@ -142,7 +142,7 @@ func (a *AllocGarbageCollector) destroyAllocRunner(ar *AllocRunner, reason strin if alloc := ar.Alloc(); alloc != nil { id = alloc.ID } - a.logger.Printf("[INFO] client: garbage collecting allocation %s due to %s", id, reason) + a.logger.Printf("[INFO] client.gc: garbage collecting allocation %s due to %s", id, reason) // Acquire the destroy lock select { @@ -158,7 +158,7 @@ func (a *AllocGarbageCollector) destroyAllocRunner(ar *AllocRunner, reason strin case <-a.shutdownCh: } - a.logger.Printf("[DEBUG] client: garbage collected %q", ar.Alloc().ID) + a.logger.Printf("[DEBUG] client.gc: garbage collected %q", ar.Alloc().ID) // Release the lock <-a.destroyCh @@ -199,6 +199,11 @@ func (a *AllocGarbageCollector) CollectAll() { // MakeRoomFor garbage collects enough number of allocations in the terminal // state to make room for new allocations func (a *AllocGarbageCollector) MakeRoomFor(allocations []*structs.Allocation) error { + if len(allocations) == 0 { + // Nothing to make room for! + return nil + } + // GC allocs until below the max limit + the new allocations max := a.config.MaxAllocs - len(allocations) for a.numAllocs() > max { diff --git a/client/gc_test.go b/client/gc_test.go index b09ca3fe5ba3..0db3c8196f75 100644 --- a/client/gc_test.go +++ b/client/gc_test.go @@ -310,8 +310,11 @@ func TestAllocGarbageCollector_MakeRoomForAllocations_MaxAllocs(t *testing.T) { defer client.Shutdown() waitTilNodeReady(client, t) + callN := 0 assertAllocs := func(expectedAll, expectedDestroyed int) { // Wait for allocs to be started + callN++ + client.logger.Printf("[TEST] %d -- Waiting for %d total allocs, %d GC'd", callN, expectedAll, expectedDestroyed) testutil.WaitForResult(func() (bool, error) { all, destroyed := 0, 0 for _, ar := range client.getAllocRunners() { @@ -319,14 +322,19 @@ func TestAllocGarbageCollector_MakeRoomForAllocations_MaxAllocs(t *testing.T) { if ar.IsDestroyed() { destroyed++ } + + // assert is waiting + // 2017/10/26 21:38:01.649166 } return all == expectedAll && destroyed == expectedDestroyed, fmt.Errorf( "expected %d allocs (found %d); expected %d destroy (found %d)", expectedAll, all, expectedDestroyed, destroyed, ) }, func(err error) { - t.Fatalf("alloc state: %v", err) + client.logger.Printf("[TEST] %d -- FAILED to find %d total allocs, %d GC'd!", callN, expectedAll, expectedDestroyed) + t.Fatalf("%d alloc state: %v", callN, err) }) + client.logger.Printf("[TEST] %d -- Found %d total allocs, %d GC'd!", callN, expectedAll, expectedDestroyed) } // Create a job