-
Notifications
You must be signed in to change notification settings - Fork 106
Support file base node discovery #928
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 2 commits
1e7fa4a
9f9397f
5d31afd
f086261
ba56d0f
3fe3550
8845442
967b0ea
b568d16
afe56c5
4817136
3b89d83
218e3e3
a4fea10
32050db
02e5e9e
e4d8815
b8e92b9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -33,7 +33,8 @@ import ( | |||||||||
| "github.com/apache/skywalking-banyandb/api/common" | ||||||||||
| commonv1 "github.com/apache/skywalking-banyandb/api/proto/banyandb/common/v1" | ||||||||||
| databasev1 "github.com/apache/skywalking-banyandb/api/proto/banyandb/database/v1" | ||||||||||
| "github.com/apache/skywalking-banyandb/banyand/metadata/dns" | ||||||||||
| "github.com/apache/skywalking-banyandb/banyand/metadata/discovery/dns" | ||||||||||
| "github.com/apache/skywalking-banyandb/banyand/metadata/discovery/file" | ||||||||||
| "github.com/apache/skywalking-banyandb/banyand/metadata/schema" | ||||||||||
| "github.com/apache/skywalking-banyandb/banyand/observability" | ||||||||||
| "github.com/apache/skywalking-banyandb/pkg/logger" | ||||||||||
|
|
@@ -52,6 +53,8 @@ const ( | |||||||||
| NodeDiscoveryModeEtcd = "etcd" | ||||||||||
| // NodeDiscoveryModeDNS represents DNS-based node discovery mode. | ||||||||||
| NodeDiscoveryModeDNS = "dns" | ||||||||||
| // NodeDiscoveryModeFile represents file-based node discovery mode. | ||||||||||
| NodeDiscoveryModeFile = "file" | ||||||||||
| ) | ||||||||||
|
|
||||||||||
| const flagEtcdUsername = "etcd-username" | ||||||||||
|
|
@@ -76,6 +79,7 @@ func NewClient(toRegisterNode, forceRegisterNode bool) (Service, error) { | |||||||||
| type clientService struct { | ||||||||||
| schemaRegistry schema.Registry | ||||||||||
| dnsDiscovery *dns.Service | ||||||||||
| fileDiscovery *file.Service | ||||||||||
| closer *run.Closer | ||||||||||
| nodeInfo *databasev1.Node | ||||||||||
| etcdTLSCertFile string | ||||||||||
|
|
@@ -86,6 +90,7 @@ type clientService struct { | |||||||||
| etcdTLSKeyFile string | ||||||||||
| namespace string | ||||||||||
| nodeDiscoveryMode string | ||||||||||
| filePath string | ||||||||||
| dnsSRVAddresses []string | ||||||||||
| endpoints []string | ||||||||||
| registryTimeout time.Duration | ||||||||||
|
|
@@ -94,6 +99,7 @@ type clientService struct { | |||||||||
| dnsFetchInterval time.Duration | ||||||||||
| grpcTimeout time.Duration | ||||||||||
| etcdFullSyncInterval time.Duration | ||||||||||
| fileFetchInterval time.Duration | ||||||||||
| nodeInfoMux sync.Mutex | ||||||||||
| forceRegisterNode bool | ||||||||||
| toRegisterNode bool | ||||||||||
|
|
@@ -118,7 +124,7 @@ func (s *clientService) FlagSet() *run.FlagSet { | |||||||||
|
|
||||||||||
| // node discovery configuration | ||||||||||
| fs.StringVar(&s.nodeDiscoveryMode, "node-discovery-mode", NodeDiscoveryModeEtcd, | ||||||||||
| "Node discovery mode: 'etcd' for etcd-based discovery, 'dns' for DNS-based discovery") | ||||||||||
| "Node discovery mode: 'etcd' for etcd-based discovery, 'dns' for DNS-based discovery, 'file' for file-based discovery") | ||||||||||
| fs.StringSliceVar(&s.dnsSRVAddresses, "node-discovery-dns-srv-addresses", []string{}, | ||||||||||
| "DNS SRV addresses for node discovery (e.g., _grpc._tcp.banyandb.svc.cluster.local)") | ||||||||||
| fs.DurationVar(&s.dnsFetchInitInterval, "node-discovery-dns-fetch-init-interval", 5*time.Second, | ||||||||||
|
|
@@ -133,13 +139,19 @@ func (s *clientService) FlagSet() *run.FlagSet { | |||||||||
| "Enable TLS for DNS discovery gRPC connections") | ||||||||||
| fs.StringSliceVar(&s.dnsCACertPaths, "node-discovery-dns-ca-certs", []string{}, | ||||||||||
| "Comma-separated list of CA certificate files to verify DNS discovered nodes (one per SRV address, in same order)") | ||||||||||
| fs.StringVar(&s.filePath, "node-discovery-file-path", "", | ||||||||||
| "File path for static node configuration (file mode only)") | ||||||||||
| fs.DurationVar(&s.fileFetchInterval, "node-discovery-file-fetch-interval", 20*time.Second, | ||||||||||
| "Fetch file interval for nodes in file discovery mode") | ||||||||||
|
||||||||||
| fs.DurationVar(&s.fileFetchInterval, "node-discovery-file-fetch-interval", 20*time.Second, | |
| "Fetch file interval for nodes in file discovery mode") | |
| fs.DurationVar(&s.fileFetchInterval, "node-discovery-file-retry-interval", 20*time.Second, | |
| "Retry interval for reading node configuration file in file discovery mode") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be changed to this one: "Interval to poll the discovery file and retry failed nodes in file discovery mode".
Copilot
AI
Jan 7, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment says "skip node registration if DNS/file mode is enabled" but the actual condition only skips if the mode is DNS or File AND toRegisterNode is true. The comment should be more precise: "skip node registration if DNS/file mode is enabled (these modes don't support manual registration) or node registration is disabled".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wrong comment, for now, the judgment is correct. If there is no need to register a node, or if it is DNS discovery, or if it is file discovery, the register would be ignored.
Outdated
Copilot
AI
Jan 7, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The order of checking discovery services in NodeRegistry() is inconsistent with the order in RegisterHandler(). In RegisterHandler, file discovery is checked before DNS discovery (lines 373-379), but in NodeRegistry, DNS discovery is checked before file discovery (lines 414-422). This inconsistency could lead to confusion. Consider using a consistent priority order across all methods.
Uh oh!
There was an error while loading. Please reload this page.