From c714b1e895941e0aad36a5753be7eb6dc6bc5c3d Mon Sep 17 00:00:00 2001 From: Manohar HT Date: Fri, 16 Dec 2022 16:08:42 +0530 Subject: [PATCH 1/4] add ListPeers method that returns remote fetchers. --- galaxycache.go | 5 +++++ peers.go | 7 +++++++ 2 files changed, 12 insertions(+) diff --git a/galaxycache.go b/galaxycache.go index 0b0912a9..b51965ba 100644 --- a/galaxycache.go +++ b/galaxycache.go @@ -232,6 +232,11 @@ func (universe *Universe) RemovePeers(ids ...string) error { return universe.peerPicker.remove(ids...) } +// ListPeers returns Universe's remote fetchers map keyed by peer IDs +func (universe *Universe) ListPeers() map[string]RemoteFetcher { + return universe.peerPicker.listPeers() +} + // Shutdown closes all open fetcher connections func (universe *Universe) Shutdown() error { return universe.peerPicker.shutdown() diff --git a/peers.go b/peers.go index 782f0b6e..60c315ae 100644 --- a/peers.go +++ b/peers.go @@ -395,6 +395,13 @@ func (pp *PeerPicker) remove(ids ...string) error { return eg.Wait() } +func (pp *PeerPicker) listPeers() map[string]RemoteFetcher { + pp.mu.Lock() + defer pp.mu.Unlock() + fetchers := pp.fetchers + return fetchers +} + func (pp *PeerPicker) shutdown() error { pp.setIncludeSelf(false) // Clear out all the existing peers From 1fdd87ee2554003f6165fd8046947540369db4d6 Mon Sep 17 00:00:00 2001 From: Manohar HT Date: Fri, 16 Dec 2022 22:08:08 +0530 Subject: [PATCH 2/4] * deep copy of fetchers map * use read lock * add checks inside incremental peers test for listPeers method * fix doc comments for ListPeers --- galaxycache.go | 3 ++- peers.go | 12 +++++++++--- peers_test.go | 27 +++++++++++++++++++++++++++ 3 files changed, 38 insertions(+), 4 deletions(-) diff --git a/galaxycache.go b/galaxycache.go index b51965ba..ca89c913 100644 --- a/galaxycache.go +++ b/galaxycache.go @@ -232,7 +232,8 @@ func (universe *Universe) RemovePeers(ids ...string) error { return universe.peerPicker.remove(ids...) } -// ListPeers returns Universe's remote fetchers map keyed by peer IDs +// ListPeers returns a map of remote fetchers keyed by Peer ID, +// useful for testing incremental changes to galaxycache peers. func (universe *Universe) ListPeers() map[string]RemoteFetcher { return universe.peerPicker.listPeers() } diff --git a/peers.go b/peers.go index 60c315ae..0a127599 100644 --- a/peers.go +++ b/peers.go @@ -396,9 +396,15 @@ func (pp *PeerPicker) remove(ids ...string) error { } func (pp *PeerPicker) listPeers() map[string]RemoteFetcher { - pp.mu.Lock() - defer pp.mu.Unlock() - fetchers := pp.fetchers + pp.mu.RLock() + defer pp.mu.RUnlock() + + // deep copy of pp.fetchers map. + fetchers := make(map[string]RemoteFetcher) + for p, f := range pp.fetchers { + fetchers[p] = f + } + return fetchers } diff --git a/peers_test.go b/peers_test.go index 5c709a30..c3636e70 100644 --- a/peers_test.go +++ b/peers_test.go @@ -244,6 +244,14 @@ func TestPeersIncremental(t *testing.T) { fetcherURIs[f.(*TestFetcher).uri] = struct{}{} } + allFetchers := u.peerPicker.listPeers() + allPeerIDs := make(map[string]struct{}, len(allFetchers)) + allFetcherURIs := make(map[string]struct{}, len(allFetchers)) + for peerID, fetcher := range allFetchers { + allPeerIDs[peerID] = struct{}{} + allFetcherURIs[fetcher.(*TestFetcher).uri] = struct{}{} + } + for _, expPeer := range step.expectedPeers { if _, ok := allPeers[expPeer.ID]; !ok { t.Errorf("missing peer %q from hashring at step %d", expPeer.ID, si) @@ -258,6 +266,17 @@ func TestPeersIncremental(t *testing.T) { expPeer.ID, expPeer.URI, si) } delete(fetcherURIs, expPeer.URI) + // Checks for peer IDs and fetchers from listPeers method. + if _, ok := allPeerIDs[expPeer.ID]; !ok { + t.Errorf("missing peer %q from copy of fetchers map keys at step %d", + expPeer.ID, si) + } + delete(allPeerIDs, expPeer.ID) + if _, ok := allFetcherURIs[expPeer.URI]; !ok { + t.Errorf("missing peer %q (URI %q) from copy of fetchers values at step %d", + expPeer.ID, expPeer.URI, si) + } + delete(allFetcherURIs, expPeer.URI) } if step.includeSelf { if _, ok := allPeers[selfID]; !ok { @@ -274,6 +293,14 @@ func TestPeersIncremental(t *testing.T) { if len(fetcherURIs) > 0 { t.Errorf("unexpected peer(s)' URI(s) in fetcher-map at step %d: %v", si, fetcherURIs) } + // Checks for peer IDs and fetchers from listPeers method. + if len(allPeerIDs) > 0 { + t.Errorf("unexpected peer(s) in copy of fetcher-map at step %d: %v", si, allPeerIDs) + } + if len(allFetcherURIs) > 0 { + t.Errorf("unexpected peer(s)' URI(s) in copy of fetcher-map at step %d: %v", + si, allFetcherURIs) + } } }) } From 39d4a106dc10c8bcecdd5fa55a5993a9e37216aa Mon Sep 17 00:00:00 2001 From: Manohar HT Date: Sat, 17 Dec 2022 00:35:49 +0530 Subject: [PATCH 3/4] - initialize fetchers map with size of length pp.fetchers - use ListPeers in test case. --- peers.go | 4 ++-- peers_test.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/peers.go b/peers.go index 0a127599..79bcd6e5 100644 --- a/peers.go +++ b/peers.go @@ -399,8 +399,8 @@ func (pp *PeerPicker) listPeers() map[string]RemoteFetcher { pp.mu.RLock() defer pp.mu.RUnlock() - // deep copy of pp.fetchers map. - fetchers := make(map[string]RemoteFetcher) + // deep copy of fetchers map with size equal to length of pp.fetchers. + fetchers := make(map[string]RemoteFetcher, len(pp.fetchers)) for p, f := range pp.fetchers { fetchers[p] = f } diff --git a/peers_test.go b/peers_test.go index c3636e70..0b4f75be 100644 --- a/peers_test.go +++ b/peers_test.go @@ -244,7 +244,7 @@ func TestPeersIncremental(t *testing.T) { fetcherURIs[f.(*TestFetcher).uri] = struct{}{} } - allFetchers := u.peerPicker.listPeers() + allFetchers := u.ListPeers() allPeerIDs := make(map[string]struct{}, len(allFetchers)) allFetcherURIs := make(map[string]struct{}, len(allFetchers)) for peerID, fetcher := range allFetchers { From 928f2cd84083a0c14c47d7af8002a9da892d15dc Mon Sep 17 00:00:00 2001 From: Manohar HT Date: Sat, 17 Dec 2022 10:37:50 +0530 Subject: [PATCH 4/4] fix comment for fetchers copy. --- peers.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/peers.go b/peers.go index 79bcd6e5..d61174b1 100644 --- a/peers.go +++ b/peers.go @@ -399,7 +399,7 @@ func (pp *PeerPicker) listPeers() map[string]RemoteFetcher { pp.mu.RLock() defer pp.mu.RUnlock() - // deep copy of fetchers map with size equal to length of pp.fetchers. + // copy of pp.fetchers map. fetchers := make(map[string]RemoteFetcher, len(pp.fetchers)) for p, f := range pp.fetchers { fetchers[p] = f