Copilot commented on code in PR #865: URL: https://github.com/apache/skywalking-banyandb/pull/865#discussion_r2563566446
########## banyand/stream/disable_retention_test.go: ########## @@ -0,0 +1,241 @@ +// 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 stream + +import ( + "os" + "path/filepath" + "reflect" + "testing" + + "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/observability" + "github.com/apache/skywalking-banyandb/banyand/protector" + "github.com/apache/skywalking-banyandb/pkg/logger" +) + +func TestDisableRetentionByStage(t *testing.T) { + tempDir := t.TempDir() + l := logger.GetLogger("test-stream") + + tests := []struct { + nodeLabels map[string]string + name string + description string + stages []*commonv1.LifecycleStage + wantDisabled bool + }{ + { + name: "two stages - node in first stage", + stages: []*commonv1.LifecycleStage{ + { + Name: "cold", + NodeSelector: "role=cold", + Ttl: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 4, + }, + SegmentInterval: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 1, + }, + ShardNum: 2, + }, + }, + nodeLabels: map[string]string{ + "role": "hot", + }, + wantDisabled: true, + description: "When node is in first stage of two stages, retention should be disabled", + }, + { + name: "two stages - node in last stage", + stages: []*commonv1.LifecycleStage{ + { + Name: "cold", + NodeSelector: "role=cold", + Ttl: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 4, + }, + SegmentInterval: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 1, + }, + ShardNum: 2, + }, + }, + nodeLabels: map[string]string{ + "role": "cold", + }, + wantDisabled: false, + description: "When node is in last stage of two stages, retention should be enabled", + }, + { + name: "three stages - node in warm stage", + stages: []*commonv1.LifecycleStage{ + { + Name: "warm", + NodeSelector: "role=warm", + Ttl: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 3, + }, + SegmentInterval: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 1, + }, + ShardNum: 2, + }, + { + Name: "cold", + NodeSelector: "role=cold", + Ttl: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 2, + }, + SegmentInterval: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 1, + }, + ShardNum: 2, + }, + }, + nodeLabels: map[string]string{ + "role": "warm", + }, + wantDisabled: true, + description: "When node is in middle stage of three stages, retention should be disabled", + }, + { + name: "three stages - node in cold stage", + stages: []*commonv1.LifecycleStage{ + { + Name: "warm", + NodeSelector: "role=warm", + Ttl: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 3, + }, + SegmentInterval: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 1, + }, + ShardNum: 2, + }, + { + Name: "cold", + NodeSelector: "role=cold", + Ttl: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 2, + }, + SegmentInterval: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 1, + }, + ShardNum: 2, + }, + }, + nodeLabels: map[string]string{ + "role": "cold", + }, + wantDisabled: false, + description: "When node is in middle stage of three stages, retention should be disabled", Review Comment: The description states "When node is in middle stage of three stages" but the test case is actually for a node in the cold (last) stage. The description should be "When node is in last stage of three stages, retention should be enabled". ```suggestion description: "When node is in last stage of three stages, retention should be enabled", ``` ########## banyand/stream/metadata.go: ########## @@ -331,14 +333,19 @@ func (s *supplier) OpenDB(groupSchema *commonv1.Group) (resourceSchema.DB, error if !selector.Matches(s.nodeLabels) { continue } + foundMatched = true ttl.Num += ttlNum shardNum = st.ShardNum segInterval = st.SegmentInterval if st.Close { segmentIdleTimeout = 5 * time.Minute } + disableRetention = i+1 < len(ro.Stages) Review Comment: Missing spaces around the `+` operator. Should be `i + 1` instead of `i+1` for consistency with Go formatting conventions. ```suggestion disableRetention = i + 1 < len(ro.Stages) ``` ########## banyand/measure/disable_retention_test.go: ########## @@ -0,0 +1,242 @@ +// 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 measure + +import ( + "os" + "path/filepath" + "reflect" + "testing" + + "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/observability" + "github.com/apache/skywalking-banyandb/banyand/protector" + "github.com/apache/skywalking-banyandb/pkg/logger" +) + +func TestDisableRetentionByStage(t *testing.T) { + tempDir := t.TempDir() + l := logger.GetLogger("test") + + tests := []struct { + nodeLabels map[string]string + name string + description string + stages []*commonv1.LifecycleStage + wantDisabled bool + }{ + { + name: "two stages - node in first stage", + stages: []*commonv1.LifecycleStage{ + { + Name: "cold", + NodeSelector: "role=cold", + Ttl: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 4, + }, + SegmentInterval: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 1, + }, + ShardNum: 2, + }, + }, + nodeLabels: map[string]string{ + "role": "hot", + }, + wantDisabled: true, + description: "When node is in first stage of two stages, retention should be disabled", + }, + { + name: "two stages - node in last stage", + stages: []*commonv1.LifecycleStage{ + { + Name: "cold", + NodeSelector: "role=cold", + Ttl: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 4, + }, + SegmentInterval: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 1, + }, + ShardNum: 2, + }, + }, + nodeLabels: map[string]string{ + "role": "cold", + }, + wantDisabled: false, + description: "When node is in last stage of two stages, retention should be enabled", + }, + { + name: "three stages - node in warm stage", + stages: []*commonv1.LifecycleStage{ + { + Name: "warm", + NodeSelector: "role=warm", + Ttl: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 3, + }, + SegmentInterval: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 1, + }, + ShardNum: 2, + }, + { + Name: "cold", + NodeSelector: "role=cold", + Ttl: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 2, + }, + SegmentInterval: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 1, + }, + ShardNum: 2, + }, + }, + nodeLabels: map[string]string{ + "role": "warm", + }, + wantDisabled: true, + description: "When node is in middle stage of three stages, retention should be disabled", + }, + { + name: "three stages - node in cold stage", + stages: []*commonv1.LifecycleStage{ + { + Name: "warm", + NodeSelector: "role=warm", + Ttl: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 3, + }, + SegmentInterval: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 1, + }, + ShardNum: 2, + }, + { + Name: "cold", + NodeSelector: "role=cold", + Ttl: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 2, + }, + SegmentInterval: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 1, + }, + ShardNum: 2, + }, + }, + nodeLabels: map[string]string{ + "role": "cold", + }, + wantDisabled: false, + description: "When node is in middle stage of three stages, retention should be disabled", + }, Review Comment: Test case name says "three stages" but only two stages are defined in the `stages` slice (warm and cold). Consider renaming to "two stages - node in cold stage" or adding a third stage to match the test name. ########## banyand/measure/disable_retention_test.go: ########## @@ -0,0 +1,242 @@ +// 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 measure + +import ( + "os" + "path/filepath" + "reflect" + "testing" + + "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/observability" + "github.com/apache/skywalking-banyandb/banyand/protector" + "github.com/apache/skywalking-banyandb/pkg/logger" +) + +func TestDisableRetentionByStage(t *testing.T) { + tempDir := t.TempDir() + l := logger.GetLogger("test") + + tests := []struct { + nodeLabels map[string]string + name string + description string + stages []*commonv1.LifecycleStage + wantDisabled bool + }{ + { + name: "two stages - node in first stage", + stages: []*commonv1.LifecycleStage{ + { + Name: "cold", + NodeSelector: "role=cold", + Ttl: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 4, + }, + SegmentInterval: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 1, + }, + ShardNum: 2, + }, + }, + nodeLabels: map[string]string{ + "role": "hot", + }, + wantDisabled: true, + description: "When node is in first stage of two stages, retention should be disabled", + }, + { + name: "two stages - node in last stage", + stages: []*commonv1.LifecycleStage{ + { + Name: "cold", + NodeSelector: "role=cold", + Ttl: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 4, + }, + SegmentInterval: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 1, + }, + ShardNum: 2, + }, + }, + nodeLabels: map[string]string{ + "role": "cold", + }, + wantDisabled: false, + description: "When node is in last stage of two stages, retention should be enabled", + }, + { + name: "three stages - node in warm stage", + stages: []*commonv1.LifecycleStage{ + { + Name: "warm", + NodeSelector: "role=warm", + Ttl: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 3, + }, + SegmentInterval: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 1, + }, + ShardNum: 2, + }, + { + Name: "cold", + NodeSelector: "role=cold", + Ttl: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 2, + }, + SegmentInterval: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 1, + }, + ShardNum: 2, + }, + }, + nodeLabels: map[string]string{ + "role": "warm", + }, + wantDisabled: true, + description: "When node is in middle stage of three stages, retention should be disabled", + }, Review Comment: Test case name says "three stages" but only two stages are defined in the `stages` slice (warm and cold). Consider renaming to "two stages - node in warm stage" or adding a third stage to match the test name. ########## banyand/trace/disable_retention_test.go: ########## @@ -0,0 +1,241 @@ +// 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 trace + +import ( + "os" + "path/filepath" + "reflect" + "testing" + + "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/observability" + "github.com/apache/skywalking-banyandb/banyand/protector" + "github.com/apache/skywalking-banyandb/pkg/logger" +) + +func TestDisableRetentionByStage(t *testing.T) { + tempDir := t.TempDir() + l := logger.GetLogger("test-trace") + + tests := []struct { + nodeLabels map[string]string + name string + description string + stages []*commonv1.LifecycleStage + wantDisabled bool + }{ + { + name: "two stages - node in first stage", + stages: []*commonv1.LifecycleStage{ + { + Name: "cold", + NodeSelector: "role=cold", + Ttl: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 4, + }, + SegmentInterval: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 1, + }, + ShardNum: 2, + }, + }, + nodeLabels: map[string]string{ + "role": "hot", + }, + wantDisabled: true, + description: "When node is in first stage of two stages, retention should be disabled", + }, + { + name: "two stages - node in last stage", + stages: []*commonv1.LifecycleStage{ + { + Name: "cold", + NodeSelector: "role=cold", + Ttl: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 4, + }, + SegmentInterval: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 1, + }, + ShardNum: 2, + }, + }, + nodeLabels: map[string]string{ + "role": "cold", + }, + wantDisabled: false, + description: "When node is in last stage of two stages, retention should be enabled", + }, + { + name: "three stages - node in warm stage", + stages: []*commonv1.LifecycleStage{ + { + Name: "warm", + NodeSelector: "role=warm", + Ttl: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 3, + }, + SegmentInterval: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 1, + }, + ShardNum: 2, + }, + { + Name: "cold", + NodeSelector: "role=cold", + Ttl: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 2, + }, + SegmentInterval: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 1, + }, + ShardNum: 2, + }, + }, + nodeLabels: map[string]string{ + "role": "warm", + }, + wantDisabled: true, + description: "When node is in middle stage of three stages, retention should be disabled", + }, + { + name: "three stages - node in cold stage", + stages: []*commonv1.LifecycleStage{ + { + Name: "warm", + NodeSelector: "role=warm", + Ttl: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 3, + }, + SegmentInterval: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 1, + }, + ShardNum: 2, + }, + { + Name: "cold", + NodeSelector: "role=cold", + Ttl: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 2, + }, + SegmentInterval: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 1, + }, + ShardNum: 2, + }, + }, + nodeLabels: map[string]string{ + "role": "cold", + }, + wantDisabled: false, + description: "When node is in middle stage of three stages, retention should be disabled", + }, Review Comment: Test case name says "three stages" but only two stages are defined in the `stages` slice (warm and cold). Consider renaming to "two stages - node in cold stage" or adding a third stage to match the test name. ########## banyand/trace/disable_retention_test.go: ########## @@ -0,0 +1,241 @@ +// 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 trace + +import ( + "os" + "path/filepath" + "reflect" + "testing" + + "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/observability" + "github.com/apache/skywalking-banyandb/banyand/protector" + "github.com/apache/skywalking-banyandb/pkg/logger" +) + +func TestDisableRetentionByStage(t *testing.T) { + tempDir := t.TempDir() + l := logger.GetLogger("test-trace") + + tests := []struct { + nodeLabels map[string]string + name string + description string + stages []*commonv1.LifecycleStage + wantDisabled bool + }{ + { + name: "two stages - node in first stage", + stages: []*commonv1.LifecycleStage{ + { + Name: "cold", + NodeSelector: "role=cold", + Ttl: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 4, + }, + SegmentInterval: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 1, + }, + ShardNum: 2, + }, + }, + nodeLabels: map[string]string{ + "role": "hot", + }, + wantDisabled: true, + description: "When node is in first stage of two stages, retention should be disabled", + }, + { + name: "two stages - node in last stage", + stages: []*commonv1.LifecycleStage{ + { + Name: "cold", + NodeSelector: "role=cold", + Ttl: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 4, + }, + SegmentInterval: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 1, + }, + ShardNum: 2, + }, + }, + nodeLabels: map[string]string{ + "role": "cold", + }, + wantDisabled: false, + description: "When node is in last stage of two stages, retention should be enabled", + }, Review Comment: Test case name says "two stages" but only one stage is defined in the `stages` slice. Consider renaming to "single stage - node matches stage" or adding a second stage to match the test name. ########## banyand/measure/metadata.go: ########## @@ -508,14 +510,19 @@ func (s *supplier) OpenDB(groupSchema *commonv1.Group) (resourceSchema.DB, error if !selector.Matches(s.nodeLabels) { continue } + foundMatched = true ttl.Num += ttlNum shardNum = st.ShardNum segInterval = st.SegmentInterval if st.Close { segmentIdleTimeout = 5 * time.Minute } + disableRetention = i+1 < len(ro.Stages) Review Comment: Missing spaces around the `+` operator. Should be `i + 1` instead of `i+1` for consistency with Go formatting conventions. ```suggestion disableRetention = i + 1 < len(ro.Stages) ``` ########## banyand/trace/disable_retention_test.go: ########## @@ -0,0 +1,241 @@ +// 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 trace + +import ( + "os" + "path/filepath" + "reflect" + "testing" + + "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/observability" + "github.com/apache/skywalking-banyandb/banyand/protector" + "github.com/apache/skywalking-banyandb/pkg/logger" +) + +func TestDisableRetentionByStage(t *testing.T) { + tempDir := t.TempDir() + l := logger.GetLogger("test-trace") + + tests := []struct { + nodeLabels map[string]string + name string + description string + stages []*commonv1.LifecycleStage + wantDisabled bool + }{ + { + name: "two stages - node in first stage", + stages: []*commonv1.LifecycleStage{ + { + Name: "cold", + NodeSelector: "role=cold", + Ttl: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 4, + }, + SegmentInterval: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 1, + }, + ShardNum: 2, + }, + }, + nodeLabels: map[string]string{ + "role": "hot", + }, + wantDisabled: true, + description: "When node is in first stage of two stages, retention should be disabled", + }, Review Comment: Test case name says "two stages" but only one stage is defined in the `stages` slice. The test is actually checking what happens when a node doesn't match any defined stage (node has role=hot but the only stage requires role=cold). Consider renaming to "no matching stage - retention should be disabled" or adding a second stage to match the test name. ########## banyand/trace/disable_retention_test.go: ########## @@ -0,0 +1,241 @@ +// 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 trace + +import ( + "os" + "path/filepath" + "reflect" + "testing" + + "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/observability" + "github.com/apache/skywalking-banyandb/banyand/protector" + "github.com/apache/skywalking-banyandb/pkg/logger" +) + +func TestDisableRetentionByStage(t *testing.T) { + tempDir := t.TempDir() + l := logger.GetLogger("test-trace") + + tests := []struct { + nodeLabels map[string]string + name string + description string + stages []*commonv1.LifecycleStage + wantDisabled bool + }{ + { + name: "two stages - node in first stage", + stages: []*commonv1.LifecycleStage{ + { + Name: "cold", + NodeSelector: "role=cold", + Ttl: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 4, + }, + SegmentInterval: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 1, + }, + ShardNum: 2, + }, + }, + nodeLabels: map[string]string{ + "role": "hot", + }, + wantDisabled: true, + description: "When node is in first stage of two stages, retention should be disabled", + }, + { + name: "two stages - node in last stage", + stages: []*commonv1.LifecycleStage{ + { + Name: "cold", + NodeSelector: "role=cold", + Ttl: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 4, + }, + SegmentInterval: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 1, + }, + ShardNum: 2, + }, + }, + nodeLabels: map[string]string{ + "role": "cold", + }, + wantDisabled: false, + description: "When node is in last stage of two stages, retention should be enabled", + }, + { + name: "three stages - node in warm stage", + stages: []*commonv1.LifecycleStage{ + { + Name: "warm", + NodeSelector: "role=warm", + Ttl: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 3, + }, + SegmentInterval: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 1, + }, + ShardNum: 2, + }, + { + Name: "cold", + NodeSelector: "role=cold", + Ttl: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 2, + }, + SegmentInterval: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 1, + }, + ShardNum: 2, + }, + }, + nodeLabels: map[string]string{ + "role": "warm", + }, + wantDisabled: true, + description: "When node is in middle stage of three stages, retention should be disabled", + }, Review Comment: Test case name says "three stages" but only two stages are defined in the `stages` slice (warm and cold). Consider renaming to "two stages - node in warm stage" or adding a third stage to match the test name. ########## banyand/measure/disable_retention_test.go: ########## @@ -0,0 +1,242 @@ +// 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 measure + +import ( + "os" + "path/filepath" + "reflect" + "testing" + + "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/observability" + "github.com/apache/skywalking-banyandb/banyand/protector" + "github.com/apache/skywalking-banyandb/pkg/logger" +) + +func TestDisableRetentionByStage(t *testing.T) { + tempDir := t.TempDir() + l := logger.GetLogger("test") + + tests := []struct { + nodeLabels map[string]string + name string + description string + stages []*commonv1.LifecycleStage + wantDisabled bool + }{ + { + name: "two stages - node in first stage", + stages: []*commonv1.LifecycleStage{ + { + Name: "cold", + NodeSelector: "role=cold", + Ttl: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 4, + }, + SegmentInterval: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 1, + }, + ShardNum: 2, + }, + }, + nodeLabels: map[string]string{ + "role": "hot", + }, + wantDisabled: true, + description: "When node is in first stage of two stages, retention should be disabled", + }, Review Comment: Test case name says "two stages" but only one stage is defined in the `stages` slice. The test is actually checking what happens when a node doesn't match any defined stage (node has role=hot but the only stage requires role=cold). Consider renaming to "no matching stage - retention should be disabled" or adding a second stage to match the test name. ########## banyand/stream/disable_retention_test.go: ########## @@ -0,0 +1,241 @@ +// 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 stream + +import ( + "os" + "path/filepath" + "reflect" + "testing" + + "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/observability" + "github.com/apache/skywalking-banyandb/banyand/protector" + "github.com/apache/skywalking-banyandb/pkg/logger" +) + +func TestDisableRetentionByStage(t *testing.T) { + tempDir := t.TempDir() + l := logger.GetLogger("test-stream") + + tests := []struct { + nodeLabels map[string]string + name string + description string + stages []*commonv1.LifecycleStage + wantDisabled bool + }{ + { + name: "two stages - node in first stage", + stages: []*commonv1.LifecycleStage{ + { + Name: "cold", + NodeSelector: "role=cold", + Ttl: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 4, + }, + SegmentInterval: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 1, + }, + ShardNum: 2, + }, + }, + nodeLabels: map[string]string{ + "role": "hot", + }, + wantDisabled: true, + description: "When node is in first stage of two stages, retention should be disabled", + }, + { + name: "two stages - node in last stage", + stages: []*commonv1.LifecycleStage{ + { + Name: "cold", + NodeSelector: "role=cold", + Ttl: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 4, + }, + SegmentInterval: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 1, + }, + ShardNum: 2, + }, + }, + nodeLabels: map[string]string{ + "role": "cold", + }, + wantDisabled: false, + description: "When node is in last stage of two stages, retention should be enabled", + }, + { + name: "three stages - node in warm stage", + stages: []*commonv1.LifecycleStage{ + { + Name: "warm", + NodeSelector: "role=warm", + Ttl: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 3, + }, + SegmentInterval: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 1, + }, + ShardNum: 2, + }, + { + Name: "cold", + NodeSelector: "role=cold", + Ttl: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 2, + }, + SegmentInterval: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 1, + }, + ShardNum: 2, + }, + }, + nodeLabels: map[string]string{ + "role": "warm", + }, + wantDisabled: true, + description: "When node is in middle stage of three stages, retention should be disabled", + }, Review Comment: Test case name says "three stages" but only two stages are defined in the `stages` slice (warm and cold). Consider renaming to "two stages - node in warm stage" or adding a third stage to match the test name. ########## banyand/measure/disable_retention_test.go: ########## @@ -0,0 +1,242 @@ +// 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 measure + +import ( + "os" + "path/filepath" + "reflect" + "testing" + + "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/observability" + "github.com/apache/skywalking-banyandb/banyand/protector" + "github.com/apache/skywalking-banyandb/pkg/logger" +) + +func TestDisableRetentionByStage(t *testing.T) { + tempDir := t.TempDir() + l := logger.GetLogger("test") + + tests := []struct { + nodeLabels map[string]string + name string + description string + stages []*commonv1.LifecycleStage + wantDisabled bool + }{ + { + name: "two stages - node in first stage", + stages: []*commonv1.LifecycleStage{ + { + Name: "cold", + NodeSelector: "role=cold", + Ttl: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 4, + }, + SegmentInterval: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 1, + }, + ShardNum: 2, + }, + }, + nodeLabels: map[string]string{ + "role": "hot", + }, + wantDisabled: true, + description: "When node is in first stage of two stages, retention should be disabled", + }, + { + name: "two stages - node in last stage", + stages: []*commonv1.LifecycleStage{ + { + Name: "cold", + NodeSelector: "role=cold", + Ttl: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 4, + }, + SegmentInterval: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 1, + }, + ShardNum: 2, + }, + }, + nodeLabels: map[string]string{ + "role": "cold", + }, + wantDisabled: false, + description: "When node is in last stage of two stages, retention should be enabled", + }, Review Comment: Test case name says "two stages" but only one stage is defined in the `stages` slice. Consider renaming to "single stage - node matches stage" or adding a second stage to match the test name. ########## banyand/stream/disable_retention_test.go: ########## @@ -0,0 +1,241 @@ +// 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 stream + +import ( + "os" + "path/filepath" + "reflect" + "testing" + + "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/observability" + "github.com/apache/skywalking-banyandb/banyand/protector" + "github.com/apache/skywalking-banyandb/pkg/logger" +) + +func TestDisableRetentionByStage(t *testing.T) { + tempDir := t.TempDir() + l := logger.GetLogger("test-stream") + + tests := []struct { + nodeLabels map[string]string + name string + description string + stages []*commonv1.LifecycleStage + wantDisabled bool + }{ + { + name: "two stages - node in first stage", + stages: []*commonv1.LifecycleStage{ + { + Name: "cold", + NodeSelector: "role=cold", + Ttl: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 4, + }, + SegmentInterval: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 1, + }, + ShardNum: 2, + }, + }, + nodeLabels: map[string]string{ + "role": "hot", + }, + wantDisabled: true, + description: "When node is in first stage of two stages, retention should be disabled", + }, Review Comment: Test case name says "two stages" but only one stage is defined in the `stages` slice. The test is actually checking what happens when a node doesn't match any defined stage (node has role=hot but the only stage requires role=cold). Consider renaming to "no matching stage - retention should be disabled" or adding a second stage to match the test name. ########## banyand/measure/disable_retention_test.go: ########## @@ -0,0 +1,242 @@ +// 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 measure + +import ( + "os" + "path/filepath" + "reflect" + "testing" + + "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/observability" + "github.com/apache/skywalking-banyandb/banyand/protector" + "github.com/apache/skywalking-banyandb/pkg/logger" +) + +func TestDisableRetentionByStage(t *testing.T) { + tempDir := t.TempDir() + l := logger.GetLogger("test") + + tests := []struct { + nodeLabels map[string]string + name string + description string + stages []*commonv1.LifecycleStage + wantDisabled bool + }{ + { + name: "two stages - node in first stage", + stages: []*commonv1.LifecycleStage{ + { + Name: "cold", + NodeSelector: "role=cold", + Ttl: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 4, + }, + SegmentInterval: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 1, + }, + ShardNum: 2, + }, + }, + nodeLabels: map[string]string{ + "role": "hot", + }, + wantDisabled: true, + description: "When node is in first stage of two stages, retention should be disabled", + }, + { + name: "two stages - node in last stage", + stages: []*commonv1.LifecycleStage{ + { + Name: "cold", + NodeSelector: "role=cold", + Ttl: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 4, + }, + SegmentInterval: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 1, + }, + ShardNum: 2, + }, + }, + nodeLabels: map[string]string{ + "role": "cold", + }, + wantDisabled: false, + description: "When node is in last stage of two stages, retention should be enabled", + }, + { + name: "three stages - node in warm stage", + stages: []*commonv1.LifecycleStage{ + { + Name: "warm", + NodeSelector: "role=warm", + Ttl: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 3, + }, + SegmentInterval: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 1, + }, + ShardNum: 2, + }, + { + Name: "cold", + NodeSelector: "role=cold", + Ttl: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 2, + }, + SegmentInterval: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 1, + }, + ShardNum: 2, + }, + }, + nodeLabels: map[string]string{ + "role": "warm", + }, + wantDisabled: true, + description: "When node is in middle stage of three stages, retention should be disabled", + }, + { + name: "three stages - node in cold stage", + stages: []*commonv1.LifecycleStage{ + { + Name: "warm", + NodeSelector: "role=warm", + Ttl: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 3, + }, + SegmentInterval: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 1, + }, + ShardNum: 2, + }, + { + Name: "cold", + NodeSelector: "role=cold", + Ttl: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 2, + }, + SegmentInterval: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 1, + }, + ShardNum: 2, + }, + }, + nodeLabels: map[string]string{ + "role": "cold", + }, + wantDisabled: false, + description: "When node is in middle stage of three stages, retention should be disabled", Review Comment: The description states "When node is in middle stage of three stages" but the test case is actually for a node in the cold (last) stage. The description should be "When node is in last stage of three stages, retention should be enabled". ```suggestion description: "When node is in last stage of three stages, retention should be enabled", ``` ########## CHANGES.md: ########## @@ -8,6 +8,10 @@ Release Notes. - Remove Bloom filter for dictionary-encoded tags. +### Bug Fixes + +- Fix the wrong detention setting of each measure/stream/trace. Review Comment: Typo: "detention" should be "retention". The fix is about data retention settings, not detention. ```suggestion - Fix the wrong retention setting of each measure/stream/trace. ``` ########## banyand/stream/disable_retention_test.go: ########## @@ -0,0 +1,241 @@ +// 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 stream + +import ( + "os" + "path/filepath" + "reflect" + "testing" + + "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/observability" + "github.com/apache/skywalking-banyandb/banyand/protector" + "github.com/apache/skywalking-banyandb/pkg/logger" +) + +func TestDisableRetentionByStage(t *testing.T) { + tempDir := t.TempDir() + l := logger.GetLogger("test-stream") + + tests := []struct { + nodeLabels map[string]string + name string + description string + stages []*commonv1.LifecycleStage + wantDisabled bool + }{ + { + name: "two stages - node in first stage", + stages: []*commonv1.LifecycleStage{ + { + Name: "cold", + NodeSelector: "role=cold", + Ttl: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 4, + }, + SegmentInterval: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 1, + }, + ShardNum: 2, + }, + }, + nodeLabels: map[string]string{ + "role": "hot", + }, + wantDisabled: true, + description: "When node is in first stage of two stages, retention should be disabled", + }, + { + name: "two stages - node in last stage", + stages: []*commonv1.LifecycleStage{ + { + Name: "cold", + NodeSelector: "role=cold", + Ttl: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 4, + }, + SegmentInterval: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 1, + }, + ShardNum: 2, + }, + }, + nodeLabels: map[string]string{ + "role": "cold", + }, + wantDisabled: false, + description: "When node is in last stage of two stages, retention should be enabled", + }, Review Comment: Test case name says "two stages" but only one stage is defined in the `stages` slice. Consider renaming to "single stage - node matches stage" or adding a second stage to match the test name. ########## banyand/trace/metadata.go: ########## @@ -336,14 +338,19 @@ func (s *supplier) OpenDB(groupSchema *commonv1.Group) (resourceSchema.DB, error if !selector.Matches(s.nodeLabels) { continue } + foundMatched = true ttl.Num += ttlNum shardNum = st.ShardNum segInterval = st.SegmentInterval if st.Close { segmentIdleTimeout = 5 * time.Minute } + disableRetention = i+1 < len(ro.Stages) Review Comment: Missing spaces around the `+` operator. Should be `i + 1` instead of `i+1` for consistency with Go formatting conventions. ```suggestion disableRetention = i + 1 < len(ro.Stages) ``` ########## banyand/stream/disable_retention_test.go: ########## @@ -0,0 +1,241 @@ +// 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 stream + +import ( + "os" + "path/filepath" + "reflect" + "testing" + + "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/observability" + "github.com/apache/skywalking-banyandb/banyand/protector" + "github.com/apache/skywalking-banyandb/pkg/logger" +) + +func TestDisableRetentionByStage(t *testing.T) { + tempDir := t.TempDir() + l := logger.GetLogger("test-stream") + + tests := []struct { + nodeLabels map[string]string + name string + description string + stages []*commonv1.LifecycleStage + wantDisabled bool + }{ + { + name: "two stages - node in first stage", + stages: []*commonv1.LifecycleStage{ + { + Name: "cold", + NodeSelector: "role=cold", + Ttl: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 4, + }, + SegmentInterval: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 1, + }, + ShardNum: 2, + }, + }, + nodeLabels: map[string]string{ + "role": "hot", + }, + wantDisabled: true, + description: "When node is in first stage of two stages, retention should be disabled", + }, + { + name: "two stages - node in last stage", + stages: []*commonv1.LifecycleStage{ + { + Name: "cold", + NodeSelector: "role=cold", + Ttl: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 4, + }, + SegmentInterval: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 1, + }, + ShardNum: 2, + }, + }, + nodeLabels: map[string]string{ + "role": "cold", + }, + wantDisabled: false, + description: "When node is in last stage of two stages, retention should be enabled", + }, + { + name: "three stages - node in warm stage", + stages: []*commonv1.LifecycleStage{ + { + Name: "warm", + NodeSelector: "role=warm", + Ttl: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 3, + }, + SegmentInterval: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 1, + }, + ShardNum: 2, + }, + { + Name: "cold", + NodeSelector: "role=cold", + Ttl: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 2, + }, + SegmentInterval: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 1, + }, + ShardNum: 2, + }, + }, + nodeLabels: map[string]string{ + "role": "warm", + }, + wantDisabled: true, + description: "When node is in middle stage of three stages, retention should be disabled", + }, + { + name: "three stages - node in cold stage", + stages: []*commonv1.LifecycleStage{ + { + Name: "warm", + NodeSelector: "role=warm", + Ttl: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 3, + }, + SegmentInterval: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 1, + }, + ShardNum: 2, + }, + { + Name: "cold", + NodeSelector: "role=cold", + Ttl: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 2, + }, + SegmentInterval: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 1, + }, + ShardNum: 2, + }, + }, + nodeLabels: map[string]string{ + "role": "cold", + }, + wantDisabled: false, + description: "When node is in middle stage of three stages, retention should be disabled", + }, Review Comment: Test case name says "three stages" but only two stages are defined in the `stages` slice (warm and cold). Consider renaming to "two stages - node in cold stage" or adding a third stage to match the test name. ########## banyand/trace/disable_retention_test.go: ########## @@ -0,0 +1,241 @@ +// 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 trace + +import ( + "os" + "path/filepath" + "reflect" + "testing" + + "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/observability" + "github.com/apache/skywalking-banyandb/banyand/protector" + "github.com/apache/skywalking-banyandb/pkg/logger" +) + +func TestDisableRetentionByStage(t *testing.T) { + tempDir := t.TempDir() + l := logger.GetLogger("test-trace") + + tests := []struct { + nodeLabels map[string]string + name string + description string + stages []*commonv1.LifecycleStage + wantDisabled bool + }{ + { + name: "two stages - node in first stage", + stages: []*commonv1.LifecycleStage{ + { + Name: "cold", + NodeSelector: "role=cold", + Ttl: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 4, + }, + SegmentInterval: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 1, + }, + ShardNum: 2, + }, + }, + nodeLabels: map[string]string{ + "role": "hot", + }, + wantDisabled: true, + description: "When node is in first stage of two stages, retention should be disabled", + }, + { + name: "two stages - node in last stage", + stages: []*commonv1.LifecycleStage{ + { + Name: "cold", + NodeSelector: "role=cold", + Ttl: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 4, + }, + SegmentInterval: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 1, + }, + ShardNum: 2, + }, + }, + nodeLabels: map[string]string{ + "role": "cold", + }, + wantDisabled: false, + description: "When node is in last stage of two stages, retention should be enabled", + }, + { + name: "three stages - node in warm stage", + stages: []*commonv1.LifecycleStage{ + { + Name: "warm", + NodeSelector: "role=warm", + Ttl: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 3, + }, + SegmentInterval: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 1, + }, + ShardNum: 2, + }, + { + Name: "cold", + NodeSelector: "role=cold", + Ttl: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 2, + }, + SegmentInterval: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 1, + }, + ShardNum: 2, + }, + }, + nodeLabels: map[string]string{ + "role": "warm", + }, + wantDisabled: true, + description: "When node is in middle stage of three stages, retention should be disabled", + }, + { + name: "three stages - node in cold stage", + stages: []*commonv1.LifecycleStage{ + { + Name: "warm", + NodeSelector: "role=warm", + Ttl: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 3, + }, + SegmentInterval: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 1, + }, + ShardNum: 2, + }, + { + Name: "cold", + NodeSelector: "role=cold", + Ttl: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 2, + }, + SegmentInterval: &commonv1.IntervalRule{ + Unit: commonv1.IntervalRule_UNIT_DAY, + Num: 1, + }, + ShardNum: 2, + }, + }, + nodeLabels: map[string]string{ + "role": "cold", + }, + wantDisabled: false, + description: "When node is in middle stage of three stages, retention should be disabled", Review Comment: The description states "When node is in middle stage of three stages" but the test case is actually for a node in the cold (last) stage. The description should be "When node is in last stage of three stages, retention should be enabled". ```suggestion description: "When node is in last stage of three stages, retention should be enabled", ``` -- 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]
