SUPPORT node: Compare node info during initial bootstrap properly #693

Merged
dstepanov-yadro merged 1 commit from fyrchik/frostfs-node:fix-bootstrap into support/v0.37 2023-09-18 07:30:17 +00:00

View file

@ -282,11 +282,62 @@ func initNetmapState(c *cfg) {
c.handleLocalNodeInfo(ni) c.handleLocalNodeInfo(ni)
} }
func sameNodeInfo(a, b *netmapSDK.NodeInfo) bool { func needsUpdate(local, remote *netmapSDK.NodeInfo) bool {
Review

The function should be named "noUpdateRequired"

The function should be named "noUpdateRequired"
// Suboptimal, but we do this once on the node startup. return bytes.Equal(local.PublicKey(), remote.PublicKey()) && equalEndpoints(local, remote) && equalAttributes(local, remote)
rawA := a.Marshal() }
rawB := b.Marshal()
return bytes.Equal(rawA, rawB) func equalAttributes(local, remote *netmapSDK.NodeInfo) bool {
asA := make(map[string]string)
local.IterateAttributes(func(k, v string) {
asA[k] = v
})
allMatched := true
count := 0
remote.IterateAttributes(func(k, vb string) {
// IR adds new attributes derived from the locode, they should be skipped.
if isLocodeAttribute(k) {
return
}
if va, ok := asA[k]; !ok || va != vb {
allMatched = false
return
}
count++
})
return allMatched && count == len(asA)
Review

Should we take into account locode attributes in asA?

Should we take into account `locode` attributes in `asA`?
Review

As I understand it, locode attributes are added from the locode database, so they are not in the local config.

As I understand it, `locode` attributes are added from the locode database, so they are not in the local config.
}
func isLocodeAttribute(k string) bool {
// See https://git.frostfs.info/TrueCloudLab/frostfs-api/src/branch/master/netmap/types.proto#L171
switch k {
case "Continent", "Country", "CountryCode", "Location", "SubDiv", "SubDivCode":
return true
default:
return false
}
}
func equalEndpoints(a, b *netmapSDK.NodeInfo) bool {
var esA, esB []string
a.IterateNetworkEndpoints(func(e string) bool {
esA = append(esA, e)
return false
})
b.IterateNetworkEndpoints(func(e string) bool {
esB = append(esB, e)
return false
})
if len(esA) != len(esB) {
fyrchik marked this conversation as resolved
Review

What about return slices.Equal?

What about `return slices.Equal`?
Review

Is it present in go1.20?

Is it present in go1.20?
Review

I think it is presented since go1.17 as generics (or 18?) appeared. If it is not, let's keep like that

I think it is presented since go1.17 as generics (or 18?) appeared. If it is not, let's keep like that
Review

It was moved to stdlib in 1.21 https://tip.golang.org/doc/go1.21#slices, previously was in /x/exp

It was moved to stdlib in 1.21 https://tip.golang.org/doc/go1.21#slices, previously was in /x/exp
Review

Ah, ok then

Ah, ok then
return false
}
for i := range esA {
if esA[i] != esB[i] {
return false
}
}
return true
} }
func nodeState(ni *netmapSDK.NodeInfo) string { func nodeState(ni *netmapSDK.NodeInfo) string {
@ -314,7 +365,7 @@ func (c *cfg) netmapInitLocalNodeState(epoch uint64) (*netmapSDK.NodeInfo, bool,
for i := range nmNodes { for i := range nmNodes {
if bytes.Equal(nmNodes[i].PublicKey(), c.binPublicKey) { if bytes.Equal(nmNodes[i].PublicKey(), c.binPublicKey) {
candidate = &nmNodes[i] candidate = &nmNodes[i]
alreadyBootstraped = candidate.IsOnline() && sameNodeInfo(&c.cfgNodeInfo.localInfo, candidate) alreadyBootstraped = candidate.IsOnline() && needsUpdate(&c.cfgNodeInfo.localInfo, candidate)
break break
} }
} }