You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by ha...@apache.org on 2026/05/08 08:39:26 UTC
(skywalking-banyandb) 11/14: Fix missing property group adapt when inspect group (#1117)
This is an automated email from the ASF dual-hosted git repository.
hanahmily pushed a commit to branch v0.10.x
in repository https://gitbox.apache.org/repos/asf/skywalking-banyandb.git
commit e4e10d309ac897e71d85622dabade3d1bb6d6e14
Author: mrproliu <74...@qq.com>
AuthorDate: Thu May 7 15:34:35 2026 +0800
Fix missing property group adapt when inspect group (#1117)
---
CHANGES.md | 1 +
banyand/metadata/schema/collector.go | 18 +++++
banyand/metadata/schema/collector_test.go | 118 ++++++++++++++++++++++++++++++
3 files changed, 137 insertions(+)
diff --git a/CHANGES.md b/CHANGES.md
index 5cd522b15..22ba94f29 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -34,6 +34,7 @@ Release Notes.
- Fix trace `block_writer` panic on out-of-order timestamps within the same traceID, which dropped one trace-write batch per panic in multi-agent SkyWalking deployments. Spans of a single trace originate from independently-clocked services, and trace storage is organized by traceID rather than timestamp, so per-traceID timestamp monotonicity is not a writer invariant.
- Fix nil-pointer panic on cold-tier data nodes when FODC `InspectAll` raced with idle-segment cleanup.
- Add `GroupLifecycleInfo.errors` to surface per-group collection failures from FODC `InspectAll` instead of silently dropping the affected node entry.
+- Fix `CollectDataInfo` and `CollectLiaisonInfo` not handling `CATALOG_PROPERTY` groups.
### Chores
diff --git a/banyand/metadata/schema/collector.go b/banyand/metadata/schema/collector.go
index 6ece113de..27d198a71 100644
--- a/banyand/metadata/schema/collector.go
+++ b/banyand/metadata/schema/collector.go
@@ -99,6 +99,15 @@ func (icr *InfoCollectorRegistry) CollectDataInfo(ctx context.Context, group str
if getErr != nil {
return nil, nil, getErr
}
+ // PROPERTY groups are key-value metadata: no DataInfoCollector is
+ // registered for them and no Topic*CollectDataInfo broadcast applies.
+ // Skip the entire pipeline so the inspector reports nothing instead
+ // of falling into the unsupported-catalog default branch. Return a
+ // non-nil empty slice to match the shape of every other success path
+ // in this method (see localInfoList and broadcastCollectDataInfo).
+ if g.Catalog == commonv1.Catalog_CATALOG_PROPERTY {
+ return []*databasev1.DataInfo{}, nil, nil
+ }
localInfo, localErr := icr.collectDataInfoLocal(ctx, g.Catalog, group)
if localErr != nil {
return nil, nil, localErr
@@ -173,6 +182,15 @@ func (icr *InfoCollectorRegistry) CollectLiaisonInfo(ctx context.Context, group
if getErr != nil {
return nil, getErr
}
+ // PROPERTY groups are key-value metadata: no LiaisonInfoCollector is
+ // registered for them and no Topic*CollectLiaisonInfo broadcast applies.
+ // Skip the entire pipeline so the inspector reports nothing instead of
+ // falling into the unsupported-catalog default branch. Return a non-nil
+ // empty slice to match the shape of every other success path in this
+ // method (see localInfoList and broadcastCollectLiaisonInfo).
+ if g.Catalog == commonv1.Catalog_CATALOG_PROPERTY {
+ return []*databasev1.LiaisonInfo{}, nil
+ }
localInfo, localErr := icr.collectLiaisonInfoLocal(ctx, g.Catalog, group)
if localErr != nil {
return nil, localErr
diff --git a/banyand/metadata/schema/collector_test.go b/banyand/metadata/schema/collector_test.go
new file mode 100644
index 000000000..5ddf49732
--- /dev/null
+++ b/banyand/metadata/schema/collector_test.go
@@ -0,0 +1,118 @@
+// Licensed to Apache Software Foundation (ASF) under one or more contributor
+// license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright
+// ownership. Apache Software Foundation (ASF) licenses this file to you under
+// the Apache License, Version 2.0 (the "License"); you may
+// not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package schema_test
+
+import (
+ "context"
+ "errors"
+ "testing"
+ "time"
+
+ "github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
+
+ commonv1 "github.com/apache/skywalking-banyandb/api/proto/banyandb/common/v1"
+ "github.com/apache/skywalking-banyandb/banyand/metadata/schema"
+ "github.com/apache/skywalking-banyandb/pkg/bus"
+ "github.com/apache/skywalking-banyandb/pkg/logger"
+)
+
+type stubGroupGetter struct {
+ group *commonv1.Group
+ err error
+}
+
+func (s *stubGroupGetter) GetGroup(_ context.Context, _ string) (*commonv1.Group, error) {
+ if s.err != nil {
+ return nil, s.err
+ }
+ return s.group, nil
+}
+
+// failingBroadcaster fails the test if Broadcast is invoked. Used to assert
+// that catalogs without a lifecycle (e.g. CATALOG_PROPERTY) short-circuit
+// before reaching the broadcast path.
+type failingBroadcaster struct {
+ t *testing.T
+}
+
+func (f *failingBroadcaster) Broadcast(_ time.Duration, _ bus.Topic, _ bus.Message) ([]bus.Future, error) {
+ f.t.Helper()
+ f.t.Fatalf("Broadcast must not be called for catalogs without lifecycle inspection")
+ return nil, errors.New("unreachable")
+}
+
+func TestCollectDataInfo_PropertyCatalogShortCircuits(t *testing.T) {
+ registry := schema.NewInfoCollectorRegistry(logger.GetLogger("test"), &stubGroupGetter{
+ group: &commonv1.Group{
+ Metadata: &commonv1.Metadata{Name: "sw_property"},
+ Catalog: commonv1.Catalog_CATALOG_PROPERTY,
+ },
+ })
+ registry.SetDataBroadcaster(&failingBroadcaster{t: t})
+
+ dataInfo, collectionErrs, err := registry.CollectDataInfo(context.Background(), "sw_property")
+ require.NoError(t, err)
+ assert.Empty(t, dataInfo)
+ assert.Empty(t, collectionErrs)
+}
+
+func TestCollectLiaisonInfo_PropertyCatalogShortCircuits(t *testing.T) {
+ registry := schema.NewInfoCollectorRegistry(logger.GetLogger("test"), &stubGroupGetter{
+ group: &commonv1.Group{
+ Metadata: &commonv1.Metadata{Name: "_deletion_task"},
+ Catalog: commonv1.Catalog_CATALOG_PROPERTY,
+ },
+ })
+ registry.SetLiaisonBroadcaster(&failingBroadcaster{t: t})
+
+ liaisonInfo, err := registry.CollectLiaisonInfo(context.Background(), "_deletion_task")
+ require.NoError(t, err)
+ assert.Empty(t, liaisonInfo)
+}
+
+func TestCollectDataInfo_UnspecifiedCatalogStillReturnsError(t *testing.T) {
+ registry := schema.NewInfoCollectorRegistry(logger.GetLogger("test"), &stubGroupGetter{
+ group: &commonv1.Group{
+ Metadata: &commonv1.Metadata{Name: "weird_group"},
+ Catalog: commonv1.Catalog_CATALOG_UNSPECIFIED,
+ },
+ })
+ registry.SetDataBroadcaster(&failingBroadcaster{t: t})
+
+ dataInfo, collectionErrs, err := registry.CollectDataInfo(context.Background(), "weird_group")
+ require.Error(t, err)
+ assert.Contains(t, err.Error(), "unsupported catalog type")
+ assert.Nil(t, dataInfo)
+ assert.Nil(t, collectionErrs)
+}
+
+func TestCollectLiaisonInfo_UnspecifiedCatalogStillReturnsError(t *testing.T) {
+ registry := schema.NewInfoCollectorRegistry(logger.GetLogger("test"), &stubGroupGetter{
+ group: &commonv1.Group{
+ Metadata: &commonv1.Metadata{Name: "weird_group"},
+ Catalog: commonv1.Catalog_CATALOG_UNSPECIFIED,
+ },
+ })
+ registry.SetLiaisonBroadcaster(&failingBroadcaster{t: t})
+
+ liaisonInfo, err := registry.CollectLiaisonInfo(context.Background(), "weird_group")
+ require.Error(t, err)
+ assert.Contains(t, err.Error(), "unsupported catalog type")
+ assert.Nil(t, liaisonInfo)
+}