From 7ebe4a6972f39be4e69fc18361943861d8c231fe Mon Sep 17 00:00:00 2001 From: Diptanu Choudhury Date: Mon, 19 Dec 2016 17:53:11 -0800 Subject: [PATCH] Added comments --- client/client.go | 10 ++-------- client/gc.go | 8 +++++--- client/gc_test.go | 22 ++++++++++++++++++++++ client/stats/host.go | 7 ++++++- command/agent/alloc_endpoint.go | 6 ++---- 5 files changed, 37 insertions(+), 16 deletions(-) diff --git a/client/client.go b/client/client.go index 2a3aade7dcd..80c3e81a372 100644 --- a/client/client.go +++ b/client/client.go @@ -445,19 +445,13 @@ func (c *Client) Stats() map[string]map[string]string { // CollectAllocation garbage collects a single allocation func (c *Client) CollectAllocation(allocID string) error { - if err := c.garbageCollector.Collect(allocID); err != nil { - return err - } - return nil + return c.garbageCollector.Collect(allocID) } // CollectAllAllocs garbage collects all allocations on a node in the terminal // state func (c *Client) CollectAllAllocs() error { - if err := c.garbageCollector.CollectAll(); err != nil { - return err - } - return nil + return c.garbageCollector.CollectAll() } // Node returns the locally registered node diff --git a/client/gc.go b/client/gc.go index 190e7bbff80..9d9c4b43add 100644 --- a/client/gc.go +++ b/client/gc.go @@ -28,6 +28,8 @@ const ( MB = 1024 * 1024 ) +// GCAlloc wraps an allocation runner and an index enabling it to be used within +// a PQ type GCAlloc struct { timeStamp time.Time allocRunner *AllocRunner @@ -162,7 +164,7 @@ func (a *AllocGarbageCollector) run() { select { case <-ticker.C: if err := a.keepUsageBelowThreshold(); err != nil { - a.logger.Printf("[ERR] client: error GCing allocation: %v", err) + a.logger.Printf("[ERR] client: error garbage collecting allocation: %v", err) } case <-a.shutdownCh: ticker.Stop() @@ -214,7 +216,7 @@ func (a *AllocGarbageCollector) keepUsageBelowThreshold() error { } func (a *AllocGarbageCollector) Stop() { - a.shutdownCh <- struct{}{} + close(a.shutdownCh) } // Collect garbage collects a single allocation on a node @@ -281,7 +283,7 @@ func (a *AllocGarbageCollector) MakeRoomFor(allocations []*structs.Allocation) e } if allocDirStats != nil { - if allocDirStats.Available >= uint64(totalResource.DiskMB*1024*1024) { + if allocDirStats.Available >= uint64(totalResource.DiskMB*MB) { break } } else { diff --git a/client/gc_test.go b/client/gc_test.go index 5e3572023d5..8ae97a3cc6e 100644 --- a/client/gc_test.go +++ b/client/gc_test.go @@ -162,6 +162,7 @@ func TestAllocGarbageCollector_MakeRoomForAllocations_EnoughSpace(t *testing.T) statsCollector.inodePercents = []float64{0} alloc := mock.Alloc() + alloc.Resources.DiskMB = 150 if err := gc.MakeRoomFor([]*structs.Allocation{alloc}); err != nil { t.Fatalf("err: %v", err) } @@ -197,6 +198,7 @@ func TestAllocGarbageCollector_MakeRoomForAllocations_GC_Partial(t *testing.T) { statsCollector.inodePercents = []float64{0, 0, 0} alloc := mock.Alloc() + alloc.Resources.DiskMB = 150 if err := gc.MakeRoomFor([]*structs.Allocation{alloc}); err != nil { t.Fatalf("err: %v", err) } @@ -233,6 +235,7 @@ func TestAllocGarbageCollector_MakeRoomForAllocations_GC_All(t *testing.T) { statsCollector.inodePercents = []float64{0, 0, 0} alloc := mock.Alloc() + alloc.Resources.DiskMB = 150 if err := gc.MakeRoomFor([]*structs.Allocation{alloc}); err != nil { t.Fatalf("err: %v", err) } @@ -260,6 +263,7 @@ func TestAllocGarbageCollector_MakeRoomForAllocations_GC_Fallback(t *testing.T) } alloc := mock.Alloc() + alloc.Resources.DiskMB = 150 if err := gc.MakeRoomFor([]*structs.Allocation{alloc}); err != nil { t.Fatalf("err: %v", err) } @@ -297,6 +301,14 @@ func TestAllocGarbageCollector_UsageBelowThreshold(t *testing.T) { if err := gc.keepUsageBelowThreshold(); err != nil { t.Fatalf("err: %v", err) } + + // We shouldn't GC any of the allocs since the used percent values are below + // threshold + for i := 0; i < 2; i++ { + if gcAlloc := gc.allocRunners.Pop(); gcAlloc == nil { + t.Fatalf("err: %v", gcAlloc) + } + } } func TestAllocGarbageCollector_UsedPercentThreshold(t *testing.T) { @@ -322,4 +334,14 @@ func TestAllocGarbageCollector_UsedPercentThreshold(t *testing.T) { if err := gc.keepUsageBelowThreshold(); err != nil { t.Fatalf("err: %v", err) } + + // We should be GC-ing only one of the alloc runners since the second time + // used percent returns a number below threshold. + if gcAlloc := gc.allocRunners.Pop(); gcAlloc == nil { + t.Fatalf("err: %v", gcAlloc) + } + + if gcAlloc := gc.allocRunners.Pop(); gcAlloc != nil { + t.Fatalf("gcAlloc: %v", gcAlloc) + } } diff --git a/client/stats/host.go b/client/stats/host.go index 1583c30278a..95f67c9449a 100644 --- a/client/stats/host.go +++ b/client/stats/host.go @@ -54,6 +54,8 @@ type DiskStats struct { InodesUsedPercent float64 } +// NodeStatsCollector is an interface which is used for the puproses of mocking +// the HostStatsCollector in the tests type NodeStatsCollector interface { Collect() error Stats() *HostStats @@ -70,7 +72,9 @@ type HostStatsCollector struct { allocDir string } -// NewHostStatsCollector returns a HostStatsCollector +// NewHostStatsCollector returns a HostStatsCollector. The allocDir is passed in +// so that we can present the disk related statistics for the mountpoint where +// the allocation directory lives func NewHostStatsCollector(logger *log.Logger, allocDir string) *HostStatsCollector { numCores := runtime.NumCPU() statsCalculator := make(map[string]*HostCpuStatsCalculator) @@ -138,6 +142,7 @@ func (h *HostStatsCollector) Collect() error { } hs.DiskStats = diskStats + // Getting the disk stats for the allocation directory usage, err := disk.Usage(h.allocDir) if err != nil { return err diff --git a/command/agent/alloc_endpoint.go b/command/agent/alloc_endpoint.go index 40a62189be4..3afdfebadb1 100644 --- a/command/agent/alloc_endpoint.go +++ b/command/agent/alloc_endpoint.go @@ -90,13 +90,11 @@ func (s *HTTPServer) ClientGCRequest(resp http.ResponseWriter, req *http.Request if s.agent.client == nil { return nil, clientNotRunning } - err := s.agent.Client().CollectAllAllocs() - return nil, err + return nil, s.agent.Client().CollectAllAllocs() } func (s *HTTPServer) allocGC(allocID string, resp http.ResponseWriter, req *http.Request) (interface{}, error) { - err := s.agent.Client().CollectAllocation(allocID) - return nil, err + return nil, s.agent.Client().CollectAllocation(allocID) } func (s *HTTPServer) allocSnapshot(allocID string, resp http.ResponseWriter, req *http.Request) (interface{}, error) {