Copilot commented on code in PR #1065:
URL:
https://github.com/apache/skywalking-banyandb/pull/1065#discussion_r3070515968
##########
banyand/liaison/grpc/node.go:
##########
@@ -39,7 +39,10 @@ var (
// NodeRegistry is for locating data node with group/name of the metadata
// together with the shardID calculated from the incoming data.
type NodeRegistry interface {
+ // Locate returns the data node assigned to the given (group, name,
shardID, replicaID).
Locate(group, name string, shardID, replicaID uint32) (string, error)
+ // LocateAll returns all distinct data nodes owning the given shard
across all replicas.
+ LocateAll(group string, shardID uint32, replicas int) ([]string, error)
fmt.Stringer
Review Comment:
LocateAll accepts an int "replicas" and passes it directly as the map
capacity and loop bound. If a caller accidentally passes 0 or a negative value
(easy to do given the replicas vs copies ambiguity), this can return an empty
list or even panic (negative hint). Consider validating replicas/copies >= 1
and returning a clear error.
##########
banyand/liaison/grpc/node.go:
##########
@@ -74,6 +77,22 @@ func (n *clusterNodeService) Locate(group, name string,
shardID, replicaID uint3
return nodeID, nil
}
+func (n *clusterNodeService) LocateAll(group string, shardID uint32, replicas
int) ([]string, error) {
+ nodeSet := make(map[string]struct{}, replicas)
+ for replica := 0; replica < replicas; replica++ {
+ nodeID, err := n.Locate(group, "", shardID, uint32(replica))
+ if err != nil {
+ return nil, errors.Wrapf(err, "fail to locate
%s/%d/%d", group, shardID, replica)
+ }
+ nodeSet[nodeID] = struct{}{}
+ }
+ nodes := make([]string, 0, len(nodeSet))
+ for nodeID := range nodeSet {
+ nodes = append(nodes, nodeID)
+ }
+ return nodes, nil
Review Comment:
LocateAll builds the result slice by ranging over a map, which produces a
non-deterministic node order. Since callers use this to keep resolveAssignments
and GetNodes consistent, returning a stable ordering (e.g., sort the nodes
slice) will avoid flaky behavior and order-dependent logic/regressions.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
