From 68565d9617ddf73c85acf67f661d93f51c7d9d6f Mon Sep 17 00:00:00 2001 From: Leonard Lyubich Date: Mon, 8 Nov 2021 16:21:48 +0300 Subject: [PATCH] [#938] ir/netmap: Call AddPeer method if existing candidate was updated In previous implementation IR handler of `AddPeer` notification didn't send registration to contract if existing peer changed has changed its information. as a consequence, the network map members could not update the information without going into offline. Change `processAddPeer` handler to check if * candidate in the network map is a brand new * or information about the network map member was changed and call `AddPeer` method if so. Signed-off-by: Leonard Lyubich --- .../processors/netmap/cleanup_table.go | 48 ++++++++++++++----- .../processors/netmap/cleanup_table_test.go | 24 ++++++---- .../processors/netmap/process_peers.go | 28 +++++------ 3 files changed, 66 insertions(+), 34 deletions(-) diff --git a/pkg/innerring/processors/netmap/cleanup_table.go b/pkg/innerring/processors/netmap/cleanup_table.go index e091b8cbd..a7b88e793 100644 --- a/pkg/innerring/processors/netmap/cleanup_table.go +++ b/pkg/innerring/processors/netmap/cleanup_table.go @@ -1,7 +1,9 @@ package netmap import ( + "bytes" "encoding/hex" + "fmt" "sync" "github.com/nspcc-dev/neofs-api-go/pkg/netmap" @@ -12,13 +14,19 @@ type ( *sync.RWMutex enabled bool threshold uint64 - lastAccess map[string]epochStamp + lastAccess map[string]epochStampWithNodeInfo } epochStamp struct { epoch uint64 removeFlag bool } + + epochStampWithNodeInfo struct { + epochStamp + + binNodeInfo []byte + } ) func newCleanupTable(enabled bool, threshold uint64) cleanupTable { @@ -26,7 +34,7 @@ func newCleanupTable(enabled bool, threshold uint64) cleanupTable { RWMutex: new(sync.RWMutex), enabled: enabled, threshold: threshold, - lastAccess: make(map[string]epochStamp), + lastAccess: make(map[string]epochStampWithNodeInfo), } } @@ -36,33 +44,51 @@ func (c *cleanupTable) update(snapshot *netmap.Netmap, now uint64) { defer c.Unlock() // replacing map is less memory efficient but faster - newMap := make(map[string]epochStamp, len(snapshot.Nodes)) + newMap := make(map[string]epochStampWithNodeInfo, len(snapshot.Nodes)) for i := range snapshot.Nodes { - keyString := hex.EncodeToString(snapshot.Nodes[i].PublicKey()) - if access, ok := c.lastAccess[keyString]; ok { - access.removeFlag = false // reset remove Flag on each Update - newMap[keyString] = access - } else { - newMap[keyString] = epochStamp{epoch: now} + binNodeInfo, err := snapshot.Nodes[i].Marshal() + if err != nil { + panic(fmt.Errorf("could not marshal node info: %w", err)) // seems better than ignore } + + keyString := hex.EncodeToString(snapshot.Nodes[i].PublicKey()) + + access, ok := c.lastAccess[keyString] + if ok { + access.removeFlag = false // reset remove Flag on each Update + } else { + access.epoch = now + } + + access.binNodeInfo = binNodeInfo + + newMap[keyString] = access } c.lastAccess = newMap } -func (c *cleanupTable) touch(keyString string, now uint64) bool { +// updates last access time of the netmap node by string public key. +// +// Returns true if at least one condition is met: +// * node hasn't been accessed yet; +// * remove flag is set; +// * binary node info has changed. +func (c *cleanupTable) touch(keyString string, now uint64, binNodeInfo []byte) bool { c.Lock() defer c.Unlock() access, ok := c.lastAccess[keyString] - result := !access.removeFlag && ok + result := !ok || access.removeFlag || !bytes.Equal(access.binNodeInfo, binNodeInfo) access.removeFlag = false // reset remove flag on each touch if now > access.epoch { access.epoch = now } + access.binNodeInfo = binNodeInfo // update binary node info + c.lastAccess[keyString] = access return result diff --git a/pkg/innerring/processors/netmap/cleanup_table_test.go b/pkg/innerring/processors/netmap/cleanup_table_test.go index 6ed91a51e..0ba468e9c 100644 --- a/pkg/innerring/processors/netmap/cleanup_table_test.go +++ b/pkg/innerring/processors/netmap/cleanup_table_test.go @@ -25,10 +25,13 @@ func TestCleanupTable(t *testing.T) { networkMap, err := netmap.NewNetmap(netmap.NodesFromInfo(infos)) require.NoError(t, err) - mapInfos := map[string]struct{}{ - hex.EncodeToString(infos[0].PublicKey()): {}, - hex.EncodeToString(infos[1].PublicKey()): {}, - hex.EncodeToString(infos[2].PublicKey()): {}, + mapInfos := make(map[string][]byte) + + for i := range infos { + binNodeInfo, err := infos[i].Marshal() + require.NoError(t, err) + + mapInfos[hex.EncodeToString(infos[i].PublicKey())] = binNodeInfo } t.Run("update", func(t *testing.T) { @@ -59,10 +62,15 @@ func TestCleanupTable(t *testing.T) { c.update(networkMap, 1) key := hex.EncodeToString(infos[1].PublicKey()) - require.True(t, c.touch(key, 11)) + require.False(t, c.touch(key, 11, mapInfos[key])) require.EqualValues(t, 11, c.lastAccess[key].epoch) - require.False(t, c.touch(key+"x", 12)) + updNodeInfo := []byte("changed node info") + + require.True(t, c.touch(key, 11, updNodeInfo)) + require.EqualValues(t, 11, c.lastAccess[key].epoch) + + require.True(t, c.touch(key+"x", 12, updNodeInfo)) require.EqualValues(t, 12, c.lastAccess[key+"x"].epoch) }) @@ -74,7 +82,7 @@ func TestCleanupTable(t *testing.T) { c.flag(key) require.True(t, c.lastAccess[key].removeFlag) - require.False(t, c.touch(key, 2)) + require.True(t, c.touch(key, 2, mapInfos[key])) require.False(t, c.lastAccess[key].removeFlag) }) @@ -108,7 +116,7 @@ func TestCleanupTable(t *testing.T) { cnt := 0 key := hex.EncodeToString(infos[1].PublicKey()) - require.False(t, c.touch(key, 4)) // one node was updated + require.True(t, c.touch(key, 4, mapInfos[key])) // one node was updated require.NoError(t, c.forEachRemoveCandidate(4, func(s string) error { diff --git a/pkg/innerring/processors/netmap/process_peers.go b/pkg/innerring/processors/netmap/process_peers.go index 142cc5701..5b7a48ae5 100644 --- a/pkg/innerring/processors/netmap/process_peers.go +++ b/pkg/innerring/processors/netmap/process_peers.go @@ -50,27 +50,25 @@ func (np *Processor) processAddPeer(ev netmapEvent.AddPeer) { }) nodeInfo.SetAttributes(a...) + // marshal updated node info structure + nodeInfoBinary, err := nodeInfo.Marshal() + if err != nil { + np.log.Warn("could not marshal updated network map candidate", + zap.String("error", err.Error()), + ) + + return + } + keyString := hex.EncodeToString(nodeInfo.PublicKey()) - exists := np.netmapSnapshot.touch(keyString, np.epochState.EpochCounter()) - if !exists { + updated := np.netmapSnapshot.touch(keyString, np.epochState.EpochCounter(), nodeInfoBinary) + + if updated { np.log.Info("approving network map candidate", zap.String("key", keyString)) if nr := ev.NotaryRequest(); nr != nil { - // notary event case - - var nodeInfoBinary []byte - - nodeInfoBinary, err = nodeInfo.Marshal() - if err != nil { - np.log.Warn("could not marshal updated network map candidate", - zap.String("error", err.Error()), - ) - - return - } - // create new notary request with the original nonce err = np.netmapClient.Morph().NotaryInvoke( np.netmapClient.ContractAddress(),