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 <[email protected]> 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) +}
