From 2d6f305474a448ab29c40474f46a4958dc8397bf Mon Sep 17 00:00:00 2001 From: Kathy Spradlin Date: Wed, 19 Nov 2014 16:12:31 -0600 Subject: [PATCH 1/5] Fix minor issues related to attributes throughout the code --- gossip/keys.go | 2 +- proto/config.go | 4 ++-- storage/allocator_test.go | 5 ++--- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/gossip/keys.go b/gossip/keys.go index e2269394cfa2..2d2eb75a55b5 100644 --- a/gossip/keys.go +++ b/gossip/keys.go @@ -36,7 +36,7 @@ const ( // KeyMaxAvailCapacityPrefix is the key prefix for gossiping available // store capacity. The suffix is composed of: - // --. The value is a + // --. The value is a // storage.StoreDescriptor struct. KeyMaxAvailCapacityPrefix = "max-avail-capacity-" diff --git a/proto/config.go b/proto/config.go index 4761ef78dbed..af0549034713 100644 --- a/proto/config.go +++ b/proto/config.go @@ -25,8 +25,8 @@ import ( "strings" ) -// IsSubset returns whether attributes list b is a subset of -// attributes list a. +// IsSubset returns whether attributes list a is a subset of +// attributes list b. func (a Attributes) IsSubset(b Attributes) bool { m := map[string]struct{}{} for _, s := range b.Attrs { diff --git a/storage/allocator_test.go b/storage/allocator_test.go index 987513c2049f..3da472b8253f 100644 --- a/storage/allocator_test.go +++ b/storage/allocator_test.go @@ -51,9 +51,8 @@ var multiDCConfig = proto.ZoneConfig{ func filterStores(a proto.Attributes, stores []*StoreDescriptor) ([]*StoreDescriptor, error) { var filtered []*StoreDescriptor for _, s := range stores { - b := s.Attrs.Attrs - b = append(b, s.Node.Attrs.Attrs...) - if a.IsSubset(proto.Attributes{Attrs: b}) { + sAttrs := s.CombinedAttrs() + if a.IsSubset(*sAttrs) { filtered = append(filtered, s) } } From 07401474c50acf5e7030c5835f2fc5f19b30d4a7 Mon Sep 17 00:00:00 2001 From: Kathy Spradlin Date: Mon, 24 Nov 2014 23:40:28 -0600 Subject: [PATCH 2/5] Capacity gossip is now gossiped per store, not by group. This should be sufficient for the beta stages. When more stores are added, then switch to gossiping capacity through groups, so that only the top N will be gossiped. This will lower the gossip and storage load on all machines. --- gossip/infostore.go | 2 +- gossip/keys.go | 5 ++--- server/node.go | 9 ++++----- 3 files changed, 7 insertions(+), 9 deletions(-) diff --git a/gossip/infostore.go b/gossip/infostore.go index c9b4c9e1f3b8..f9013e9a92f2 100644 --- a/gossip/infostore.go +++ b/gossip/infostore.go @@ -210,7 +210,7 @@ func (is *infoStore) addInfo(i *info) error { if existingInfo, ok := is.Infos[i.Key]; ok { if i.Timestamp < existingInfo.Timestamp || (i.Timestamp == existingInfo.Timestamp && i.Hops >= existingInfo.Hops) { - return util.Errorf("info %+v older than current group info %+v", i, existingInfo) + return util.Errorf("info %+v older than current info %+v", i, existingInfo) } contentsChanged = !reflect.DeepEqual(existingInfo.Val, i.Val) } else { diff --git a/gossip/keys.go b/gossip/keys.go index 2d2eb75a55b5..c2bd9ea62802 100644 --- a/gossip/keys.go +++ b/gossip/keys.go @@ -35,9 +35,8 @@ const ( KeyConfigZone = "zones" // KeyMaxAvailCapacityPrefix is the key prefix for gossiping available - // store capacity. The suffix is composed of: - // --. The value is a - // storage.StoreDescriptor struct. + // store capacity. The suffix is composed of: -. + // The value is a storage.StoreDescriptor struct. KeyMaxAvailCapacityPrefix = "max-avail-capacity-" // KeyNodeCount is the count of gossip nodes in the network. The diff --git a/server/node.go b/server/node.go index 80c31a5153d0..14f2562f97ec 100644 --- a/server/node.go +++ b/server/node.go @@ -368,11 +368,10 @@ func (n *Node) gossipCapacities() { log.Warningf("problem getting store descriptor for store %+v: %v", s.Ident, err) return nil } - gossipPrefix := gossip.KeyMaxAvailCapacityPrefix + storeDesc.CombinedAttrs().SortedString() - keyMaxCapacity := gossipPrefix + strconv.FormatInt(int64(storeDesc.Node.NodeID), 10) + "-" + - strconv.FormatInt(int64(storeDesc.StoreID), 10) - // Register gossip group. - n.gossip.RegisterGroup(gossipPrefix, gossipGroupLimit, gossip.MaxGroup) + // Unique gossip key per store. + keyMaxCapacity := gossip.KeyMaxAvailCapacityPrefix + + strconv.FormatInt(int64(storeDesc.Node.NodeID), 16) + "-" + + strconv.FormatInt(int64(storeDesc.StoreID), 16) // Gossip store descriptor. n.gossip.AddInfo(keyMaxCapacity, *storeDesc, ttlCapacityGossip) return nil From 32fa1a83bf5407009f31217d804484a784b90f56 Mon Sep 17 00:00:00 2001 From: Kathy Spradlin Date: Tue, 25 Nov 2014 00:09:48 -0600 Subject: [PATCH 3/5] Simple, linear implementation of a store finder for allocation decisions Store now tracks gossip keys of store capacity, and scans the capacity gossip for attribute matches when they need to make allocation decisions. A simple, linear implementation that has a straightforward way to implement garbage collection of capacity keys, but faster methods can be employed later. It is likely sufficient as long as it is sufficient for all stores to gossip capacity, but will need to be reevaluated when capacity is gossiped through groups. --- storage/allocator.go | 3 - storage/store.go | 30 +++++---- storage/store_finder.go | 88 +++++++++++++++++++++++++ storage/store_finder_test.go | 121 +++++++++++++++++++++++++++++++++++ 4 files changed, 227 insertions(+), 15 deletions(-) create mode 100644 storage/store_finder.go create mode 100644 storage/store_finder_test.go diff --git a/storage/allocator.go b/storage/allocator.go index 0668ca04b553..bdcf1d09ef78 100644 --- a/storage/allocator.go +++ b/storage/allocator.go @@ -25,9 +25,6 @@ import ( "github.com/cockroachdb/cockroach/util" ) -// StoreFinder finds the disks in a datacenter with the most available capacity. -type StoreFinder func(proto.Attributes) ([]*StoreDescriptor, error) - // allocator makes allocation decisions based on a zone configuration, // existing range metadata and available stores. Configuration // settings and range metadata information is stored directly in the diff --git a/storage/store.go b/storage/store.go index 339d828ed1a8..7870fba84a4d 100644 --- a/storage/store.go +++ b/storage/store.go @@ -164,17 +164,18 @@ func (s StoreDescriptor) Less(b util.Ordered) bool { // A Store maintains a map of ranges by start key. A Store corresponds // to one physical device. type Store struct { - Ident proto.StoreIdent - clock *hlc.Clock - engine engine.Engine // The underlying key-value store - db *client.KV // Cockroach KV DB - allocator *allocator // Makes allocation decisions - gossip *gossip.Gossip // Configs and store capacities - raftIDAlloc *IDAllocator // Raft ID allocator - rangeIDAlloc *IDAllocator // Range ID allocator - configMu sync.Mutex // Limit config update processing - raft raft - closer chan struct{} + Ident proto.StoreIdent + clock *hlc.Clock + engine engine.Engine // The underlying key-value store + db *client.KV // Cockroach KV DB + allocator *allocator // Makes allocation decisions + gossip *gossip.Gossip // Configs and store capacities + raftIDAlloc *IDAllocator // Raft ID allocator + rangeIDAlloc *IDAllocator // Range ID allocator + configMu sync.Mutex // Limit config update processing + finderContext storeFinderContext // Context to find stores for allocation + raft raft + closer chan struct{} mu sync.RWMutex // Protects variables below... ranges map[int64]*Range // Map of ranges by range ID @@ -184,7 +185,7 @@ type Store struct { // NewStore returns a new instance of a store. func NewStore(clock *hlc.Clock, eng engine.Engine, db *client.KV, gossip *gossip.Gossip) *Store { - return &Store{ + s := &Store{ clock: clock, engine: eng, db: db, @@ -195,6 +196,8 @@ func NewStore(clock *hlc.Clock, eng engine.Engine, db *client.KV, gossip *gossip ranges: map[int64]*Range{}, rangesByRaftID: map[int64]*Range{}, } + s.allocator.storeFinder = s.findStores + return s } // Stop calls Range.Stop() on all active ranges. @@ -286,6 +289,9 @@ func (s *Store) Start() error { if s.gossip != nil { s.gossip.RegisterCallback(gossip.KeyConfigAccounting, s.configGossipUpdate) s.gossip.RegisterCallback(gossip.KeyConfigZone, s.configGossipUpdate) + // Callback triggers on capacity gossip from all stores. + capacityRegex := fmt.Sprintf("%s.*", gossip.KeyMaxAvailCapacityPrefix) + s.gossip.RegisterCallback(capacityRegex, s.capacityGossipUpdate) } return nil diff --git a/storage/store_finder.go b/storage/store_finder.go new file mode 100644 index 000000000000..1524dd11013d --- /dev/null +++ b/storage/store_finder.go @@ -0,0 +1,88 @@ +// Copyright 2014 The Cockroach Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or +// implied. See the License for the specific language governing +// permissions and limitations under the License. See the AUTHORS file +// for names of contributors. +// +// Author: Kathy Spradlin (kathyspradlin@gmail.com) + +package storage + +import ( + "fmt" + "sync" + + "github.com/cockroachdb/cockroach/gossip" + "github.com/cockroachdb/cockroach/proto" +) + +// StoreFinder finds the disks in a datacenter that have the requested +// attributes. +type StoreFinder func(proto.Attributes) ([]*StoreDescriptor, error) + +type stringSet map[string]struct{} + +type storeFinderContext struct { + sync.Mutex + capacityKeys stringSet +} + +// capacityGossipUpdate is a gossip callback triggered whenever capacity +// information is gossiped. It just tracks keys used for capacity gossip. +func (s *Store) capacityGossipUpdate(key string, contentsChanged bool) { + s.finderContext.Lock() + defer s.finderContext.Unlock() + + if s.finderContext.capacityKeys == nil { + s.finderContext.capacityKeys = stringSet{} + } + s.finderContext.capacityKeys[key] = struct{}{} +} + +// findStores is the Store's implementation of a StoreFinder. It returns a list +// of stores with attributes that are a superset of the required attributes. It +// never returns an error. +// +// If it cannot retrieve a StoreDescriptor from the Store's gossip, it garbage +// collects the failed key. +func (s *Store) findStores(required proto.Attributes) ([]*StoreDescriptor, error) { + s.finderContext.Lock() + defer s.finderContext.Unlock() + var stores []*StoreDescriptor + for key := range s.finderContext.capacityKeys { + storeDesc, err := storeDescFromGossip(key, s.gossip) + if err != nil { + // We can no longer retrieve this key from the gossip store, + // perhaps it expired. + delete(s.finderContext.capacityKeys, key) + } else if required.IsSubset(storeDesc.Attrs) { + stores = append(stores, storeDesc) + } + } + return stores, nil +} + +// storeDescFromGossip retrieves a StoreDescriptor from the specified capacity +// gossip key. Returns an error if the gossip doesn't exist or is not +// a StoreDescriptor. +func storeDescFromGossip(key string, g *gossip.Gossip) (*StoreDescriptor, error) { + info, err := g.GetInfo(key) + + if err != nil { + return nil, err + } + storeDesc, ok := info.(StoreDescriptor) + if !ok { + return nil, fmt.Errorf("gossiped info is not a StoreDescriptor: %+v", info) + } + return &storeDesc, nil +} diff --git a/storage/store_finder_test.go b/storage/store_finder_test.go new file mode 100644 index 000000000000..8e0b49134bdf --- /dev/null +++ b/storage/store_finder_test.go @@ -0,0 +1,121 @@ +// Copyright 2014 The Cockroach Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or +// implied. See the License for the specific language governing +// permissions and limitations under the License. See the AUTHORS file +// for names of contributors. +// +// Author: Kathy Spradlin (kathyspradlin@gmail.com) + +package storage + +import ( + "reflect" + "sort" + "testing" + "time" + + "github.com/cockroachdb/cockroach/proto" +) + +func TestCapacityGossipUpdate(t *testing.T) { + s, _ := createTestStore(t) + defer s.Stop() + key := "testkey" + + // Order and value of contentsChanged shouldn't matter. + s.capacityGossipUpdate(key, true) + s.capacityGossipUpdate(key, false) + + expectedKeys := stringSet{key: struct{}{}} + s.finderContext.Lock() + actualKeys := s.finderContext.capacityKeys + s.finderContext.Unlock() + + if !reflect.DeepEqual(expectedKeys, actualKeys) { + t.Errorf("expected to fetch %+v, instead %+v", expectedKeys, actualKeys) + } +} + +func TestStoreFinder(t *testing.T) { + s, _ := createTestStore(t) + defer s.Stop() + required := []string{"ssd", "dc"} + // Nothing yet. + if stores, _ := s.findStores(proto.Attributes{Attrs: required}); stores != nil { + t.Errorf("expected no stores, instead %+v", stores) + } + + matchingStore := StoreDescriptor{ + Attrs: proto.Attributes{Attrs: required}, + } + supersetStore := StoreDescriptor{ + Attrs: proto.Attributes{Attrs: append(required, "db")}, + } + unmatchingStore := StoreDescriptor{ + Attrs: proto.Attributes{Attrs: []string{"ssd", "otherdc"}}, + } + emptyStore := StoreDescriptor{Attrs: proto.Attributes{}} + + // Explicitly add keys rather than registering a gossip callback to avoid + // waiting for the goroutine callback to finish. + s.finderContext.capacityKeys = stringSet{ + "k1": struct{}{}, + "k2": struct{}{}, + "k3": struct{}{}, + "k4": struct{}{}, + } + s.gossip.AddInfo("k1", matchingStore, time.Hour) + s.gossip.AddInfo("k2", supersetStore, time.Hour) + s.gossip.AddInfo("k3", unmatchingStore, time.Hour) + s.gossip.AddInfo("k4", emptyStore, time.Hour) + + expected := []string{matchingStore.Attrs.SortedString(), supersetStore.Attrs.SortedString()} + stores, err := s.findStores(proto.Attributes{Attrs: required}) + if err != nil { + t.Errorf("expected no err, got %s", err) + } + var actual []string + for _, store := range stores { + actual = append(actual, store.Attrs.SortedString()) + } + sort.Strings(expected) + sort.Strings(actual) + if !reflect.DeepEqual(expected, actual) { + t.Errorf("expected %+v Attrs, instead %+v", expected, actual) + } +} + +// TestStoreFinderGarbageCollection ensures removal of capacity gossip keys in +// the map, if their gossip does not exist when we try to retrieve them. +func TestStoreFinderGarbageCollection(t *testing.T) { + s, _ := createTestStore(t) + defer s.Stop() + + s.finderContext.capacityKeys = stringSet{ + "key0": struct{}{}, + "key1": struct{}{}, + } + required := []string{} + + // No gossip added for either key, so they should be removed. + stores, err := s.findStores(proto.Attributes{Attrs: required}) + if err != nil { + t.Errorf("unexpected error retrieving stores %s", err) + } else if len(stores) != 0 { + t.Errorf("expected no stores found, instead %+v", stores) + } + + if len(s.finderContext.capacityKeys) != 0 { + t.Errorf("expected keys to be cleared, instead are %+v", + s.finderContext.capacityKeys) + } +} From adb66006d7f2784ef8b6287dd131b9e629c9f990 Mon Sep 17 00:00:00 2001 From: Kathy Spradlin Date: Tue, 25 Nov 2014 15:17:54 -0600 Subject: [PATCH 4/5] Address Spencer's PR comments Changes integer base on capacity gossip keys and adds TODO for possible speedup. --- gossip/keys.go | 2 +- server/node.go | 4 ++-- storage/store_finder.go | 4 ++++ 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/gossip/keys.go b/gossip/keys.go index c2bd9ea62802..c74eac56933e 100644 --- a/gossip/keys.go +++ b/gossip/keys.go @@ -35,7 +35,7 @@ const ( KeyConfigZone = "zones" // KeyMaxAvailCapacityPrefix is the key prefix for gossiping available - // store capacity. The suffix is composed of: -. + // store capacity. The suffix is composed of: -. // The value is a storage.StoreDescriptor struct. KeyMaxAvailCapacityPrefix = "max-avail-capacity-" diff --git a/server/node.go b/server/node.go index 14f2562f97ec..bbad85a0413f 100644 --- a/server/node.go +++ b/server/node.go @@ -370,8 +370,8 @@ func (n *Node) gossipCapacities() { } // Unique gossip key per store. keyMaxCapacity := gossip.KeyMaxAvailCapacityPrefix + - strconv.FormatInt(int64(storeDesc.Node.NodeID), 16) + "-" + - strconv.FormatInt(int64(storeDesc.StoreID), 16) + strconv.FormatInt(int64(storeDesc.Node.NodeID), 10) + "-" + + strconv.FormatInt(int64(storeDesc.StoreID), 10) // Gossip store descriptor. n.gossip.AddInfo(keyMaxCapacity, *storeDesc, ttlCapacityGossip) return nil diff --git a/storage/store_finder.go b/storage/store_finder.go index 1524dd11013d..b7254243b4fa 100644 --- a/storage/store_finder.go +++ b/storage/store_finder.go @@ -54,6 +54,10 @@ func (s *Store) capacityGossipUpdate(key string, contentsChanged bool) { // // If it cannot retrieve a StoreDescriptor from the Store's gossip, it garbage // collects the failed key. +// +// TODO(embark, spencer): consider using a reverse index map from Attr->stores, +// for efficiency. Ensure that entries in this map still have an opportunity +// to be garbage collected. func (s *Store) findStores(required proto.Attributes) ([]*StoreDescriptor, error) { s.finderContext.Lock() defer s.finderContext.Unlock() From e142aa5c801f6b22474f7c87c2c689561dd6d8d1 Mon Sep 17 00:00:00 2001 From: Kathy Spradlin Date: Wed, 26 Nov 2014 12:35:45 -0600 Subject: [PATCH 5/5] Embeds a StoreFinder into Store In response to Spencer's PR comments, rather than increasing the length of storage/store by defining store finder methods on Store itself. --- storage/allocator.go | 2 +- storage/store.go | 27 ++++++++++++++------------ storage/store_finder.go | 37 +++++++++++++++++++----------------- storage/store_finder_test.go | 21 ++++++++++---------- 4 files changed, 46 insertions(+), 41 deletions(-) diff --git a/storage/allocator.go b/storage/allocator.go index bdcf1d09ef78..74ac52d63a73 100644 --- a/storage/allocator.go +++ b/storage/allocator.go @@ -31,7 +31,7 @@ import ( // engine-backed range they describe. Information on suitability and // availability of servers is gleaned from the gossip network. type allocator struct { - storeFinder StoreFinder + storeFinder FindStoreFunc rand rand.Rand } diff --git a/storage/store.go b/storage/store.go index 7870fba84a4d..cbaa4e6aed00 100644 --- a/storage/store.go +++ b/storage/store.go @@ -164,18 +164,19 @@ func (s StoreDescriptor) Less(b util.Ordered) bool { // A Store maintains a map of ranges by start key. A Store corresponds // to one physical device. type Store struct { - Ident proto.StoreIdent - clock *hlc.Clock - engine engine.Engine // The underlying key-value store - db *client.KV // Cockroach KV DB - allocator *allocator // Makes allocation decisions - gossip *gossip.Gossip // Configs and store capacities - raftIDAlloc *IDAllocator // Raft ID allocator - rangeIDAlloc *IDAllocator // Range ID allocator - configMu sync.Mutex // Limit config update processing - finderContext storeFinderContext // Context to find stores for allocation - raft raft - closer chan struct{} + *StoreFinder + + Ident proto.StoreIdent + clock *hlc.Clock + engine engine.Engine // The underlying key-value store + db *client.KV // Cockroach KV DB + allocator *allocator // Makes allocation decisions + gossip *gossip.Gossip // Configs and store capacities + raftIDAlloc *IDAllocator // Raft ID allocator + rangeIDAlloc *IDAllocator // Range ID allocator + configMu sync.Mutex // Limit config update processing + raft raft + closer chan struct{} mu sync.RWMutex // Protects variables below... ranges map[int64]*Range // Map of ranges by range ID @@ -186,6 +187,8 @@ type Store struct { // NewStore returns a new instance of a store. func NewStore(clock *hlc.Clock, eng engine.Engine, db *client.KV, gossip *gossip.Gossip) *Store { s := &Store{ + StoreFinder: &StoreFinder{gossip: gossip}, + clock: clock, engine: eng, db: db, diff --git a/storage/store_finder.go b/storage/store_finder.go index b7254243b4fa..1f6ce804052b 100644 --- a/storage/store_finder.go +++ b/storage/store_finder.go @@ -25,27 +25,30 @@ import ( "github.com/cockroachdb/cockroach/proto" ) -// StoreFinder finds the disks in a datacenter that have the requested +// FindStoreFunc finds the disks in a datacenter that have the requested // attributes. -type StoreFinder func(proto.Attributes) ([]*StoreDescriptor, error) +type FindStoreFunc func(proto.Attributes) ([]*StoreDescriptor, error) type stringSet map[string]struct{} -type storeFinderContext struct { - sync.Mutex - capacityKeys stringSet +// StoreFinder provides the data necessary to find stores with particular +// attributes. +type StoreFinder struct { + finderMu sync.Mutex + capacityKeys stringSet // Tracks gosisp keys used for capacity + gossip *gossip.Gossip } // capacityGossipUpdate is a gossip callback triggered whenever capacity // information is gossiped. It just tracks keys used for capacity gossip. -func (s *Store) capacityGossipUpdate(key string, contentsChanged bool) { - s.finderContext.Lock() - defer s.finderContext.Unlock() +func (sf *StoreFinder) capacityGossipUpdate(key string, contentsChanged bool) { + sf.finderMu.Lock() + defer sf.finderMu.Unlock() - if s.finderContext.capacityKeys == nil { - s.finderContext.capacityKeys = stringSet{} + if sf.capacityKeys == nil { + sf.capacityKeys = stringSet{} } - s.finderContext.capacityKeys[key] = struct{}{} + sf.capacityKeys[key] = struct{}{} } // findStores is the Store's implementation of a StoreFinder. It returns a list @@ -58,16 +61,16 @@ func (s *Store) capacityGossipUpdate(key string, contentsChanged bool) { // TODO(embark, spencer): consider using a reverse index map from Attr->stores, // for efficiency. Ensure that entries in this map still have an opportunity // to be garbage collected. -func (s *Store) findStores(required proto.Attributes) ([]*StoreDescriptor, error) { - s.finderContext.Lock() - defer s.finderContext.Unlock() +func (sf *StoreFinder) findStores(required proto.Attributes) ([]*StoreDescriptor, error) { + sf.finderMu.Lock() + defer sf.finderMu.Unlock() var stores []*StoreDescriptor - for key := range s.finderContext.capacityKeys { - storeDesc, err := storeDescFromGossip(key, s.gossip) + for key := range sf.capacityKeys { + storeDesc, err := storeDescFromGossip(key, sf.gossip) if err != nil { // We can no longer retrieve this key from the gossip store, // perhaps it expired. - delete(s.finderContext.capacityKeys, key) + delete(sf.capacityKeys, key) } else if required.IsSubset(storeDesc.Attrs) { stores = append(stores, storeDesc) } diff --git a/storage/store_finder_test.go b/storage/store_finder_test.go index 8e0b49134bdf..6a0852cf7622 100644 --- a/storage/store_finder_test.go +++ b/storage/store_finder_test.go @@ -27,18 +27,17 @@ import ( ) func TestCapacityGossipUpdate(t *testing.T) { - s, _ := createTestStore(t) - defer s.Stop() + sf := StoreFinder{} key := "testkey" // Order and value of contentsChanged shouldn't matter. - s.capacityGossipUpdate(key, true) - s.capacityGossipUpdate(key, false) + sf.capacityGossipUpdate(key, true) + sf.capacityGossipUpdate(key, false) expectedKeys := stringSet{key: struct{}{}} - s.finderContext.Lock() - actualKeys := s.finderContext.capacityKeys - s.finderContext.Unlock() + sf.finderMu.Lock() + actualKeys := sf.capacityKeys + sf.finderMu.Unlock() if !reflect.DeepEqual(expectedKeys, actualKeys) { t.Errorf("expected to fetch %+v, instead %+v", expectedKeys, actualKeys) @@ -67,7 +66,7 @@ func TestStoreFinder(t *testing.T) { // Explicitly add keys rather than registering a gossip callback to avoid // waiting for the goroutine callback to finish. - s.finderContext.capacityKeys = stringSet{ + s.capacityKeys = stringSet{ "k1": struct{}{}, "k2": struct{}{}, "k3": struct{}{}, @@ -100,7 +99,7 @@ func TestStoreFinderGarbageCollection(t *testing.T) { s, _ := createTestStore(t) defer s.Stop() - s.finderContext.capacityKeys = stringSet{ + s.capacityKeys = stringSet{ "key0": struct{}{}, "key1": struct{}{}, } @@ -114,8 +113,8 @@ func TestStoreFinderGarbageCollection(t *testing.T) { t.Errorf("expected no stores found, instead %+v", stores) } - if len(s.finderContext.capacityKeys) != 0 { + if len(s.capacityKeys) != 0 { t.Errorf("expected keys to be cleared, instead are %+v", - s.finderContext.capacityKeys) + s.capacityKeys) } }