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)
+}