hanahmily commented on code in PR #1111:
URL:
https://github.com/apache/skywalking-banyandb/pull/1111#discussion_r3178086727
##########
banyand/liaison/grpc/server.go:
##########
@@ -253,6 +252,19 @@ func NewServer(_ context.Context, tir1Client, tir2Client,
broadcaster queue.Clie
protector: protectorService,
routeTableProviders: routeProviders,
}
+ // Phase 2 Step 2.2: wire the cluster-wide AwaitRevisionApplied fan-out.
+ // Tier1 (peer liaisons) and tier2 (data nodes) connection pools are
+ // borrowed via the queue.Client interface added in #1109; the receiving
+ // liaison's name is read from s.curNode, which initCurrentNode
populates
+ // during PreRun — hence the closure indirection.
+ if cacheProvider != nil {
+ s.barrierSVC = newBarrierServiceCluster(
+ cacheProvider,
+ func() queue.Client { return tir1Client },
+ func() queue.Client { return tir2Client },
+ func() string { return
s.curNode.GetMetadata().GetName() },
Review Comment:
Technically this won't panic — `Node.GetMetadata()` and `Metadata.GetName()`
are both nil-safe (proto-gen Go always emits the `if x != nil` guard; verified
in `api/proto/banyandb/database/v1/database.pb.go` and
`api/proto/banyandb/common/v1/common.pb.go`). So
`s.curNode.GetMetadata().GetName()` returns "" on nil rather than crashing.
That said, the suggestion is in the spirit of "make the safety obvious to
readers without proto-internal knowledge." Applied an explicit `if s.curNode ==
nil` guard in 3f7a6c75 — costs 3 lines, removes any reader doubt, future-proofs
against changes to proto-gen behavior. The empty-selfName path is already
exercised by `TestFanOut_EmptyActiveSet_ReturnsUnavailable`.
--
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]
