From 501c0108e9771bdcaabf6386e2852213cf754cb3 Mon Sep 17 00:00:00 2001 From: ajeanlaurent Date: Mon, 23 Sep 2024 01:08:08 +0000 Subject: [PATCH 01/18] intial commit --- scripts/proto-gen.sh | 3 + src/aggregator/aggregator/handler/config.go | 2 + .../handler/filter/percentageFilter.go | 2 +- .../handler/filter/percentageFilter_test.go | 6 +- .../aggregator/handler/filter/shard.go | 2 +- .../aggregator/handler/filter/shard_test.go | 8 +- .../aggregator/handler/writer/protobuf.go | 2 +- .../handler/writer/protobuf_test.go | 6 +- src/msg/generated/proto/msgpb/msg.pb.go | 20 - src/msg/generated/proto/topicpb/topic.pb.go | 780 ++++++++++++++++-- src/msg/generated/proto/topicpb/topic.proto | 19 + src/msg/producer/ref_counted.go | 2 +- src/msg/producer/ref_counted_test.go | 17 +- src/msg/producer/types.go | 22 +- .../writer/consumer_service_writer.go | 17 +- .../writer/consumer_service_writer_mock.go | 12 + .../writer/consumer_service_writer_test.go | 4 +- src/msg/producer/writer/writer.go | 30 +- src/msg/producer/writer/writer_test.go | 4 +- src/msg/topic/topic.go | 12 + src/msg/topic/types.go | 21 + 21 files changed, 895 insertions(+), 96 deletions(-) diff --git a/scripts/proto-gen.sh b/scripts/proto-gen.sh index d108b15fed..597ca817cb 100755 --- a/scripts/proto-gen.sh +++ b/scripts/proto-gen.sh @@ -17,10 +17,13 @@ if [[ -n "$BUILDKITE" ]]; then fi PROTO_SRC=$1 +GOPATH="/home/user/Uber/gocode" for i in "${GOPATH}/src/${PROTO_SRC}"/*; do + echo $i if ! [ -d $i ]; then continue fi + echo $i if ls $i/*.proto > /dev/null 2>&1; then proto_files=$(ls $i/*.proto | sed -e "s@${GOPATH}@@g") diff --git a/src/aggregator/aggregator/handler/config.go b/src/aggregator/aggregator/handler/config.go index aff25d1838..759553f5c2 100644 --- a/src/aggregator/aggregator/handler/config.go +++ b/src/aggregator/aggregator/handler/config.go @@ -186,6 +186,7 @@ func (c *DynamicBackendConfiguration) newProtobufHandler( return nil, err } logger := instrumentOpts.Logger() + // NOTE: where we setup the static filters for _, filter := range c.ShardSetFilters { sid, f := filter.NewConsumerServiceFilter() p.RegisterFilter(sid, f) @@ -215,6 +216,7 @@ type storagePolicyFilterConfiguration struct { StoragePolicies []policy.StoragePolicy `yaml:"storagePolicies" validate:"nonzero"` } + func (c storagePolicyFilterConfiguration) NewConsumerServiceFilter() (services.ServiceID, producer.FilterFunc) { return c.ServiceID.NewServiceID(), writer.NewStoragePolicyFilter(c.StoragePolicies) } diff --git a/src/aggregator/aggregator/handler/filter/percentageFilter.go b/src/aggregator/aggregator/handler/filter/percentageFilter.go index dfac9b62ba..27214497cc 100644 --- a/src/aggregator/aggregator/handler/filter/percentageFilter.go +++ b/src/aggregator/aggregator/handler/filter/percentageFilter.go @@ -41,7 +41,7 @@ func NewPercentageFilter(percentage float64) producer.FilterFunc { } f := percentageFilter{rate: rate} - return f.Filter + return producer.NewFilterFunc(f.Filter, producer.PercentageFilter) } func (f percentageFilter) Filter(_ producer.Message) bool { diff --git a/src/aggregator/aggregator/handler/filter/percentageFilter_test.go b/src/aggregator/aggregator/handler/filter/percentageFilter_test.go index 3f848643eb..21b1f1e211 100644 --- a/src/aggregator/aggregator/handler/filter/percentageFilter_test.go +++ b/src/aggregator/aggregator/handler/filter/percentageFilter_test.go @@ -35,10 +35,10 @@ func TestPercentageFilter(t *testing.T) { mm := producer.NewMockMessage(ctrl) f0 := NewPercentageFilter(0) - require.False(t, f0(mm)) + require.False(t, f0.Function((mm))) f1 := NewPercentageFilter(1) - require.True(t, f1(mm)) + require.True(t, f1.Function(mm)) } var filterResult bool @@ -58,7 +58,7 @@ func BenchmarkPercentageFilter(b *testing.B) { f := NewPercentageFilter(0.5) var r bool for i := 0; i < b.N; i++ { - r = f(nil) + r = f.Function(nil) } filterResult = r } diff --git a/src/aggregator/aggregator/handler/filter/shard.go b/src/aggregator/aggregator/handler/filter/shard.go index b81a94fec2..b3bbef8609 100644 --- a/src/aggregator/aggregator/handler/filter/shard.go +++ b/src/aggregator/aggregator/handler/filter/shard.go @@ -32,7 +32,7 @@ type shardSetFilter struct { // NewShardSetFilter creates a filter based on ShardSet. func NewShardSetFilter(shardSet sharding.ShardSet) producer.FilterFunc { f := shardSetFilter{shardSet: shardSet} - return f.Filter + return producer.NewFilterFunc(f.Filter, producer.ShardSetFilter) } func (f shardSetFilter) Filter(m producer.Message) bool { diff --git a/src/aggregator/aggregator/handler/filter/shard_test.go b/src/aggregator/aggregator/handler/filter/shard_test.go index d78aa6c188..e612ce0f1f 100644 --- a/src/aggregator/aggregator/handler/filter/shard_test.go +++ b/src/aggregator/aggregator/handler/filter/shard_test.go @@ -39,11 +39,11 @@ func TestShardSetFilter(t *testing.T) { mm := producer.NewMockMessage(ctrl) mm.EXPECT().Shard().Return(uint32(0)) - require.True(t, f(mm)) + require.True(t, f.Function(mm)) mm.EXPECT().Shard().Return(uint32(10)) - require.True(t, f(mm)) + require.True(t, f.Function(mm)) mm.EXPECT().Shard().Return(uint32(511)) - require.True(t, f(mm)) + require.True(t, f.Function(mm)) mm.EXPECT().Shard().Return(uint32(512)) - require.False(t, f(mm)) + require.False(t, f.Function(mm)) } diff --git a/src/aggregator/aggregator/handler/writer/protobuf.go b/src/aggregator/aggregator/handler/writer/protobuf.go index 11e767916a..d281edd792 100644 --- a/src/aggregator/aggregator/handler/writer/protobuf.go +++ b/src/aggregator/aggregator/handler/writer/protobuf.go @@ -169,7 +169,7 @@ type storagePolicyFilter struct { // NewStoragePolicyFilter creates a new storage policy based filter. func NewStoragePolicyFilter(acceptedStoragePolicies []policy.StoragePolicy) producer.FilterFunc { - return storagePolicyFilter{acceptedStoragePolicies}.Filter + return producer.NewFilterFunc(storagePolicyFilter{acceptedStoragePolicies}.Filter, producer.StoragePolicyFilter) } func (f storagePolicyFilter) Filter(m producer.Message) bool { diff --git a/src/aggregator/aggregator/handler/writer/protobuf_test.go b/src/aggregator/aggregator/handler/writer/protobuf_test.go index f0331d1f7c..3cece70227 100644 --- a/src/aggregator/aggregator/handler/writer/protobuf_test.go +++ b/src/aggregator/aggregator/handler/writer/protobuf_test.go @@ -92,9 +92,9 @@ func TestStoragePolicyFilter(t *testing.T) { f := NewStoragePolicyFilter([]policy.StoragePolicy{sp2}) - require.True(t, f(m2)) - require.False(t, f(newMessage(0, sp1, protobuf.Buffer{}))) - require.True(t, f(newMessage(0, sp2, protobuf.Buffer{}))) + require.True(t, f.Function(m2)) + require.False(t, f.Function(newMessage(0, sp1, protobuf.Buffer{}))) + require.True(t, f.Function(newMessage(0, sp2, protobuf.Buffer{}))) } func TestProtobufWriterWriteClosed(t *testing.T) { diff --git a/src/msg/generated/proto/msgpb/msg.pb.go b/src/msg/generated/proto/msgpb/msg.pb.go index 30395d7060..17fed520a0 100644 --- a/src/msg/generated/proto/msgpb/msg.pb.go +++ b/src/msg/generated/proto/msgpb/msg.pb.go @@ -1,26 +1,6 @@ // Code generated by protoc-gen-gogo. DO NOT EDIT. // source: github.com/m3db/m3/src/msg/generated/proto/msgpb/msg.proto -// Copyright (c) 2021 Uber Technologies, Inc. -// -// Permission is hereby granted, free of charge, to any person obtaining a copy -// of this software and associated documentation files (the "Software"), to deal -// in the Software without restriction, including without limitation the rights -// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell -// copies of the Software, and to permit persons to whom the Software is -// furnished to do so, subject to the following conditions: -// -// The above copyright notice and this permission notice shall be included in -// all copies or substantial portions of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN -// THE SOFTWARE. - /* Package msgpb is a generated protocol buffer package. diff --git a/src/msg/generated/proto/topicpb/topic.pb.go b/src/msg/generated/proto/topicpb/topic.pb.go index c749befdd4..622309fab7 100644 --- a/src/msg/generated/proto/topicpb/topic.pb.go +++ b/src/msg/generated/proto/topicpb/topic.pb.go @@ -1,26 +1,6 @@ // Code generated by protoc-gen-gogo. DO NOT EDIT. // source: github.com/m3db/m3/src/msg/generated/proto/topicpb/topic.proto -// Copyright (c) 2018 Uber Technologies, Inc. -// -// Permission is hereby granted, free of charge, to any person obtaining a copy -// of this software and associated documentation files (the "Software"), to deal -// in the Software without restriction, including without limitation the rights -// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell -// copies of the Software, and to permit persons to whom the Software is -// furnished to do so, subject to the following conditions: -// -// The above copyright notice and this permission notice shall be included in -// all copies or substantial portions of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN -// THE SOFTWARE. - /* Package topicpb is a generated protocol buffer package. @@ -30,6 +10,10 @@ It has these top-level messages: Topic ConsumerService + Filters + StoragePolicyFilter + PercentageFilter + ShardSetFilter ServiceID */ package topicpb @@ -111,6 +95,7 @@ type ConsumerService struct { ServiceId *ServiceID `protobuf:"bytes,1,opt,name=service_id,json=serviceId" json:"service_id,omitempty"` ConsumptionType ConsumptionType `protobuf:"varint,2,opt,name=consumption_type,json=consumptionType,proto3,enum=topicpb.ConsumptionType" json:"consumption_type,omitempty"` MessageTtlNanos int64 `protobuf:"varint,3,opt,name=message_ttl_nanos,json=messageTtlNanos,proto3" json:"message_ttl_nanos,omitempty"` + Filters *Filters `protobuf:"bytes,4,opt,name=filters" json:"filters,omitempty"` } func (m *ConsumerService) Reset() { *m = ConsumerService{} } @@ -139,6 +124,93 @@ func (m *ConsumerService) GetMessageTtlNanos() int64 { return 0 } +func (m *ConsumerService) GetFilters() *Filters { + if m != nil { + return m.Filters + } + return nil +} + +type Filters struct { + StoragePolicyFilter *StoragePolicyFilter `protobuf:"bytes,1,opt,name=storage_policy_filter,json=storagePolicyFilter" json:"storage_policy_filter,omitempty"` + PercentageFilter *PercentageFilter `protobuf:"bytes,2,opt,name=percentage_filter,json=percentageFilter" json:"percentage_filter,omitempty"` + ShardSetFilter *ShardSetFilter `protobuf:"bytes,3,opt,name=shard_set_filter,json=shardSetFilter" json:"shard_set_filter,omitempty"` +} + +func (m *Filters) Reset() { *m = Filters{} } +func (m *Filters) String() string { return proto.CompactTextString(m) } +func (*Filters) ProtoMessage() {} +func (*Filters) Descriptor() ([]byte, []int) { return fileDescriptorTopic, []int{2} } + +func (m *Filters) GetStoragePolicyFilter() *StoragePolicyFilter { + if m != nil { + return m.StoragePolicyFilter + } + return nil +} + +func (m *Filters) GetPercentageFilter() *PercentageFilter { + if m != nil { + return m.PercentageFilter + } + return nil +} + +func (m *Filters) GetShardSetFilter() *ShardSetFilter { + if m != nil { + return m.ShardSetFilter + } + return nil +} + +type StoragePolicyFilter struct { + StoragePolicies []string `protobuf:"bytes,1,rep,name=storage_policies,json=storagePolicies" json:"storage_policies,omitempty"` +} + +func (m *StoragePolicyFilter) Reset() { *m = StoragePolicyFilter{} } +func (m *StoragePolicyFilter) String() string { return proto.CompactTextString(m) } +func (*StoragePolicyFilter) ProtoMessage() {} +func (*StoragePolicyFilter) Descriptor() ([]byte, []int) { return fileDescriptorTopic, []int{3} } + +func (m *StoragePolicyFilter) GetStoragePolicies() []string { + if m != nil { + return m.StoragePolicies + } + return nil +} + +type PercentageFilter struct { + Percentage uint32 `protobuf:"varint,1,opt,name=percentage,proto3" json:"percentage,omitempty"` +} + +func (m *PercentageFilter) Reset() { *m = PercentageFilter{} } +func (m *PercentageFilter) String() string { return proto.CompactTextString(m) } +func (*PercentageFilter) ProtoMessage() {} +func (*PercentageFilter) Descriptor() ([]byte, []int) { return fileDescriptorTopic, []int{4} } + +func (m *PercentageFilter) GetPercentage() uint32 { + if m != nil { + return m.Percentage + } + return 0 +} + +type ShardSetFilter struct { + ShardSet string `protobuf:"bytes,1,opt,name=shard_set,json=shardSet,proto3" json:"shard_set,omitempty"` +} + +func (m *ShardSetFilter) Reset() { *m = ShardSetFilter{} } +func (m *ShardSetFilter) String() string { return proto.CompactTextString(m) } +func (*ShardSetFilter) ProtoMessage() {} +func (*ShardSetFilter) Descriptor() ([]byte, []int) { return fileDescriptorTopic, []int{5} } + +func (m *ShardSetFilter) GetShardSet() string { + if m != nil { + return m.ShardSet + } + return "" +} + type ServiceID struct { Name string `protobuf:"bytes,1,opt,name=name,proto3" json:"name,omitempty"` Environment string `protobuf:"bytes,2,opt,name=environment,proto3" json:"environment,omitempty"` @@ -148,7 +220,7 @@ type ServiceID struct { func (m *ServiceID) Reset() { *m = ServiceID{} } func (m *ServiceID) String() string { return proto.CompactTextString(m) } func (*ServiceID) ProtoMessage() {} -func (*ServiceID) Descriptor() ([]byte, []int) { return fileDescriptorTopic, []int{2} } +func (*ServiceID) Descriptor() ([]byte, []int) { return fileDescriptorTopic, []int{6} } func (m *ServiceID) GetName() string { if m != nil { @@ -174,6 +246,10 @@ func (m *ServiceID) GetZone() string { func init() { proto.RegisterType((*Topic)(nil), "topicpb.Topic") proto.RegisterType((*ConsumerService)(nil), "topicpb.ConsumerService") + proto.RegisterType((*Filters)(nil), "topicpb.Filters") + proto.RegisterType((*StoragePolicyFilter)(nil), "topicpb.StoragePolicyFilter") + proto.RegisterType((*PercentageFilter)(nil), "topicpb.PercentageFilter") + proto.RegisterType((*ShardSetFilter)(nil), "topicpb.ShardSetFilter") proto.RegisterType((*ServiceID)(nil), "topicpb.ServiceID") proto.RegisterEnum("topicpb.ConsumptionType", ConsumptionType_name, ConsumptionType_value) } @@ -253,6 +329,144 @@ func (m *ConsumerService) MarshalTo(dAtA []byte) (int, error) { i++ i = encodeVarintTopic(dAtA, i, uint64(m.MessageTtlNanos)) } + if m.Filters != nil { + dAtA[i] = 0x22 + i++ + i = encodeVarintTopic(dAtA, i, uint64(m.Filters.Size())) + n2, err := m.Filters.MarshalTo(dAtA[i:]) + if err != nil { + return 0, err + } + i += n2 + } + return i, nil +} + +func (m *Filters) Marshal() (dAtA []byte, err error) { + size := m.Size() + dAtA = make([]byte, size) + n, err := m.MarshalTo(dAtA) + if err != nil { + return nil, err + } + return dAtA[:n], nil +} + +func (m *Filters) MarshalTo(dAtA []byte) (int, error) { + var i int + _ = i + var l int + _ = l + if m.StoragePolicyFilter != nil { + dAtA[i] = 0xa + i++ + i = encodeVarintTopic(dAtA, i, uint64(m.StoragePolicyFilter.Size())) + n3, err := m.StoragePolicyFilter.MarshalTo(dAtA[i:]) + if err != nil { + return 0, err + } + i += n3 + } + if m.PercentageFilter != nil { + dAtA[i] = 0x12 + i++ + i = encodeVarintTopic(dAtA, i, uint64(m.PercentageFilter.Size())) + n4, err := m.PercentageFilter.MarshalTo(dAtA[i:]) + if err != nil { + return 0, err + } + i += n4 + } + if m.ShardSetFilter != nil { + dAtA[i] = 0x1a + i++ + i = encodeVarintTopic(dAtA, i, uint64(m.ShardSetFilter.Size())) + n5, err := m.ShardSetFilter.MarshalTo(dAtA[i:]) + if err != nil { + return 0, err + } + i += n5 + } + return i, nil +} + +func (m *StoragePolicyFilter) Marshal() (dAtA []byte, err error) { + size := m.Size() + dAtA = make([]byte, size) + n, err := m.MarshalTo(dAtA) + if err != nil { + return nil, err + } + return dAtA[:n], nil +} + +func (m *StoragePolicyFilter) MarshalTo(dAtA []byte) (int, error) { + var i int + _ = i + var l int + _ = l + if len(m.StoragePolicies) > 0 { + for _, s := range m.StoragePolicies { + dAtA[i] = 0xa + i++ + l = len(s) + for l >= 1<<7 { + dAtA[i] = uint8(uint64(l)&0x7f | 0x80) + l >>= 7 + i++ + } + dAtA[i] = uint8(l) + i++ + i += copy(dAtA[i:], s) + } + } + return i, nil +} + +func (m *PercentageFilter) Marshal() (dAtA []byte, err error) { + size := m.Size() + dAtA = make([]byte, size) + n, err := m.MarshalTo(dAtA) + if err != nil { + return nil, err + } + return dAtA[:n], nil +} + +func (m *PercentageFilter) MarshalTo(dAtA []byte) (int, error) { + var i int + _ = i + var l int + _ = l + if m.Percentage != 0 { + dAtA[i] = 0x8 + i++ + i = encodeVarintTopic(dAtA, i, uint64(m.Percentage)) + } + return i, nil +} + +func (m *ShardSetFilter) Marshal() (dAtA []byte, err error) { + size := m.Size() + dAtA = make([]byte, size) + n, err := m.MarshalTo(dAtA) + if err != nil { + return nil, err + } + return dAtA[:n], nil +} + +func (m *ShardSetFilter) MarshalTo(dAtA []byte) (int, error) { + var i int + _ = i + var l int + _ = l + if len(m.ShardSet) > 0 { + dAtA[i] = 0xa + i++ + i = encodeVarintTopic(dAtA, i, uint64(len(m.ShardSet))) + i += copy(dAtA[i:], m.ShardSet) + } return i, nil } @@ -333,6 +547,59 @@ func (m *ConsumerService) Size() (n int) { if m.MessageTtlNanos != 0 { n += 1 + sovTopic(uint64(m.MessageTtlNanos)) } + if m.Filters != nil { + l = m.Filters.Size() + n += 1 + l + sovTopic(uint64(l)) + } + return n +} + +func (m *Filters) Size() (n int) { + var l int + _ = l + if m.StoragePolicyFilter != nil { + l = m.StoragePolicyFilter.Size() + n += 1 + l + sovTopic(uint64(l)) + } + if m.PercentageFilter != nil { + l = m.PercentageFilter.Size() + n += 1 + l + sovTopic(uint64(l)) + } + if m.ShardSetFilter != nil { + l = m.ShardSetFilter.Size() + n += 1 + l + sovTopic(uint64(l)) + } + return n +} + +func (m *StoragePolicyFilter) Size() (n int) { + var l int + _ = l + if len(m.StoragePolicies) > 0 { + for _, s := range m.StoragePolicies { + l = len(s) + n += 1 + l + sovTopic(uint64(l)) + } + } + return n +} + +func (m *PercentageFilter) Size() (n int) { + var l int + _ = l + if m.Percentage != 0 { + n += 1 + sovTopic(uint64(m.Percentage)) + } + return n +} + +func (m *ShardSetFilter) Size() (n int) { + var l int + _ = l + l = len(m.ShardSet) + if l > 0 { + n += 1 + l + sovTopic(uint64(l)) + } return n } @@ -596,6 +863,415 @@ func (m *ConsumerService) Unmarshal(dAtA []byte) error { break } } + case 4: + if wireType != 2 { + return fmt.Errorf("proto: wrong wireType = %d for field Filters", wireType) + } + var msglen int + for shift := uint(0); ; shift += 7 { + if shift >= 64 { + return ErrIntOverflowTopic + } + if iNdEx >= l { + return io.ErrUnexpectedEOF + } + b := dAtA[iNdEx] + iNdEx++ + msglen |= (int(b) & 0x7F) << shift + if b < 0x80 { + break + } + } + if msglen < 0 { + return ErrInvalidLengthTopic + } + postIndex := iNdEx + msglen + if postIndex > l { + return io.ErrUnexpectedEOF + } + if m.Filters == nil { + m.Filters = &Filters{} + } + if err := m.Filters.Unmarshal(dAtA[iNdEx:postIndex]); err != nil { + return err + } + iNdEx = postIndex + default: + iNdEx = preIndex + skippy, err := skipTopic(dAtA[iNdEx:]) + if err != nil { + return err + } + if skippy < 0 { + return ErrInvalidLengthTopic + } + if (iNdEx + skippy) > l { + return io.ErrUnexpectedEOF + } + iNdEx += skippy + } + } + + if iNdEx > l { + return io.ErrUnexpectedEOF + } + return nil +} +func (m *Filters) Unmarshal(dAtA []byte) error { + l := len(dAtA) + iNdEx := 0 + for iNdEx < l { + preIndex := iNdEx + var wire uint64 + for shift := uint(0); ; shift += 7 { + if shift >= 64 { + return ErrIntOverflowTopic + } + if iNdEx >= l { + return io.ErrUnexpectedEOF + } + b := dAtA[iNdEx] + iNdEx++ + wire |= (uint64(b) & 0x7F) << shift + if b < 0x80 { + break + } + } + fieldNum := int32(wire >> 3) + wireType := int(wire & 0x7) + if wireType == 4 { + return fmt.Errorf("proto: Filters: wiretype end group for non-group") + } + if fieldNum <= 0 { + return fmt.Errorf("proto: Filters: illegal tag %d (wire type %d)", fieldNum, wire) + } + switch fieldNum { + case 1: + if wireType != 2 { + return fmt.Errorf("proto: wrong wireType = %d for field StoragePolicyFilter", wireType) + } + var msglen int + for shift := uint(0); ; shift += 7 { + if shift >= 64 { + return ErrIntOverflowTopic + } + if iNdEx >= l { + return io.ErrUnexpectedEOF + } + b := dAtA[iNdEx] + iNdEx++ + msglen |= (int(b) & 0x7F) << shift + if b < 0x80 { + break + } + } + if msglen < 0 { + return ErrInvalidLengthTopic + } + postIndex := iNdEx + msglen + if postIndex > l { + return io.ErrUnexpectedEOF + } + if m.StoragePolicyFilter == nil { + m.StoragePolicyFilter = &StoragePolicyFilter{} + } + if err := m.StoragePolicyFilter.Unmarshal(dAtA[iNdEx:postIndex]); err != nil { + return err + } + iNdEx = postIndex + case 2: + if wireType != 2 { + return fmt.Errorf("proto: wrong wireType = %d for field PercentageFilter", wireType) + } + var msglen int + for shift := uint(0); ; shift += 7 { + if shift >= 64 { + return ErrIntOverflowTopic + } + if iNdEx >= l { + return io.ErrUnexpectedEOF + } + b := dAtA[iNdEx] + iNdEx++ + msglen |= (int(b) & 0x7F) << shift + if b < 0x80 { + break + } + } + if msglen < 0 { + return ErrInvalidLengthTopic + } + postIndex := iNdEx + msglen + if postIndex > l { + return io.ErrUnexpectedEOF + } + if m.PercentageFilter == nil { + m.PercentageFilter = &PercentageFilter{} + } + if err := m.PercentageFilter.Unmarshal(dAtA[iNdEx:postIndex]); err != nil { + return err + } + iNdEx = postIndex + case 3: + if wireType != 2 { + return fmt.Errorf("proto: wrong wireType = %d for field ShardSetFilter", wireType) + } + var msglen int + for shift := uint(0); ; shift += 7 { + if shift >= 64 { + return ErrIntOverflowTopic + } + if iNdEx >= l { + return io.ErrUnexpectedEOF + } + b := dAtA[iNdEx] + iNdEx++ + msglen |= (int(b) & 0x7F) << shift + if b < 0x80 { + break + } + } + if msglen < 0 { + return ErrInvalidLengthTopic + } + postIndex := iNdEx + msglen + if postIndex > l { + return io.ErrUnexpectedEOF + } + if m.ShardSetFilter == nil { + m.ShardSetFilter = &ShardSetFilter{} + } + if err := m.ShardSetFilter.Unmarshal(dAtA[iNdEx:postIndex]); err != nil { + return err + } + iNdEx = postIndex + default: + iNdEx = preIndex + skippy, err := skipTopic(dAtA[iNdEx:]) + if err != nil { + return err + } + if skippy < 0 { + return ErrInvalidLengthTopic + } + if (iNdEx + skippy) > l { + return io.ErrUnexpectedEOF + } + iNdEx += skippy + } + } + + if iNdEx > l { + return io.ErrUnexpectedEOF + } + return nil +} +func (m *StoragePolicyFilter) Unmarshal(dAtA []byte) error { + l := len(dAtA) + iNdEx := 0 + for iNdEx < l { + preIndex := iNdEx + var wire uint64 + for shift := uint(0); ; shift += 7 { + if shift >= 64 { + return ErrIntOverflowTopic + } + if iNdEx >= l { + return io.ErrUnexpectedEOF + } + b := dAtA[iNdEx] + iNdEx++ + wire |= (uint64(b) & 0x7F) << shift + if b < 0x80 { + break + } + } + fieldNum := int32(wire >> 3) + wireType := int(wire & 0x7) + if wireType == 4 { + return fmt.Errorf("proto: StoragePolicyFilter: wiretype end group for non-group") + } + if fieldNum <= 0 { + return fmt.Errorf("proto: StoragePolicyFilter: illegal tag %d (wire type %d)", fieldNum, wire) + } + switch fieldNum { + case 1: + if wireType != 2 { + return fmt.Errorf("proto: wrong wireType = %d for field StoragePolicies", wireType) + } + var stringLen uint64 + for shift := uint(0); ; shift += 7 { + if shift >= 64 { + return ErrIntOverflowTopic + } + if iNdEx >= l { + return io.ErrUnexpectedEOF + } + b := dAtA[iNdEx] + iNdEx++ + stringLen |= (uint64(b) & 0x7F) << shift + if b < 0x80 { + break + } + } + intStringLen := int(stringLen) + if intStringLen < 0 { + return ErrInvalidLengthTopic + } + postIndex := iNdEx + intStringLen + if postIndex > l { + return io.ErrUnexpectedEOF + } + m.StoragePolicies = append(m.StoragePolicies, string(dAtA[iNdEx:postIndex])) + iNdEx = postIndex + default: + iNdEx = preIndex + skippy, err := skipTopic(dAtA[iNdEx:]) + if err != nil { + return err + } + if skippy < 0 { + return ErrInvalidLengthTopic + } + if (iNdEx + skippy) > l { + return io.ErrUnexpectedEOF + } + iNdEx += skippy + } + } + + if iNdEx > l { + return io.ErrUnexpectedEOF + } + return nil +} +func (m *PercentageFilter) Unmarshal(dAtA []byte) error { + l := len(dAtA) + iNdEx := 0 + for iNdEx < l { + preIndex := iNdEx + var wire uint64 + for shift := uint(0); ; shift += 7 { + if shift >= 64 { + return ErrIntOverflowTopic + } + if iNdEx >= l { + return io.ErrUnexpectedEOF + } + b := dAtA[iNdEx] + iNdEx++ + wire |= (uint64(b) & 0x7F) << shift + if b < 0x80 { + break + } + } + fieldNum := int32(wire >> 3) + wireType := int(wire & 0x7) + if wireType == 4 { + return fmt.Errorf("proto: PercentageFilter: wiretype end group for non-group") + } + if fieldNum <= 0 { + return fmt.Errorf("proto: PercentageFilter: illegal tag %d (wire type %d)", fieldNum, wire) + } + switch fieldNum { + case 1: + if wireType != 0 { + return fmt.Errorf("proto: wrong wireType = %d for field Percentage", wireType) + } + m.Percentage = 0 + for shift := uint(0); ; shift += 7 { + if shift >= 64 { + return ErrIntOverflowTopic + } + if iNdEx >= l { + return io.ErrUnexpectedEOF + } + b := dAtA[iNdEx] + iNdEx++ + m.Percentage |= (uint32(b) & 0x7F) << shift + if b < 0x80 { + break + } + } + default: + iNdEx = preIndex + skippy, err := skipTopic(dAtA[iNdEx:]) + if err != nil { + return err + } + if skippy < 0 { + return ErrInvalidLengthTopic + } + if (iNdEx + skippy) > l { + return io.ErrUnexpectedEOF + } + iNdEx += skippy + } + } + + if iNdEx > l { + return io.ErrUnexpectedEOF + } + return nil +} +func (m *ShardSetFilter) Unmarshal(dAtA []byte) error { + l := len(dAtA) + iNdEx := 0 + for iNdEx < l { + preIndex := iNdEx + var wire uint64 + for shift := uint(0); ; shift += 7 { + if shift >= 64 { + return ErrIntOverflowTopic + } + if iNdEx >= l { + return io.ErrUnexpectedEOF + } + b := dAtA[iNdEx] + iNdEx++ + wire |= (uint64(b) & 0x7F) << shift + if b < 0x80 { + break + } + } + fieldNum := int32(wire >> 3) + wireType := int(wire & 0x7) + if wireType == 4 { + return fmt.Errorf("proto: ShardSetFilter: wiretype end group for non-group") + } + if fieldNum <= 0 { + return fmt.Errorf("proto: ShardSetFilter: illegal tag %d (wire type %d)", fieldNum, wire) + } + switch fieldNum { + case 1: + if wireType != 2 { + return fmt.Errorf("proto: wrong wireType = %d for field ShardSet", wireType) + } + var stringLen uint64 + for shift := uint(0); ; shift += 7 { + if shift >= 64 { + return ErrIntOverflowTopic + } + if iNdEx >= l { + return io.ErrUnexpectedEOF + } + b := dAtA[iNdEx] + iNdEx++ + stringLen |= (uint64(b) & 0x7F) << shift + if b < 0x80 { + break + } + } + intStringLen := int(stringLen) + if intStringLen < 0 { + return ErrInvalidLengthTopic + } + postIndex := iNdEx + intStringLen + if postIndex > l { + return io.ErrUnexpectedEOF + } + m.ShardSet = string(dAtA[iNdEx:postIndex]) + iNdEx = postIndex default: iNdEx = preIndex skippy, err := skipTopic(dAtA[iNdEx:]) @@ -864,30 +1540,40 @@ func init() { } var fileDescriptorTopic = []byte{ - // 385 bytes of a gzipped FileDescriptorProto - 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x6c, 0x91, 0x4f, 0x8e, 0xd3, 0x30, - 0x14, 0x87, 0xc7, 0x53, 0x98, 0x51, 0x5e, 0x44, 0x9b, 0xf1, 0x2a, 0xab, 0x28, 0xea, 0x2a, 0x9a, - 0x45, 0x22, 0xa6, 0x3b, 0x16, 0x48, 0x43, 0x1b, 0x89, 0x0a, 0x94, 0x41, 0x6e, 0x46, 0x2c, 0xa3, - 0xfc, 0xf1, 0x64, 0x22, 0xd5, 0x76, 0x64, 0xbb, 0x95, 0xca, 0x19, 0x58, 0x70, 0x19, 0xee, 0xc0, - 0x92, 0x23, 0xa0, 0x72, 0x11, 0x14, 0xd7, 0x14, 0x0a, 0xb3, 0xca, 0xd3, 0x97, 0x9f, 0xdf, 0xfb, - 0xfc, 0x0c, 0xaf, 0xdb, 0x4e, 0x3f, 0x6e, 0xaa, 0xb8, 0x16, 0x2c, 0x61, 0xb3, 0xa6, 0x4a, 0xd8, - 0x2c, 0x51, 0xb2, 0x4e, 0x98, 0x6a, 0x93, 0x96, 0x72, 0x2a, 0x4b, 0x4d, 0x9b, 0xa4, 0x97, 0x42, - 0x8b, 0x44, 0x8b, 0xbe, 0xab, 0xfb, 0xea, 0xf0, 0x8d, 0x0d, 0xc3, 0x97, 0x16, 0x4e, 0x3f, 0x23, - 0x78, 0x9e, 0x0f, 0x35, 0xc6, 0xf0, 0x8c, 0x97, 0x8c, 0xfa, 0x28, 0x44, 0x91, 0x43, 0x4c, 0x8d, - 0x23, 0xf0, 0xf8, 0x86, 0x55, 0x54, 0x16, 0xe2, 0xa1, 0x50, 0x8f, 0xa5, 0x6c, 0x94, 0x7f, 0x1e, - 0xa2, 0xe8, 0x05, 0x19, 0x1f, 0xf8, 0xdd, 0xc3, 0xca, 0x50, 0x9c, 0xc2, 0x55, 0x2d, 0xb8, 0xda, - 0x30, 0x2a, 0x0b, 0x45, 0xe5, 0xb6, 0xab, 0xa9, 0xf2, 0x47, 0xe1, 0x28, 0x72, 0x6f, 0xfc, 0xd8, - 0x0e, 0x8b, 0xe7, 0x36, 0xb1, 0x3a, 0x04, 0x88, 0x57, 0x9f, 0x02, 0x35, 0xfd, 0x8a, 0x60, 0xf2, - 0x4f, 0x0a, 0xbf, 0x04, 0xb0, 0x1d, 0x8b, 0xae, 0x31, 0x7a, 0xee, 0x0d, 0x3e, 0xf6, 0xb4, 0xa9, - 0xe5, 0x82, 0x38, 0x36, 0xb5, 0x6c, 0xf0, 0x1c, 0x6c, 0xeb, 0x5e, 0x77, 0x82, 0x17, 0x7a, 0xd7, - 0x53, 0xe3, 0x3d, 0xfe, 0x4f, 0xc6, 0x04, 0xf2, 0x5d, 0x4f, 0xc9, 0xa4, 0x3e, 0x05, 0xf8, 0x1a, - 0xae, 0x18, 0x55, 0xaa, 0x6c, 0x69, 0xa1, 0xf5, 0xba, 0xe0, 0x25, 0x17, 0xc3, 0x95, 0x50, 0x34, - 0x22, 0x13, 0xfb, 0x23, 0xd7, 0xeb, 0x6c, 0xc0, 0xd3, 0x7b, 0x70, 0x8e, 0x22, 0x4f, 0x6e, 0x32, - 0x04, 0x97, 0xf2, 0x6d, 0x27, 0x05, 0x67, 0x94, 0x6b, 0x23, 0xe3, 0x90, 0xbf, 0xd1, 0x70, 0xea, - 0x93, 0xe0, 0xd4, 0x4c, 0x70, 0x88, 0xa9, 0xaf, 0x5f, 0xfd, 0xde, 0xc6, 0x1f, 0x2b, 0x17, 0x2e, - 0xef, 0xb3, 0x77, 0xd9, 0xdd, 0xc7, 0xcc, 0x3b, 0xc3, 0x00, 0x17, 0xab, 0xb7, 0xb7, 0x24, 0x5d, - 0x78, 0x08, 0x8f, 0x01, 0x48, 0xfa, 0xe1, 0xfd, 0x72, 0x7e, 0x9b, 0xa7, 0x0b, 0xef, 0xfc, 0x8d, - 0xf7, 0x6d, 0x1f, 0xa0, 0xef, 0xfb, 0x00, 0xfd, 0xd8, 0x07, 0xe8, 0xcb, 0xcf, 0xe0, 0xac, 0xba, - 0x30, 0x6f, 0x3f, 0xfb, 0x15, 0x00, 0x00, 0xff, 0xff, 0xd4, 0xd7, 0xf0, 0x71, 0x3d, 0x02, 0x00, - 0x00, + // 548 bytes of a gzipped FileDescriptorProto + 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x6c, 0x93, 0xdf, 0x6e, 0xda, 0x30, + 0x14, 0xc6, 0xeb, 0xd2, 0x95, 0xe5, 0xa0, 0x42, 0xea, 0x6a, 0x5a, 0xa6, 0x4d, 0x08, 0xe5, 0x2a, + 0x43, 0x1a, 0xd1, 0xe0, 0x6e, 0x17, 0xd3, 0x18, 0x50, 0x0d, 0x6d, 0xa2, 0xc8, 0x50, 0xed, 0x32, + 0x82, 0x60, 0x68, 0x24, 0x62, 0x47, 0xb6, 0xa9, 0xc4, 0x9e, 0x61, 0x17, 0x7b, 0xac, 0x5d, 0xee, + 0x11, 0x36, 0x26, 0xed, 0x39, 0xa6, 0x38, 0x26, 0xfc, 0x69, 0xaf, 0xe2, 0xfc, 0xf2, 0xf9, 0xf3, + 0x77, 0x4e, 0x8e, 0xe1, 0xfd, 0x22, 0x52, 0x77, 0xab, 0x69, 0x23, 0xe4, 0xb1, 0x1f, 0xb7, 0x66, + 0x53, 0x3f, 0x6e, 0xf9, 0x52, 0x84, 0x7e, 0x2c, 0x17, 0xfe, 0x82, 0x32, 0x2a, 0x26, 0x8a, 0xce, + 0xfc, 0x44, 0x70, 0xc5, 0x7d, 0xc5, 0x93, 0x28, 0x4c, 0xa6, 0xd9, 0xb3, 0xa1, 0x19, 0x2e, 0x1a, + 0xe8, 0x7e, 0x47, 0xf0, 0x64, 0x9c, 0xae, 0x31, 0x86, 0x33, 0x36, 0x89, 0xa9, 0x83, 0x6a, 0xc8, + 0xb3, 0x88, 0x5e, 0x63, 0x0f, 0x6c, 0xb6, 0x8a, 0xa7, 0x54, 0x04, 0x7c, 0x1e, 0xc8, 0xbb, 0x89, + 0x98, 0x49, 0xe7, 0xb4, 0x86, 0xbc, 0x0b, 0x52, 0xce, 0xf8, 0xcd, 0x7c, 0xa4, 0x29, 0xee, 0xc1, + 0x65, 0xc8, 0x99, 0x5c, 0xc5, 0x54, 0x04, 0x92, 0x8a, 0xfb, 0x28, 0xa4, 0xd2, 0x29, 0xd4, 0x0a, + 0x5e, 0xa9, 0xe9, 0x34, 0xcc, 0x61, 0x8d, 0x8e, 0x51, 0x8c, 0x32, 0x01, 0xb1, 0xc3, 0x43, 0x20, + 0xdd, 0x3f, 0x08, 0x2a, 0x47, 0x2a, 0xfc, 0x16, 0xc0, 0x38, 0x06, 0xd1, 0x4c, 0xc7, 0x2b, 0x35, + 0x71, 0xee, 0x69, 0x54, 0xfd, 0x2e, 0xb1, 0x8c, 0xaa, 0x3f, 0xc3, 0x1d, 0x30, 0xd6, 0x89, 0x8a, + 0x38, 0x0b, 0xd4, 0x3a, 0xa1, 0x3a, 0x77, 0xf9, 0x41, 0x18, 0x2d, 0x18, 0xaf, 0x13, 0x4a, 0x2a, + 0xe1, 0x21, 0xc0, 0x75, 0xb8, 0x8c, 0xa9, 0x94, 0x93, 0x05, 0x0d, 0x94, 0x5a, 0x06, 0x6c, 0xc2, + 0x78, 0x5a, 0x12, 0xf2, 0x0a, 0xa4, 0x62, 0x3e, 0x8c, 0xd5, 0x72, 0x90, 0x62, 0x5c, 0x87, 0xe2, + 0x3c, 0x5a, 0x2a, 0x2a, 0xa4, 0x73, 0xa6, 0x03, 0xda, 0xf9, 0x39, 0xd7, 0x19, 0x27, 0x5b, 0x81, + 0xfb, 0x0f, 0x41, 0xd1, 0x40, 0x3c, 0x84, 0x67, 0x52, 0x71, 0x91, 0x9e, 0x91, 0xf0, 0x65, 0x14, + 0xae, 0x83, 0x4c, 0x65, 0xca, 0x7c, 0xb5, 0x2b, 0x33, 0x53, 0x0d, 0xb5, 0x28, 0xdb, 0x4d, 0xae, + 0xe4, 0x43, 0x88, 0xaf, 0xe1, 0x32, 0xa1, 0x22, 0xa4, 0x4c, 0xa5, 0xa6, 0xc6, 0xed, 0x54, 0xbb, + 0xbd, 0xc8, 0xdd, 0x86, 0xb9, 0xc2, 0x58, 0xd9, 0xc9, 0x11, 0xc1, 0x6d, 0xb0, 0xf5, 0x0f, 0x0f, + 0x24, 0x55, 0x5b, 0x9b, 0x82, 0xb6, 0x79, 0xbe, 0x0b, 0x95, 0x0a, 0x46, 0x54, 0x19, 0x93, 0xb2, + 0x3c, 0x78, 0x77, 0x3f, 0xc0, 0xd5, 0x23, 0xb1, 0xf1, 0x6b, 0xb0, 0x0f, 0x6a, 0x8e, 0xa8, 0x74, + 0x50, 0xad, 0xe0, 0x59, 0xa4, 0xb2, 0x5f, 0x50, 0x44, 0xa5, 0xdb, 0x04, 0xfb, 0x38, 0x2a, 0xae, + 0x02, 0xec, 0xc2, 0xea, 0x3e, 0x5d, 0x90, 0x3d, 0xe2, 0xbe, 0x81, 0xf2, 0x61, 0x2e, 0xfc, 0x12, + 0xac, 0xbc, 0x14, 0x33, 0xde, 0x4f, 0xb7, 0x51, 0xdd, 0x5b, 0xb0, 0xf2, 0x11, 0x7a, 0xf4, 0x0e, + 0xd4, 0xa0, 0x44, 0xd9, 0x7d, 0x24, 0x38, 0x8b, 0x29, 0x53, 0xba, 0x95, 0x16, 0xd9, 0x47, 0xe9, + 0xae, 0x6f, 0x9c, 0x51, 0xdd, 0x1e, 0x8b, 0xe8, 0x75, 0xfd, 0xdd, 0x76, 0x8e, 0x77, 0xf3, 0x54, + 0x82, 0xe2, 0xed, 0xe0, 0xf3, 0xe0, 0xe6, 0xeb, 0xc0, 0x3e, 0xc1, 0x00, 0xe7, 0xa3, 0x4f, 0x6d, + 0xd2, 0xeb, 0xda, 0x08, 0x97, 0x01, 0x48, 0x6f, 0xf8, 0xa5, 0xdf, 0x69, 0x8f, 0x7b, 0x5d, 0xfb, + 0xf4, 0xa3, 0xfd, 0x73, 0x53, 0x45, 0xbf, 0x36, 0x55, 0xf4, 0x7b, 0x53, 0x45, 0x3f, 0xfe, 0x56, + 0x4f, 0xa6, 0xe7, 0xfa, 0xd6, 0xb6, 0xfe, 0x07, 0x00, 0x00, 0xff, 0xff, 0xd5, 0x26, 0x64, 0x8e, + 0xf7, 0x03, 0x00, 0x00, } diff --git a/src/msg/generated/proto/topicpb/topic.proto b/src/msg/generated/proto/topicpb/topic.proto index 193f146a04..054516f777 100644 --- a/src/msg/generated/proto/topicpb/topic.proto +++ b/src/msg/generated/proto/topicpb/topic.proto @@ -11,6 +11,25 @@ message ConsumerService { ServiceID service_id = 1; ConsumptionType consumption_type = 2; int64 message_ttl_nanos = 3; + Filters filters = 4; +} + +message Filters { + StoragePolicyFilter storage_policy_filter = 1; + PercentageFilter percentage_filter = 2; + ShardSetFilter shard_set_filter = 3; +} + +message StoragePolicyFilter { + repeated string storage_policies = 1; +} + +message PercentageFilter { + uint32 percentage = 1; +} + +message ShardSetFilter { + string shard_set = 1; } message ServiceID { diff --git a/src/msg/producer/ref_counted.go b/src/msg/producer/ref_counted.go index 444d47aee1..e5602231f0 100644 --- a/src/msg/producer/ref_counted.go +++ b/src/msg/producer/ref_counted.go @@ -56,7 +56,7 @@ func (rm *RefCountedMessage) Accept(fn []FilterFunc) bool { return false } for _, f := range fn { - if !f(rm.Message) { + if !f.Function(rm.Message) { return false } } diff --git a/src/msg/producer/ref_counted_test.go b/src/msg/producer/ref_counted_test.go index 7d51584f34..6e5a798a8b 100644 --- a/src/msg/producer/ref_counted_test.go +++ b/src/msg/producer/ref_counted_test.go @@ -123,15 +123,18 @@ func TestRefCountedMessageFilter(t *testing.T) { defer ctrl.Finish() var called int - filter := func(m Message) bool { - called++ - return m.Shard() == 0 - } - - sizeFilter := func(m Message) bool { + filter := NewFilterFunc( + func(m Message) bool { + called++ + return m.Shard() == 0 + }, + UnspecifiedFilter, + ) + + sizeFilter := NewFilterFunc(func(m Message) bool { called++ return m.Size() == 0 - } + }, UnspecifiedFilter) mm := NewMockMessage(ctrl) mm.EXPECT().Size().Return(0) diff --git a/src/msg/producer/types.go b/src/msg/producer/types.go index a69e0ecf53..4b886a4779 100644 --- a/src/msg/producer/types.go +++ b/src/msg/producer/types.go @@ -85,8 +85,28 @@ type Producer interface { Close(ct CloseType) } +type FilterFuncType uint8 + +const ( + ShardSetFilter FilterFuncType = iota + StoragePolicyFilter + PercentageFilter + AcceptAllFilter + UnspecifiedFilter +) + // FilterFunc can filter message. -type FilterFunc func(m Message) bool +type FilterFunc struct { + Function func(m Message) bool + FilterType FilterFuncType +} + +func NewFilterFunc(function func(m Message) bool, filterType FilterFuncType) FilterFunc { + return FilterFunc{ + Function: function, + FilterType: filterType, + } +} // Options configs a producer. type Options interface { diff --git a/src/msg/producer/writer/consumer_service_writer.go b/src/msg/producer/writer/consumer_service_writer.go index 70d1894884..2848ed36ee 100644 --- a/src/msg/producer/writer/consumer_service_writer.go +++ b/src/msg/producer/writer/consumer_service_writer.go @@ -37,8 +37,11 @@ import ( var ( acceptAllFilter = producer.FilterFunc( - func(m producer.Message) bool { - return true + producer.FilterFunc{ + Function: func(m producer.Message) bool { + return true + }, + FilterType: producer.AcceptAllFilter, }, ) @@ -115,6 +118,7 @@ type consumerServiceWriterImpl struct { wg sync.WaitGroup m consumerServiceWriterMetrics cm consumerWriterMetrics + shardSet string processFn watch.ProcessFn } @@ -147,6 +151,7 @@ func newConsumerServiceWriter( doneCh: make(chan struct{}), m: newConsumerServiceWriterMetrics(opts.InstrumentOptions().MetricsScope()), cm: newConsumerWriterMetrics(opts.InstrumentOptions().MetricsScope()), + shardSet: cs.ShardSet(), } w.processFn = w.process return w, nil @@ -326,6 +331,14 @@ func (w *consumerServiceWriterImpl) SetMessageTTLNanos(value int64) { } } +func (w *consumerServiceWriterImpl) GetShardSet() string { + return w.shardSet +} + +func (w *consumerServiceWriterImpl) SetShardSet(value string) { + w.shardSet = value +} + func (w *consumerServiceWriterImpl) RegisterFilter(filter producer.FilterFunc) { w.Lock() w.dataFilters = append(w.dataFilters, filter) diff --git a/src/msg/producer/writer/consumer_service_writer_mock.go b/src/msg/producer/writer/consumer_service_writer_mock.go index 4472d92b36..49c8d5218f 100644 --- a/src/msg/producer/writer/consumer_service_writer_mock.go +++ b/src/msg/producer/writer/consumer_service_writer_mock.go @@ -105,6 +105,18 @@ func (mr *MockconsumerServiceWriterMockRecorder) SetMessageTTLNanos(value interf return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetMessageTTLNanos", reflect.TypeOf((*MockconsumerServiceWriter)(nil).SetMessageTTLNanos), value) } +// SetShardSet mocks base method. +func (m *MockconsumerServiceWriter) SetShardSet(value string) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "SetShardSet", value) +} + +// SetShardSet indicates an expected call of SetShardSet. +func (mr *MockconsumerServiceWriterMockRecorder) SetShardSet(value interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetMessageTTLNanos", reflect.TypeOf((*MockconsumerServiceWriter)(nil).SetShardSet), value) +} + // UnregisterFilters mocks base method. func (m *MockconsumerServiceWriter) UnregisterFilters() { m.ctrl.T.Helper() diff --git a/src/msg/producer/writer/consumer_service_writer_test.go b/src/msg/producer/writer/consumer_service_writer_test.go index b11f1611cf..909b1c5c9f 100644 --- a/src/msg/producer/writer/consumer_service_writer_test.go +++ b/src/msg/producer/writer/consumer_service_writer_test.go @@ -521,7 +521,7 @@ func TestConsumerServiceWriterFilter(t *testing.T) { sw1.EXPECT().Write(gomock.Any()) csw.Write(producer.NewRefCountedMessage(mm1, nil)) - csw.RegisterFilter(func(m producer.Message) bool { return m.Shard() == uint32(0) }) + csw.RegisterFilter(producer.NewFilterFunc(func(m producer.Message) bool { return m.Shard() == uint32(0) }, producer.UnspecifiedFilter)) // Write is not expected due to mm1 shard != 0 csw.Write(producer.NewRefCountedMessage(mm1, nil)) @@ -529,7 +529,7 @@ func TestConsumerServiceWriterFilter(t *testing.T) { // Write is expected due to mm0 shard == 0 csw.Write(producer.NewRefCountedMessage(mm0, nil)) - csw.RegisterFilter(func(m producer.Message) bool { return m.Size() == 3 }) + csw.RegisterFilter(producer.NewFilterFunc(func(m producer.Message) bool { return m.Size() == 3 }, producer.UnspecifiedFilter)) sw0.EXPECT().Write(gomock.Any()) // Write is expected because to mm0 shard == 0 and mm0 size == 3 csw.Write(producer.NewRefCountedMessage(mm0, nil)) diff --git a/src/msg/producer/writer/writer.go b/src/msg/producer/writer/writer.go index 080627f707..0580c5d88f 100644 --- a/src/msg/producer/writer/writer.go +++ b/src/msg/producer/writer/writer.go @@ -28,6 +28,8 @@ import ( "github.com/uber-go/tally" "go.uber.org/zap" + "github.com/m3db/m3/src/aggregator/aggregator/handler/filter" + "github.com/m3db/m3/src/aggregator/sharding" "github.com/m3db/m3/src/cluster/services" "github.com/m3db/m3/src/msg/producer" "github.com/m3db/m3/src/msg/topic" @@ -153,7 +155,7 @@ func (w *writer) NumShards() uint32 { return n } -func (w *writer) process(update interface{}) error { +func (w *writer) process(update interface{}) error { t := update.(topic.Topic) if err := t.Validate(); err != nil { return err @@ -171,6 +173,7 @@ func (w *writer) process(update interface{}) error { toBeClosed []consumerServiceWriter multiErr xerrors.MultiError ) + // NOTE: processing topic update for _, cs := range t.ConsumerServices() { key := cs.ServiceID().String() csw, ok := w.consumerServiceWriters[key] @@ -201,6 +204,8 @@ func (w *writer) process(update interface{}) error { continue } csw.SetMessageTTLNanos(cs.MessageTTLNanos()) + csw.SetShardSet(cs.ShardSet()) + newConsumerServiceWriters[key] = csw w.logger.Info("initialized consumer service writer", zap.String("writer", cs.String())) } @@ -220,13 +225,36 @@ func (w *writer) process(update interface{}) error { // Apply the new consumer service writers. w.Lock() + + for key, csw := range newConsumerServiceWriters { + + } + for key, csw := range newConsumerServiceWriters { if filters, ok := w.filterRegistry[key]; ok { for _, filter := range filters { + // if filter.FilterType != producer.ShardSetFilter { csw.RegisterFilter(filter) + // } } + + // shardSetString := csw.GetShardSet() + // + // // only set if a shard set was specified + // if len(shardSetString) != 0 { + // shardSet, err := sharding.ParseShardSet(shardSetString) + // + // if err != nil { + // w.logger.Error("Invalid shard set for consumer service", zap.String("shardSet", shardSetString), zap.Error(err)) + // continue + // } + // + // csw.RegisterFilter(filter.NewShardSetFilter(shardSet)) + // } } } + + w.consumerServiceWriters = newConsumerServiceWriters w.numShards = t.NumberOfShards() w.Unlock() diff --git a/src/msg/producer/writer/writer_test.go b/src/msg/producer/writer/writer_test.go index e929455a54..896e3359b0 100644 --- a/src/msg/producer/writer/writer_test.go +++ b/src/msg/producer/writer/writer_test.go @@ -211,8 +211,8 @@ func TestWriterRegisterFilter(t *testing.T) { csw1 := NewMockconsumerServiceWriter(ctrl) sid2 := services.NewServiceID().SetName("s2") - filter := func(producer.Message) bool { return false } - filter2 := func(producer.Message) bool { return true } + filter := producer.NewFilterFunc(func(producer.Message) bool { return false }, producer.UnspecifiedFilter) + filter2 := producer.NewFilterFunc(func(producer.Message) bool { return true }, producer.UnspecifiedFilter) w := NewWriter(opts).(*writer) w.consumerServiceWriters[cs1.ServiceID().String()] = csw1 diff --git a/src/msg/topic/topic.go b/src/msg/topic/topic.go index b3a7885686..21cda5143d 100644 --- a/src/msg/topic/topic.go +++ b/src/msg/topic/topic.go @@ -212,6 +212,7 @@ type consumerService struct { sid services.ServiceID ct ConsumptionType ttlNanos int64 + shardSet string } // NewConsumerService creates a ConsumerService. @@ -274,6 +275,17 @@ func (cs *consumerService) SetMessageTTLNanos(value int64) ConsumerService { return &newcs } + +func (cs *consumerService) ShardSet() string { + return cs.shardSet +} + +func (cs *consumerService) SetShardSet(value string) ConsumerService { + newcs := *cs + newcs.shardSet = value + return &newcs +} + func (cs *consumerService) String() string { var buf bytes.Buffer buf.WriteString("{") diff --git a/src/msg/topic/types.go b/src/msg/topic/types.go index d6ced4fcbe..fe86796423 100644 --- a/src/msg/topic/types.go +++ b/src/msg/topic/types.go @@ -88,10 +88,31 @@ type ConsumerService interface { // SetMessageTTLNanos sets ttl for each message in nanoseconds. SetMessageTTLNanos(value int64) ConsumerService + // StoragePolicyFilter returns the storage policy data filter of the consumer service. + StoragePolicyFilter() StoragePolicyFilter + + // PercentageFilter returns the percentage data filter of the consumer service. + PercentageFilter() PercentageFilter + + // ShardSetFilte returns the shard set data filter of the consumer service. + ShardSetFilter() ShardSetFilter + // String returns the string representation of the consumer service. String() string } +type PercentageFilter interface { + Percentage() uint32 +} + +type StoragePolicyFilter interface { + StoragePolicies() string +} + +type ShardSetFilter interface { + ShardSet() +} + // Watch watches the updates of a topic. type Watch interface { // C returns the notification channel. From 6d24bc904a69e2f01bcd9b6421c7a8dcbc866e39 Mon Sep 17 00:00:00 2001 From: Adam Jean-Laurent Date: Mon, 23 Sep 2024 14:34:45 -0400 Subject: [PATCH 02/18] hookup protos --- src/aggregator/aggregator/handler/config.go | 7 +- .../handler/filter/percentageFilter.go | 4 +- .../handler/filter/percentageFilter_test.go | 6 +- .../aggregator/handler/filter/shard.go | 4 +- .../aggregator/handler/filter/shard_test.go | 2 +- .../aggregator/handler/writer/protobuf.go | 4 +- .../handler/writer/protobuf_test.go | 2 +- src/msg/generated/proto/topicpb/topic.proto | 12 +- src/msg/producer/ref_counted_test.go | 7 +- src/msg/producer/types.go | 25 ++- .../writer/consumer_service_writer.go | 7 +- .../writer/consumer_service_writer_test.go | 4 +- src/msg/producer/writer/writer.go | 165 ++++++++++++++---- src/msg/producer/writer/writer_test.go | 4 +- src/msg/topic/topic.go | 85 ++++++++- src/msg/topic/types.go | 24 ++- 16 files changed, 281 insertions(+), 81 deletions(-) diff --git a/src/aggregator/aggregator/handler/config.go b/src/aggregator/aggregator/handler/config.go index 759553f5c2..943a19dd02 100644 --- a/src/aggregator/aggregator/handler/config.go +++ b/src/aggregator/aggregator/handler/config.go @@ -216,9 +216,8 @@ type storagePolicyFilterConfiguration struct { StoragePolicies []policy.StoragePolicy `yaml:"storagePolicies" validate:"nonzero"` } - func (c storagePolicyFilterConfiguration) NewConsumerServiceFilter() (services.ServiceID, producer.FilterFunc) { - return c.ServiceID.NewServiceID(), writer.NewStoragePolicyFilter(c.StoragePolicies) + return c.ServiceID.NewServiceID(), writer.NewStoragePolicyFilter(c.StoragePolicies, producer.StaticConfig) } type percentageFilterConfiguration struct { @@ -227,7 +226,7 @@ type percentageFilterConfiguration struct { } func (c percentageFilterConfiguration) NewConsumerServiceFilter() (services.ServiceID, producer.FilterFunc) { - return c.ServiceID.NewServiceID(), filter.NewPercentageFilter(c.Percentage) + return c.ServiceID.NewServiceID(), filter.NewPercentageFilter(c.Percentage, producer.StaticConfig) } // ConsumerServiceFilterConfiguration - exported to be able to write unit tests @@ -238,7 +237,7 @@ type ConsumerServiceFilterConfiguration struct { // NewConsumerServiceFilter - exported to be able to write unit tests func (c ConsumerServiceFilterConfiguration) NewConsumerServiceFilter() (services.ServiceID, producer.FilterFunc) { - return c.ServiceID.NewServiceID(), filter.NewShardSetFilter(c.ShardSet) + return c.ServiceID.NewServiceID(), filter.NewShardSetFilter(c.ShardSet, producer.StaticConfig) } // StaticBackendConfiguration configures a static backend as a flush handler. diff --git a/src/aggregator/aggregator/handler/filter/percentageFilter.go b/src/aggregator/aggregator/handler/filter/percentageFilter.go index 27214497cc..1e1d99b873 100644 --- a/src/aggregator/aggregator/handler/filter/percentageFilter.go +++ b/src/aggregator/aggregator/handler/filter/percentageFilter.go @@ -32,7 +32,7 @@ type percentageFilter struct { } // NewPercentageFilter creates a filter based on percentage. -func NewPercentageFilter(percentage float64) producer.FilterFunc { +func NewPercentageFilter(percentage float64, configSource producer.FilterFuncConfigSourceType) producer.FilterFunc { rate := uint32(_maxRate) if percentage <= 0 { rate = 0 @@ -41,7 +41,7 @@ func NewPercentageFilter(percentage float64) producer.FilterFunc { } f := percentageFilter{rate: rate} - return producer.NewFilterFunc(f.Filter, producer.PercentageFilter) + return producer.NewFilterFunc(f.Filter, producer.PercentageFilter, configSource) } func (f percentageFilter) Filter(_ producer.Message) bool { diff --git a/src/aggregator/aggregator/handler/filter/percentageFilter_test.go b/src/aggregator/aggregator/handler/filter/percentageFilter_test.go index 21b1f1e211..c17c9397cd 100644 --- a/src/aggregator/aggregator/handler/filter/percentageFilter_test.go +++ b/src/aggregator/aggregator/handler/filter/percentageFilter_test.go @@ -34,10 +34,10 @@ func TestPercentageFilter(t *testing.T) { defer ctrl.Finish() mm := producer.NewMockMessage(ctrl) - f0 := NewPercentageFilter(0) + f0 := NewPercentageFilter(0, producer.StaticConfig) require.False(t, f0.Function((mm))) - f1 := NewPercentageFilter(1) + f1 := NewPercentageFilter(1, producer.StaticConfig) require.True(t, f1.Function(mm)) } @@ -55,7 +55,7 @@ BenchmarkPercentageFilter-12 280085259 4.232 ns/op PASS */ func BenchmarkPercentageFilter(b *testing.B) { - f := NewPercentageFilter(0.5) + f := NewPercentageFilter(0.5, producer.StaticConfig) var r bool for i := 0; i < b.N; i++ { r = f.Function(nil) diff --git a/src/aggregator/aggregator/handler/filter/shard.go b/src/aggregator/aggregator/handler/filter/shard.go index b3bbef8609..7b654eb017 100644 --- a/src/aggregator/aggregator/handler/filter/shard.go +++ b/src/aggregator/aggregator/handler/filter/shard.go @@ -30,9 +30,9 @@ type shardSetFilter struct { } // NewShardSetFilter creates a filter based on ShardSet. -func NewShardSetFilter(shardSet sharding.ShardSet) producer.FilterFunc { +func NewShardSetFilter(shardSet sharding.ShardSet, sourceType producer.FilterFuncConfigSourceType) producer.FilterFunc { f := shardSetFilter{shardSet: shardSet} - return producer.NewFilterFunc(f.Filter, producer.ShardSetFilter) + return producer.NewFilterFunc(f.Filter, producer.ShardSetFilter, sourceType) } func (f shardSetFilter) Filter(m producer.Message) bool { diff --git a/src/aggregator/aggregator/handler/filter/shard_test.go b/src/aggregator/aggregator/handler/filter/shard_test.go index e612ce0f1f..ddec749b55 100644 --- a/src/aggregator/aggregator/handler/filter/shard_test.go +++ b/src/aggregator/aggregator/handler/filter/shard_test.go @@ -35,7 +35,7 @@ func TestShardSetFilter(t *testing.T) { defer ctrl.Finish() ss := sharding.MustParseShardSet("0..511") - f := NewShardSetFilter(ss) + f := NewShardSetFilter(ss, producer.StaticConfig) mm := producer.NewMockMessage(ctrl) mm.EXPECT().Shard().Return(uint32(0)) diff --git a/src/aggregator/aggregator/handler/writer/protobuf.go b/src/aggregator/aggregator/handler/writer/protobuf.go index d281edd792..4e22b91e0a 100644 --- a/src/aggregator/aggregator/handler/writer/protobuf.go +++ b/src/aggregator/aggregator/handler/writer/protobuf.go @@ -168,8 +168,8 @@ type storagePolicyFilter struct { } // NewStoragePolicyFilter creates a new storage policy based filter. -func NewStoragePolicyFilter(acceptedStoragePolicies []policy.StoragePolicy) producer.FilterFunc { - return producer.NewFilterFunc(storagePolicyFilter{acceptedStoragePolicies}.Filter, producer.StoragePolicyFilter) +func NewStoragePolicyFilter(acceptedStoragePolicies []policy.StoragePolicy, configSource producer.FilterFuncConfigSourceType) producer.FilterFunc { + return producer.NewFilterFunc(storagePolicyFilter{acceptedStoragePolicies}.Filter, producer.StoragePolicyFilter, configSource) } func (f storagePolicyFilter) Filter(m producer.Message) bool { diff --git a/src/aggregator/aggregator/handler/writer/protobuf_test.go b/src/aggregator/aggregator/handler/writer/protobuf_test.go index 3cece70227..dffda8e39d 100644 --- a/src/aggregator/aggregator/handler/writer/protobuf_test.go +++ b/src/aggregator/aggregator/handler/writer/protobuf_test.go @@ -90,7 +90,7 @@ func TestStoragePolicyFilter(t *testing.T) { m2 := producer.NewMockMessage(nil) - f := NewStoragePolicyFilter([]policy.StoragePolicy{sp2}) + f := NewStoragePolicyFilter([]policy.StoragePolicy{sp2}, producer.StaticConfig) require.True(t, f.Function(m2)) require.False(t, f.Function(newMessage(0, sp1, protobuf.Buffer{}))) diff --git a/src/msg/generated/proto/topicpb/topic.proto b/src/msg/generated/proto/topicpb/topic.proto index 054516f777..5bc5535cd4 100644 --- a/src/msg/generated/proto/topicpb/topic.proto +++ b/src/msg/generated/proto/topicpb/topic.proto @@ -20,17 +20,11 @@ message Filters { ShardSetFilter shard_set_filter = 3; } -message StoragePolicyFilter { - repeated string storage_policies = 1; -} +message StoragePolicyFilter { repeated string storage_policies = 1; } -message PercentageFilter { - uint32 percentage = 1; -} +message PercentageFilter { Float64 percentage = 1; } -message ShardSetFilter { - string shard_set = 1; -} +message ShardSetFilter { string shard_set = 1; } message ServiceID { string name = 1; diff --git a/src/msg/producer/ref_counted_test.go b/src/msg/producer/ref_counted_test.go index 6e5a798a8b..07d53b3c90 100644 --- a/src/msg/producer/ref_counted_test.go +++ b/src/msg/producer/ref_counted_test.go @@ -127,14 +127,15 @@ func TestRefCountedMessageFilter(t *testing.T) { func(m Message) bool { called++ return m.Shard() == 0 - }, + }, UnspecifiedFilter, - ) + StaticConfig, + ) sizeFilter := NewFilterFunc(func(m Message) bool { called++ return m.Size() == 0 - }, UnspecifiedFilter) + }, UnspecifiedFilter, StaticConfig) mm := NewMockMessage(ctrl) mm.EXPECT().Size().Return(0) diff --git a/src/msg/producer/types.go b/src/msg/producer/types.go index 4b886a4779..efdea28f12 100644 --- a/src/msg/producer/types.go +++ b/src/msg/producer/types.go @@ -95,16 +95,35 @@ const ( UnspecifiedFilter ) +type FilterFuncConfigSourceType uint8 + +const ( + StaticConfig FilterFuncConfigSourceType = iota + DynamicConfig +) + +type FilterFuncMetadata struct { + FilterType FilterFuncType + SourceType FilterFuncConfigSourceType +} + +func NewFilterFuncMetadata(filterType FilterFuncType, sourceType FilterFuncConfigSourceType) FilterFuncMetadata { + return FilterFuncMetadata{ + FilterType: filterType, + SourceType: sourceType, + } +} + // FilterFunc can filter message. type FilterFunc struct { Function func(m Message) bool - FilterType FilterFuncType + Metadata FilterFuncMetadata } -func NewFilterFunc(function func(m Message) bool, filterType FilterFuncType) FilterFunc { +func NewFilterFunc(function func(m Message) bool, filterType FilterFuncType, sourceType FilterFuncConfigSourceType) FilterFunc { return FilterFunc{ Function: function, - FilterType: filterType, + Metadata: NewFilterFuncMetadata(filterType, sourceType), } } diff --git a/src/msg/producer/writer/consumer_service_writer.go b/src/msg/producer/writer/consumer_service_writer.go index 2848ed36ee..561459dba9 100644 --- a/src/msg/producer/writer/consumer_service_writer.go +++ b/src/msg/producer/writer/consumer_service_writer.go @@ -41,7 +41,10 @@ var ( Function: func(m producer.Message) bool { return true }, - FilterType: producer.AcceptAllFilter, + Metadata: producer.FilterFuncMetadata{ + FilterType: producer.AcceptAllFilter, + SourceType: producer.StaticConfig, + }, }, ) @@ -118,7 +121,7 @@ type consumerServiceWriterImpl struct { wg sync.WaitGroup m consumerServiceWriterMetrics cm consumerWriterMetrics - shardSet string + shardSet string processFn watch.ProcessFn } diff --git a/src/msg/producer/writer/consumer_service_writer_test.go b/src/msg/producer/writer/consumer_service_writer_test.go index 909b1c5c9f..3d0383b084 100644 --- a/src/msg/producer/writer/consumer_service_writer_test.go +++ b/src/msg/producer/writer/consumer_service_writer_test.go @@ -521,7 +521,7 @@ func TestConsumerServiceWriterFilter(t *testing.T) { sw1.EXPECT().Write(gomock.Any()) csw.Write(producer.NewRefCountedMessage(mm1, nil)) - csw.RegisterFilter(producer.NewFilterFunc(func(m producer.Message) bool { return m.Shard() == uint32(0) }, producer.UnspecifiedFilter)) + csw.RegisterFilter(producer.NewFilterFunc(func(m producer.Message) bool { return m.Shard() == uint32(0) }, producer.UnspecifiedFilter, producer.StaticConfig)) // Write is not expected due to mm1 shard != 0 csw.Write(producer.NewRefCountedMessage(mm1, nil)) @@ -529,7 +529,7 @@ func TestConsumerServiceWriterFilter(t *testing.T) { // Write is expected due to mm0 shard == 0 csw.Write(producer.NewRefCountedMessage(mm0, nil)) - csw.RegisterFilter(producer.NewFilterFunc(func(m producer.Message) bool { return m.Size() == 3 }, producer.UnspecifiedFilter)) + csw.RegisterFilter(producer.NewFilterFunc(func(m producer.Message) bool { return m.Size() == 3 }, producer.UnspecifiedFilter, producer.StaticConfig)) sw0.EXPECT().Write(gomock.Any()) // Write is expected because to mm0 shard == 0 and mm0 size == 3 csw.Write(producer.NewRefCountedMessage(mm0, nil)) diff --git a/src/msg/producer/writer/writer.go b/src/msg/producer/writer/writer.go index 0580c5d88f..d5b2645257 100644 --- a/src/msg/producer/writer/writer.go +++ b/src/msg/producer/writer/writer.go @@ -29,8 +29,10 @@ import ( "go.uber.org/zap" "github.com/m3db/m3/src/aggregator/aggregator/handler/filter" + handlerWriter "github.com/m3db/m3/src/aggregator/aggregator/handler/writer" "github.com/m3db/m3/src/aggregator/sharding" "github.com/m3db/m3/src/cluster/services" + "github.com/m3db/m3/src/metrics/policy" "github.com/m3db/m3/src/msg/producer" "github.com/m3db/m3/src/msg/topic" xerrors "github.com/m3db/m3/src/x/errors" @@ -155,7 +157,7 @@ func (w *writer) NumShards() uint32 { return n } -func (w *writer) process(update interface{}) error { +func (w *writer) process(update interface{}) error { t := update.(topic.Topic) if err := t.Validate(); err != nil { return err @@ -168,20 +170,35 @@ func (w *writer) process(update interface{}) error { return fmt.Errorf("invalid topic update with %d shards, expecting %d", t.NumberOfShards(), numShards) } var ( - iOpts = w.opts.InstrumentOptions() - newConsumerServiceWriters = make(map[string]consumerServiceWriter, len(t.ConsumerServices())) - toBeClosed []consumerServiceWriter - multiErr xerrors.MultiError + iOpts = w.opts.InstrumentOptions() + newConsumerServiceWriters = make(map[string]consumerServiceWriter, len(t.ConsumerServices())) + toBeClosed []consumerServiceWriter + multiErr xerrors.MultiError + consumerServicesWithDynamicFilterConfig = make(map[string]bool, len(t.ConsumerServices())) ) - // NOTE: processing topic update for _, cs := range t.ConsumerServices() { key := cs.ServiceID().String() csw, ok := w.consumerServiceWriters[key] if ok { csw.SetMessageTTLNanos(cs.MessageTTLNanos()) newConsumerServiceWriters[key] = csw + + if cs.DynamicFilterConfigs() != nil { + // NOTE: do we need to delete the ones from the static config ???? + consumerServicesWithDynamicFilterConfig[key] = true + err := RegisterDynamicFilters(csw, cs.DynamicFilterConfigs()) + if err != nil { + w.logger.Error("could not register dynamic filters", + zap.String("writer", cs.String()), zap.Error(err)) + + // NOTE: emit metric !! + multiErr = multiErr.Add(err) + } + } + continue } + // NOTE: we should be able to use this scope later when doing metrics !!! scope := iOpts.MetricsScope().Tagged(map[string]string{ "consumer-service-name": cs.ServiceID().Name(), "consumer-service-zone": cs.ServiceID().Zone(), @@ -189,6 +206,19 @@ func (w *writer) process(update interface{}) error { "consumption-type": cs.ConsumptionType().String(), }) csw, err := newConsumerServiceWriter(cs, t.NumberOfShards(), w.opts.SetInstrumentOptions(iOpts.SetMetricsScope(scope))) + + if cs.DynamicFilterConfigs() != nil { + consumerServicesWithDynamicFilterConfig[key] = true + err := RegisterDynamicFilters(csw, cs.DynamicFilterConfigs()) + if err != nil { + w.logger.Error("could not register dynamic filters", + zap.String("writer", cs.String()), zap.Error(err)) + + // NOTE: emit metric !! + multiErr = multiErr.Add(err) + } + } + if err != nil { w.logger.Error("could not create consumer service writer", zap.String("writer", cs.String()), zap.Error(err)) @@ -204,7 +234,6 @@ func (w *writer) process(update interface{}) error { continue } csw.SetMessageTTLNanos(cs.MessageTTLNanos()) - csw.SetShardSet(cs.ShardSet()) newConsumerServiceWriters[key] = csw w.logger.Info("initialized consumer service writer", zap.String("writer", cs.String())) @@ -227,33 +256,15 @@ func (w *writer) process(update interface{}) error { w.Lock() for key, csw := range newConsumerServiceWriters { - - } - - for key, csw := range newConsumerServiceWriters { - if filters, ok := w.filterRegistry[key]; ok { - for _, filter := range filters { - // if filter.FilterType != producer.ShardSetFilter { - csw.RegisterFilter(filter) - // } + _, usingDynamicConfigForFilters := consumerServicesWithDynamicFilterConfig[key] + if !usingDynamicConfigForFilters { + if filters, ok := w.filterRegistry[key]; ok { + for _, filter := range filters { + csw.RegisterFilter(filter) + } } - - // shardSetString := csw.GetShardSet() - // - // // only set if a shard set was specified - // if len(shardSetString) != 0 { - // shardSet, err := sharding.ParseShardSet(shardSetString) - // - // if err != nil { - // w.logger.Error("Invalid shard set for consumer service", zap.String("shardSet", shardSetString), zap.Error(err)) - // continue - // } - // - // csw.RegisterFilter(filter.NewShardSetFilter(shardSet)) - // } } } - w.consumerServiceWriters = newConsumerServiceWriters w.numShards = t.NumberOfShards() @@ -313,3 +324,95 @@ func (w *writer) UnregisterFilters(sid services.ServiceID) { csw.UnregisterFilters() } } + +func RegisterDynamicFilters(csw consumerServiceWriter, filterConfig topic.FilterConfig) error { + if filterConfig == nil { + return errors.New("nil filter config") + } + + if filterConfig.ShardSetFilter() != nil { + if err := RegisterShardSetFilterFromTopicUpdate(csw, filterConfig.ShardSetFilter()); err != nil { + return fmt.Errorf("Error registering shard set filter: %v", err) + } + } + + if filterConfig.StoragePolicyFilter() != nil { + if err := RegisterStoragePolicyFilterFromTopicUpdate(csw, filterConfig.StoragePolicyFilter()); err != nil { + return fmt.Errorf("Error registering storage policy filter: %v", err) + } + } + + if filterConfig.PercentageFilter() != nil { + if err := RegisterPercentageFilterFromFromTopicUpdate(csw, filterConfig.PercentageFilter()); err != nil { + return fmt.Errorf("Error registering percentage filter: %v", err) + } + } + + return nil +} + +func RegisterShardSetFilterFromTopicUpdate(csw consumerServiceWriter, ssf topic.ShardSetFilter) error { + if ssf == nil { + return errors.New("nil shard set filter") + } + + shardSetString := ssf.ShardSet() + + if len(shardSetString) == 0 { + return errors.New("no shard set string specified in filter") + } + + shardSet, err := sharding.ParseShardSet(shardSetString) + + if err != nil { + return errors.New("Error parsing shard set") + } + + csw.RegisterFilter(filter.NewShardSetFilter(shardSet, producer.DynamicConfig)) + + return nil +} + +func RegisterStoragePolicyFilterFromTopicUpdate(csw consumerServiceWriter, spf topic.StoragePolicyFilter) error { + if spf == nil { + return errors.New("nil storage policy filter") + } + + storagePolicies := spf.StoragePolicies() + + if len(storagePolicies) == 0 { + return errors.New("no storage policies specified in filter") + } + + parsedPolicies := make([]policy.StoragePolicy, len(storagePolicies)) + for _, storagePolicyString := range storagePolicies { + parsedPolicy, err := policy.ParseStoragePolicy(storagePolicyString) + + if err != nil { + return fmt.Errorf("Error parsing storage policy: %v", err) + } + + parsedPolicies = append(parsedPolicies, parsedPolicy) + + csw.RegisterFilter(handlerWriter.NewStoragePolicyFilter(parsedPolicies, producer.DynamicConfig)) + } + + return nil +} + +func RegisterPercentageFilterFromFromTopicUpdate(csw consumerServiceWriter, pf topic.PercentageFilter) error { + + if pf == nil { + return errors.New("nil percentage filter") + } + + percentage := pf.Percentage() + + if percentage == 0 { + return errors.New("no percentage specified in filter") + } + + csw.RegisterFilter(filter.NewPercentageFilter(percentage, producer.DynamicConfig)) + + return nil +} diff --git a/src/msg/producer/writer/writer_test.go b/src/msg/producer/writer/writer_test.go index 896e3359b0..38dba8112a 100644 --- a/src/msg/producer/writer/writer_test.go +++ b/src/msg/producer/writer/writer_test.go @@ -211,8 +211,8 @@ func TestWriterRegisterFilter(t *testing.T) { csw1 := NewMockconsumerServiceWriter(ctrl) sid2 := services.NewServiceID().SetName("s2") - filter := producer.NewFilterFunc(func(producer.Message) bool { return false }, producer.UnspecifiedFilter) - filter2 := producer.NewFilterFunc(func(producer.Message) bool { return true }, producer.UnspecifiedFilter) + filter := producer.NewFilterFunc(func(producer.Message) bool { return false }, producer.UnspecifiedFilter, producer.StaticConfig) + filter2 := producer.NewFilterFunc(func(producer.Message) bool { return true }, producer.UnspecifiedFilter, producer.StaticConfig) w := NewWriter(opts).(*writer) w.consumerServiceWriters[cs1.ServiceID().String()] = csw1 diff --git a/src/msg/topic/topic.go b/src/msg/topic/topic.go index 21cda5143d..e82f20d25b 100644 --- a/src/msg/topic/topic.go +++ b/src/msg/topic/topic.go @@ -163,6 +163,7 @@ func (t *topic) String() string { buf.WriteString("\tconsumerServices: {\n") } for _, cs := range t.consumerServices { + // NOTE: update to print filters? buf.WriteString(fmt.Sprintf("\t\t%s\n", cs.String())) } if len(t.consumerServices) > 0 { @@ -208,13 +209,56 @@ func ToProto(t Topic) (*topicpb.Topic, error) { }, nil } -type consumerService struct { - sid services.ServiceID - ct ConsumptionType - ttlNanos int64 +type percentageFilter struct { + percentage float64 +} + +func (filter *percentageFilter) Percentage() float64 { + return filter.percentage +} + +type storagePolicyFilter struct { + storagePolicies []string +} + +func (filter *storagePolicyFilter) StoragePolicies() []string { + return filter.storagePolicies +} + +type shardSetFilter struct { shardSet string } +func (filter *shardSetFilter) ShardSet() string { + return filter.shardSet +} + +type filterConfig struct { + shardSetFilterConfig *shardSetFilter + percentageFilterConfig *percentageFilter + storagePolicyFilterConfig *storagePolicyFilter +} + +func (fc *filterConfig) ShardSetFilter() ShardSetFilter { + return fc.shardSetFilterConfig +} + +func (fc *filterConfig) PercentageFilter() PercentageFilter { + return fc.percentageFilterConfig +} + +func (fc *filterConfig) StoragePolicyFilter() StoragePolicyFilter { + return fc.storagePolicyFilterConfig +} + +type consumerService struct { + sid services.ServiceID + ct ConsumptionType + ttlNanos int64 + shardSet string + filterConfigs *filterConfig +} + // NewConsumerService creates a ConsumerService. func NewConsumerService() ConsumerService { return new(consumerService) @@ -229,7 +273,8 @@ func NewConsumerServiceFromProto(cs *topicpb.ConsumerService) (ConsumerService, return NewConsumerService(). SetServiceID(NewServiceIDFromProto(cs.ServiceId)). SetConsumptionType(ct). - SetMessageTTLNanos(cs.MessageTtlNanos), nil + SetMessageTTLNanos(cs.MessageTtlNanos). + SetDynamicFilterConfigs(NewDynamicFilterConfigFromProto(cs.Filters)), nil } // ConsumerServiceToProto creates proto from a ConsumerService. @@ -275,7 +320,6 @@ func (cs *consumerService) SetMessageTTLNanos(value int64) ConsumerService { return &newcs } - func (cs *consumerService) ShardSet() string { return cs.shardSet } @@ -286,6 +330,16 @@ func (cs *consumerService) SetShardSet(value string) ConsumerService { return &newcs } +func (cs *consumerService) DynamicFilterConfigs() FilterConfig { + return cs.filterConfigs +} + +func (cs *consumerService) SetDynamicFilterConfigs(value FilterConfig) ConsumerService { + newcs := *cs + newcs.filterConfigs = value.(*filterConfig) + return &newcs +} + func (cs *consumerService) String() string { var buf bytes.Buffer buf.WriteString("{") @@ -310,3 +364,22 @@ func ServiceIDToProto(sid services.ServiceID) *topicpb.ServiceID { Zone: sid.Zone(), } } + +// NewDynamicFilterConfigFromProto creates filters from a proto. +func NewDynamicFilterConfigFromProto(filterProto *topicpb.Filters) FilterConfig { + if filterProto == nil { + return nil + } + var filter filterConfig + if filterProto.ShardSetFilter != nil { + filter.shardSetFilterConfig = &shardSetFilter{shardSet: filterProto.ShardSetFilter.ShardSet} + } + if filterProto.PercentageFilter != nil { + filter.percentageFilterConfig = &percentageFilter{percentage: filterProto.PercentageFilter.Percentage} + } + if filterProto.StoragePolicyFilter != nil { + filter.storagePolicyFilterConfig = &storagePolicyFilter{storagePolicies: filterProto.StoragePolicyFilter.StoragePolicies} + } + + return &filter +} diff --git a/src/msg/topic/types.go b/src/msg/topic/types.go index fe86796423..a15322fb5d 100644 --- a/src/msg/topic/types.go +++ b/src/msg/topic/types.go @@ -88,29 +88,37 @@ type ConsumerService interface { // SetMessageTTLNanos sets ttl for each message in nanoseconds. SetMessageTTLNanos(value int64) ConsumerService + // DynamicFilterConfigs returns the dynamic filters for the consumer service. + DynamicFilterConfigs() FilterConfig + + // SetDynamicFilterConfigs sets the dynamic filters for the consumer service. + SetDynamicFilterConfigs(value FilterConfig) ConsumerService + + // String returns the string representation of the consumer service. + String() string +} + +type FilterConfig interface { // StoragePolicyFilter returns the storage policy data filter of the consumer service. StoragePolicyFilter() StoragePolicyFilter // PercentageFilter returns the percentage data filter of the consumer service. PercentageFilter() PercentageFilter - - // ShardSetFilte returns the shard set data filter of the consumer service. - ShardSetFilter() ShardSetFilter - // String returns the string representation of the consumer service. - String() string + // ShardSetFilter returns the shard set data filter of the consumer service. + ShardSetFilter() ShardSetFilter } type PercentageFilter interface { - Percentage() uint32 + Percentage() float64 } type StoragePolicyFilter interface { - StoragePolicies() string + StoragePolicies() []string } type ShardSetFilter interface { - ShardSet() + ShardSet() string } // Watch watches the updates of a topic. From d080e438e7739cd932ebd338be71ffdfcac7fa33 Mon Sep 17 00:00:00 2001 From: Adam Jean-Laurent Date: Mon, 23 Sep 2024 14:37:32 -0400 Subject: [PATCH 03/18] remove unused param --- src/msg/producer/writer/consumer_service_writer.go | 1 - 1 file changed, 1 deletion(-) diff --git a/src/msg/producer/writer/consumer_service_writer.go b/src/msg/producer/writer/consumer_service_writer.go index 561459dba9..734a59c13a 100644 --- a/src/msg/producer/writer/consumer_service_writer.go +++ b/src/msg/producer/writer/consumer_service_writer.go @@ -154,7 +154,6 @@ func newConsumerServiceWriter( doneCh: make(chan struct{}), m: newConsumerServiceWriterMetrics(opts.InstrumentOptions().MetricsScope()), cm: newConsumerWriterMetrics(opts.InstrumentOptions().MetricsScope()), - shardSet: cs.ShardSet(), } w.processFn = w.process return w, nil From eca426b808bf0c63e1e5b5f8dff9263be4afdc5f Mon Sep 17 00:00:00 2001 From: ajeanlaurent Date: Mon, 23 Sep 2024 19:37:20 +0000 Subject: [PATCH 04/18] update proto types --- src/msg/generated/proto/topicpb/topic.pb.go | 107 ++++++++++---------- src/msg/generated/proto/topicpb/topic.proto | 2 +- 2 files changed, 52 insertions(+), 57 deletions(-) diff --git a/src/msg/generated/proto/topicpb/topic.pb.go b/src/msg/generated/proto/topicpb/topic.pb.go index 622309fab7..a1c3c08ebe 100644 --- a/src/msg/generated/proto/topicpb/topic.pb.go +++ b/src/msg/generated/proto/topicpb/topic.pb.go @@ -22,6 +22,8 @@ import proto "github.com/gogo/protobuf/proto" import fmt "fmt" import math "math" +import binary "encoding/binary" + import io "io" // Reference imports to suppress errors if they are not otherwise used. @@ -180,7 +182,7 @@ func (m *StoragePolicyFilter) GetStoragePolicies() []string { } type PercentageFilter struct { - Percentage uint32 `protobuf:"varint,1,opt,name=percentage,proto3" json:"percentage,omitempty"` + Percentage float64 `protobuf:"fixed64,1,opt,name=percentage,proto3" json:"percentage,omitempty"` } func (m *PercentageFilter) Reset() { *m = PercentageFilter{} } @@ -188,7 +190,7 @@ func (m *PercentageFilter) String() string { return proto.CompactText func (*PercentageFilter) ProtoMessage() {} func (*PercentageFilter) Descriptor() ([]byte, []int) { return fileDescriptorTopic, []int{4} } -func (m *PercentageFilter) GetPercentage() uint32 { +func (m *PercentageFilter) GetPercentage() float64 { if m != nil { return m.Percentage } @@ -439,9 +441,10 @@ func (m *PercentageFilter) MarshalTo(dAtA []byte) (int, error) { var l int _ = l if m.Percentage != 0 { - dAtA[i] = 0x8 + dAtA[i] = 0x9 i++ - i = encodeVarintTopic(dAtA, i, uint64(m.Percentage)) + binary.LittleEndian.PutUint64(dAtA[i:], uint64(math.Float64bits(float64(m.Percentage)))) + i += 8 } return i, nil } @@ -588,7 +591,7 @@ func (m *PercentageFilter) Size() (n int) { var l int _ = l if m.Percentage != 0 { - n += 1 + sovTopic(uint64(m.Percentage)) + n += 9 } return n } @@ -1175,24 +1178,16 @@ func (m *PercentageFilter) Unmarshal(dAtA []byte) error { } switch fieldNum { case 1: - if wireType != 0 { + if wireType != 1 { return fmt.Errorf("proto: wrong wireType = %d for field Percentage", wireType) } - m.Percentage = 0 - for shift := uint(0); ; shift += 7 { - if shift >= 64 { - return ErrIntOverflowTopic - } - if iNdEx >= l { - return io.ErrUnexpectedEOF - } - b := dAtA[iNdEx] - iNdEx++ - m.Percentage |= (uint32(b) & 0x7F) << shift - if b < 0x80 { - break - } + var v uint64 + if (iNdEx + 8) > l { + return io.ErrUnexpectedEOF } + v = uint64(binary.LittleEndian.Uint64(dAtA[iNdEx:])) + iNdEx += 8 + m.Percentage = float64(math.Float64frombits(v)) default: iNdEx = preIndex skippy, err := skipTopic(dAtA[iNdEx:]) @@ -1540,40 +1535,40 @@ func init() { } var fileDescriptorTopic = []byte{ - // 548 bytes of a gzipped FileDescriptorProto - 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x6c, 0x93, 0xdf, 0x6e, 0xda, 0x30, - 0x14, 0xc6, 0xeb, 0xd2, 0x95, 0xe5, 0xa0, 0x42, 0xea, 0x6a, 0x5a, 0xa6, 0x4d, 0x08, 0xe5, 0x2a, - 0x43, 0x1a, 0xd1, 0xe0, 0x6e, 0x17, 0xd3, 0x18, 0x50, 0x0d, 0x6d, 0xa2, 0xc8, 0x50, 0xed, 0x32, - 0x82, 0x60, 0x68, 0x24, 0x62, 0x47, 0xb6, 0xa9, 0xc4, 0x9e, 0x61, 0x17, 0x7b, 0xac, 0x5d, 0xee, - 0x11, 0x36, 0x26, 0xed, 0x39, 0xa6, 0x38, 0x26, 0xfc, 0x69, 0xaf, 0xe2, 0xfc, 0xf2, 0xf9, 0xf3, - 0x77, 0x4e, 0x8e, 0xe1, 0xfd, 0x22, 0x52, 0x77, 0xab, 0x69, 0x23, 0xe4, 0xb1, 0x1f, 0xb7, 0x66, - 0x53, 0x3f, 0x6e, 0xf9, 0x52, 0x84, 0x7e, 0x2c, 0x17, 0xfe, 0x82, 0x32, 0x2a, 0x26, 0x8a, 0xce, - 0xfc, 0x44, 0x70, 0xc5, 0x7d, 0xc5, 0x93, 0x28, 0x4c, 0xa6, 0xd9, 0xb3, 0xa1, 0x19, 0x2e, 0x1a, - 0xe8, 0x7e, 0x47, 0xf0, 0x64, 0x9c, 0xae, 0x31, 0x86, 0x33, 0x36, 0x89, 0xa9, 0x83, 0x6a, 0xc8, - 0xb3, 0x88, 0x5e, 0x63, 0x0f, 0x6c, 0xb6, 0x8a, 0xa7, 0x54, 0x04, 0x7c, 0x1e, 0xc8, 0xbb, 0x89, - 0x98, 0x49, 0xe7, 0xb4, 0x86, 0xbc, 0x0b, 0x52, 0xce, 0xf8, 0xcd, 0x7c, 0xa4, 0x29, 0xee, 0xc1, - 0x65, 0xc8, 0x99, 0x5c, 0xc5, 0x54, 0x04, 0x92, 0x8a, 0xfb, 0x28, 0xa4, 0xd2, 0x29, 0xd4, 0x0a, - 0x5e, 0xa9, 0xe9, 0x34, 0xcc, 0x61, 0x8d, 0x8e, 0x51, 0x8c, 0x32, 0x01, 0xb1, 0xc3, 0x43, 0x20, - 0xdd, 0x3f, 0x08, 0x2a, 0x47, 0x2a, 0xfc, 0x16, 0xc0, 0x38, 0x06, 0xd1, 0x4c, 0xc7, 0x2b, 0x35, - 0x71, 0xee, 0x69, 0x54, 0xfd, 0x2e, 0xb1, 0x8c, 0xaa, 0x3f, 0xc3, 0x1d, 0x30, 0xd6, 0x89, 0x8a, - 0x38, 0x0b, 0xd4, 0x3a, 0xa1, 0x3a, 0x77, 0xf9, 0x41, 0x18, 0x2d, 0x18, 0xaf, 0x13, 0x4a, 0x2a, - 0xe1, 0x21, 0xc0, 0x75, 0xb8, 0x8c, 0xa9, 0x94, 0x93, 0x05, 0x0d, 0x94, 0x5a, 0x06, 0x6c, 0xc2, - 0x78, 0x5a, 0x12, 0xf2, 0x0a, 0xa4, 0x62, 0x3e, 0x8c, 0xd5, 0x72, 0x90, 0x62, 0x5c, 0x87, 0xe2, - 0x3c, 0x5a, 0x2a, 0x2a, 0xa4, 0x73, 0xa6, 0x03, 0xda, 0xf9, 0x39, 0xd7, 0x19, 0x27, 0x5b, 0x81, - 0xfb, 0x0f, 0x41, 0xd1, 0x40, 0x3c, 0x84, 0x67, 0x52, 0x71, 0x91, 0x9e, 0x91, 0xf0, 0x65, 0x14, - 0xae, 0x83, 0x4c, 0x65, 0xca, 0x7c, 0xb5, 0x2b, 0x33, 0x53, 0x0d, 0xb5, 0x28, 0xdb, 0x4d, 0xae, - 0xe4, 0x43, 0x88, 0xaf, 0xe1, 0x32, 0xa1, 0x22, 0xa4, 0x4c, 0xa5, 0xa6, 0xc6, 0xed, 0x54, 0xbb, - 0xbd, 0xc8, 0xdd, 0x86, 0xb9, 0xc2, 0x58, 0xd9, 0xc9, 0x11, 0xc1, 0x6d, 0xb0, 0xf5, 0x0f, 0x0f, - 0x24, 0x55, 0x5b, 0x9b, 0x82, 0xb6, 0x79, 0xbe, 0x0b, 0x95, 0x0a, 0x46, 0x54, 0x19, 0x93, 0xb2, - 0x3c, 0x78, 0x77, 0x3f, 0xc0, 0xd5, 0x23, 0xb1, 0xf1, 0x6b, 0xb0, 0x0f, 0x6a, 0x8e, 0xa8, 0x74, - 0x50, 0xad, 0xe0, 0x59, 0xa4, 0xb2, 0x5f, 0x50, 0x44, 0xa5, 0xdb, 0x04, 0xfb, 0x38, 0x2a, 0xae, - 0x02, 0xec, 0xc2, 0xea, 0x3e, 0x5d, 0x90, 0x3d, 0xe2, 0xbe, 0x81, 0xf2, 0x61, 0x2e, 0xfc, 0x12, - 0xac, 0xbc, 0x14, 0x33, 0xde, 0x4f, 0xb7, 0x51, 0xdd, 0x5b, 0xb0, 0xf2, 0x11, 0x7a, 0xf4, 0x0e, - 0xd4, 0xa0, 0x44, 0xd9, 0x7d, 0x24, 0x38, 0x8b, 0x29, 0x53, 0xba, 0x95, 0x16, 0xd9, 0x47, 0xe9, - 0xae, 0x6f, 0x9c, 0x51, 0xdd, 0x1e, 0x8b, 0xe8, 0x75, 0xfd, 0xdd, 0x76, 0x8e, 0x77, 0xf3, 0x54, - 0x82, 0xe2, 0xed, 0xe0, 0xf3, 0xe0, 0xe6, 0xeb, 0xc0, 0x3e, 0xc1, 0x00, 0xe7, 0xa3, 0x4f, 0x6d, - 0xd2, 0xeb, 0xda, 0x08, 0x97, 0x01, 0x48, 0x6f, 0xf8, 0xa5, 0xdf, 0x69, 0x8f, 0x7b, 0x5d, 0xfb, - 0xf4, 0xa3, 0xfd, 0x73, 0x53, 0x45, 0xbf, 0x36, 0x55, 0xf4, 0x7b, 0x53, 0x45, 0x3f, 0xfe, 0x56, - 0x4f, 0xa6, 0xe7, 0xfa, 0xd6, 0xb6, 0xfe, 0x07, 0x00, 0x00, 0xff, 0xff, 0xd5, 0x26, 0x64, 0x8e, - 0xf7, 0x03, 0x00, 0x00, + // 549 bytes of a gzipped FileDescriptorProto + 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x6c, 0x93, 0xcf, 0x8e, 0xda, 0x3c, + 0x14, 0xc5, 0xc7, 0xc3, 0x7c, 0xc3, 0x97, 0x8b, 0x0a, 0xc1, 0xa3, 0xaa, 0xa9, 0x5a, 0x21, 0x94, + 0x55, 0x8a, 0x54, 0xa2, 0xc2, 0xae, 0x8b, 0xaa, 0x14, 0x18, 0x15, 0xb5, 0x62, 0x90, 0x61, 0xd4, + 0x65, 0x04, 0xc1, 0x30, 0x91, 0x88, 0x1d, 0xd9, 0x66, 0x24, 0xfa, 0x0c, 0x5d, 0xf4, 0xb1, 0xba, + 0xec, 0x23, 0xb4, 0x54, 0xea, 0x73, 0x54, 0x71, 0x4c, 0xf8, 0x33, 0xb3, 0x8a, 0xf3, 0xcb, 0xf1, + 0xf1, 0xb9, 0x37, 0xd7, 0xf0, 0x6e, 0x19, 0xa9, 0xbb, 0xf5, 0xac, 0x19, 0xf2, 0xd8, 0x8f, 0xdb, + 0xf3, 0x99, 0x1f, 0xb7, 0x7d, 0x29, 0x42, 0x3f, 0x96, 0x4b, 0x7f, 0x49, 0x19, 0x15, 0x53, 0x45, + 0xe7, 0x7e, 0x22, 0xb8, 0xe2, 0xbe, 0xe2, 0x49, 0x14, 0x26, 0xb3, 0xec, 0xd9, 0xd4, 0x0c, 0x17, + 0x0d, 0x74, 0xbf, 0x21, 0xf8, 0x6f, 0x92, 0xae, 0x31, 0x86, 0x0b, 0x36, 0x8d, 0xa9, 0x83, 0xea, + 0xc8, 0xb3, 0x88, 0x5e, 0x63, 0x0f, 0x6c, 0xb6, 0x8e, 0x67, 0x54, 0x04, 0x7c, 0x11, 0xc8, 0xbb, + 0xa9, 0x98, 0x4b, 0xe7, 0xbc, 0x8e, 0xbc, 0x27, 0xa4, 0x9c, 0xf1, 0x9b, 0xc5, 0x58, 0x53, 0xdc, + 0x87, 0x6a, 0xc8, 0x99, 0x5c, 0xc7, 0x54, 0x04, 0x92, 0x8a, 0xfb, 0x28, 0xa4, 0xd2, 0x29, 0xd4, + 0x0b, 0x5e, 0xa9, 0xe5, 0x34, 0xcd, 0x61, 0xcd, 0xae, 0x51, 0x8c, 0x33, 0x01, 0xb1, 0xc3, 0x63, + 0x20, 0xdd, 0xdf, 0x08, 0x2a, 0x27, 0x2a, 0xfc, 0x06, 0xc0, 0x38, 0x06, 0xd1, 0x5c, 0xc7, 0x2b, + 0xb5, 0x70, 0xee, 0x69, 0x54, 0x83, 0x1e, 0xb1, 0x8c, 0x6a, 0x30, 0xc7, 0x5d, 0x30, 0xd6, 0x89, + 0x8a, 0x38, 0x0b, 0xd4, 0x26, 0xa1, 0x3a, 0x77, 0xf9, 0x41, 0x18, 0x2d, 0x98, 0x6c, 0x12, 0x4a, + 0x2a, 0xe1, 0x31, 0xc0, 0x0d, 0xa8, 0xc6, 0x54, 0xca, 0xe9, 0x92, 0x06, 0x4a, 0xad, 0x02, 0x36, + 0x65, 0x3c, 0x2d, 0x09, 0x79, 0x05, 0x52, 0x31, 0x1f, 0x26, 0x6a, 0x35, 0x4c, 0x31, 0x6e, 0x40, + 0x71, 0x11, 0xad, 0x14, 0x15, 0xd2, 0xb9, 0xd0, 0x01, 0xed, 0xfc, 0x9c, 0xeb, 0x8c, 0x93, 0x9d, + 0xc0, 0xfd, 0x8b, 0xa0, 0x68, 0x20, 0x1e, 0xc1, 0x53, 0xa9, 0xb8, 0x48, 0xcf, 0x48, 0xf8, 0x2a, + 0x0a, 0x37, 0x41, 0xa6, 0x32, 0x65, 0xbe, 0xdc, 0x97, 0x99, 0xa9, 0x46, 0x5a, 0x94, 0xed, 0x26, + 0x57, 0xf2, 0x21, 0xc4, 0xd7, 0x50, 0x4d, 0xa8, 0x08, 0x29, 0x53, 0xa9, 0xa9, 0x71, 0x3b, 0xd7, + 0x6e, 0xcf, 0x73, 0xb7, 0x51, 0xae, 0x30, 0x56, 0x76, 0x72, 0x42, 0x70, 0x07, 0x6c, 0xfd, 0xc3, + 0x03, 0x49, 0xd5, 0xce, 0xa6, 0xa0, 0x6d, 0x9e, 0xed, 0x43, 0xa5, 0x82, 0x31, 0x55, 0xc6, 0xa4, + 0x2c, 0x8f, 0xde, 0xdd, 0xf7, 0x70, 0xf5, 0x48, 0x6c, 0xfc, 0x0a, 0xec, 0xa3, 0x9a, 0x23, 0x2a, + 0x1d, 0x54, 0x2f, 0x78, 0x16, 0xa9, 0x1c, 0x16, 0x14, 0x51, 0xe9, 0xb6, 0xc0, 0x3e, 0x8d, 0x8a, + 0x6b, 0x00, 0xfb, 0xb0, 0xba, 0x4f, 0x88, 0x1c, 0x10, 0xf7, 0x35, 0x94, 0x8f, 0x73, 0xe1, 0x17, + 0x60, 0xe5, 0xa5, 0x98, 0xf1, 0xfe, 0x7f, 0x17, 0xd5, 0xbd, 0x05, 0x2b, 0x1f, 0xa1, 0x47, 0xef, + 0x40, 0x1d, 0x4a, 0x94, 0xdd, 0x47, 0x82, 0xb3, 0x98, 0x32, 0xa5, 0x5b, 0x69, 0x91, 0x43, 0x94, + 0xee, 0xfa, 0xca, 0x19, 0xd5, 0xed, 0xb1, 0x88, 0x5e, 0x37, 0xde, 0xee, 0xe6, 0x78, 0x3f, 0x4f, + 0x25, 0x28, 0xde, 0x0e, 0x3f, 0x0d, 0x6f, 0xbe, 0x0c, 0xed, 0x33, 0x0c, 0x70, 0x39, 0xfe, 0xd8, + 0x21, 0xfd, 0x9e, 0x8d, 0x70, 0x19, 0x80, 0xf4, 0x47, 0x9f, 0x07, 0xdd, 0xce, 0xa4, 0xdf, 0xb3, + 0xcf, 0x3f, 0xd8, 0x3f, 0xb6, 0x35, 0xf4, 0x73, 0x5b, 0x43, 0xbf, 0xb6, 0x35, 0xf4, 0xfd, 0x4f, + 0xed, 0x6c, 0x76, 0xa9, 0x6f, 0x6d, 0xfb, 0x5f, 0x00, 0x00, 0x00, 0xff, 0xff, 0x6f, 0xa8, 0x59, + 0x77, 0xf7, 0x03, 0x00, 0x00, } diff --git a/src/msg/generated/proto/topicpb/topic.proto b/src/msg/generated/proto/topicpb/topic.proto index 5bc5535cd4..d45ef9fe70 100644 --- a/src/msg/generated/proto/topicpb/topic.proto +++ b/src/msg/generated/proto/topicpb/topic.proto @@ -22,7 +22,7 @@ message Filters { message StoragePolicyFilter { repeated string storage_policies = 1; } -message PercentageFilter { Float64 percentage = 1; } +message PercentageFilter { double percentage = 1; } message ShardSetFilter { string shard_set = 1; } From 6759d8bde08af023392918dc08952755492ebbbe Mon Sep 17 00:00:00 2001 From: Adam Jean-Laurent Date: Mon, 23 Sep 2024 17:10:39 -0400 Subject: [PATCH 05/18] hookup metrics --- src/msg/producer/ref_counted.go | 10 ++- src/msg/producer/types.go | 30 +++++++ .../writer/consumer_service_writer.go | 84 +++++++++++++++---- 3 files changed, 108 insertions(+), 16 deletions(-) diff --git a/src/msg/producer/ref_counted.go b/src/msg/producer/ref_counted.go index e5602231f0..474a3efa30 100644 --- a/src/msg/producer/ref_counted.go +++ b/src/msg/producer/ref_counted.go @@ -23,6 +23,7 @@ package producer import ( "sync" + "github.com/uber-go/tally" "go.uber.org/atomic" ) @@ -50,16 +51,23 @@ func NewRefCountedMessage(m Message, fn OnFinalizeFn) *RefCountedMessage { } } +// note: Not a fan at all of passing function pointers here, but passing ConsumerServiceWriterMetrics to RefCountedMessage.Accept would cause a cyclical depdenency. ¯\_(ツ)_/¯ +type filterAcceptCounterFn func(metadata FilterFuncMetadata) tally.Counter +type filterDenyCounterFn func(metadata FilterFuncMetadata) tally.Counter + // Accept returns true if the message can be accepted by the filter. -func (rm *RefCountedMessage) Accept(fn []FilterFunc) bool { +func (rm *RefCountedMessage) Accept(fn []FilterFunc, acceptCounterFn filterAcceptCounterFn, denyCounterFn filterDenyCounterFn) bool { if len(fn) == 0 { return false } for _, f := range fn { if !f.Function(rm.Message) { + denyCounterFn(f.Metadata).Inc(1) return false } + acceptCounterFn(f.Metadata).Inc(1) } + return true } diff --git a/src/msg/producer/types.go b/src/msg/producer/types.go index efdea28f12..07c4c1b966 100644 --- a/src/msg/producer/types.go +++ b/src/msg/producer/types.go @@ -95,6 +95,24 @@ const ( UnspecifiedFilter ) +func (f FilterFuncType) String() string { + switch f { + + case ShardSetFilter: + return "ShardSetFilter" + case StoragePolicyFilter: + return "StoragePolicyFilter" + case PercentageFilter: + return "PercentageFilter" + case AcceptAllFilter: + return "AcceptAllFilter" + case UnspecifiedFilter: + return "UnspecifiedFilter" + } + + return "Unknown" +} + type FilterFuncConfigSourceType uint8 const ( @@ -102,6 +120,18 @@ const ( DynamicConfig ) +func (f FilterFuncConfigSourceType) String() string { + switch f { + + case StaticConfig: + return "StaticConfig" + case DynamicConfig: + return "DynamicConfig" + } + + return "Unknown" +} + type FilterFuncMetadata struct { FilterType FilterFuncType SourceType FilterFuncConfigSourceType diff --git a/src/msg/producer/writer/consumer_service_writer.go b/src/msg/producer/writer/consumer_service_writer.go index 734a59c13a..459f1acb8a 100644 --- a/src/msg/producer/writer/consumer_service_writer.go +++ b/src/msg/producer/writer/consumer_service_writer.go @@ -85,21 +85,75 @@ type consumerServiceWriter interface { UnregisterFilters() } -type consumerServiceWriterMetrics struct { - placementError tally.Counter - placementUpdate tally.Counter - filterAccepted tally.Counter - filterNotAccepted tally.Counter - queueSize tally.Gauge +type ConsumerServiceWriterMetrics struct { + scope tally.Scope + placementError tally.Counter + placementUpdate tally.Counter + queueSize tally.Gauge + filterAccepted tally.Counter + filterNotAccepted tally.Counter + filterAcceptedGranular map[string]tally.Counter + filterAcceptedGranularLock sync.RWMutex + filterNotAcceptedGranular map[string]tally.Counter + filterNotAcceptedGranularLock sync.RWMutex } -func newConsumerServiceWriterMetrics(scope tally.Scope) consumerServiceWriterMetrics { - return consumerServiceWriterMetrics{ - placementUpdate: scope.Counter("placement-update"), - placementError: scope.Counter("placement-error"), - filterAccepted: scope.Counter("filter-accepted"), - filterNotAccepted: scope.Counter("filter-not-accepted"), - queueSize: scope.Gauge("queue-size"), +func (cswm *ConsumerServiceWriterMetrics) getGranularFilterCounterMapKey(metadata producer.FilterFuncMetadata) string { + return fmt.Sprintf("%s::%s", metadata.FilterType.String(), metadata.SourceType.String()) +} + +func (cswm *ConsumerServiceWriterMetrics) getFilterAcceptedGranularCounter(metadata producer.FilterFuncMetadata) tally.Counter { + key := cswm.getGranularFilterCounterMapKey(metadata) + + cswm.filterAcceptedGranularLock.RLock() + val, ok := cswm.filterAcceptedGranular[key] + cswm.filterAcceptedGranularLock.RUnlock() + + if !ok { + val = cswm.scope.Tagged(map[string]string{ + "config-source": metadata.SourceType.String(), + "filter-type": metadata.FilterType.String(), + }).Counter("filter-accepted-granular") + + cswm.filterAcceptedGranularLock.Lock() + cswm.filterAcceptedGranular[key] = val + cswm.filterAcceptedGranularLock.Unlock() + } + + return val +} + +func (cswm *ConsumerServiceWriterMetrics) getFilterNotAcceptedGranularCounter(metadata producer.FilterFuncMetadata) tally.Counter { + key := cswm.getGranularFilterCounterMapKey(metadata) + + cswm.filterNotAcceptedGranularLock.RLock() + val, ok := cswm.filterNotAcceptedGranular[key] + cswm.filterNotAcceptedGranularLock.RUnlock() + + if !ok { + val = cswm.scope.Tagged(map[string]string{ + "config-source": metadata.SourceType.String(), + "filter-type": metadata.FilterType.String(), + }).Counter("filter-not-accepted-granular") + + cswm.filterNotAcceptedGranularLock.Lock() + cswm.filterNotAcceptedGranular[key] = val + cswm.filterNotAcceptedGranularLock.Unlock() + } + + return val +} + +func newConsumerServiceWriterMetrics(scope tally.Scope) ConsumerServiceWriterMetrics { + return ConsumerServiceWriterMetrics{ + placementUpdate: scope.Counter("placement-update"), + placementError: scope.Counter("placement-error"), + filterAccepted: scope.Counter("filter-accepted"), + filterNotAccepted: scope.Counter("filter-not-accepted"), + scope: scope, + filterAcceptedGranular: make(map[string]tally.Counter), + filterNotAcceptedGranular: make(map[string]tally.Counter), + queueSize: scope.Gauge("queue-size"), } } @@ -119,7 +173,7 @@ type consumerServiceWriterImpl struct { closed bool doneCh chan struct{} wg sync.WaitGroup - m consumerServiceWriterMetrics + m ConsumerServiceWriterMetrics cm consumerWriterMetrics shardSet string @@ -186,7 +240,7 @@ func initShardWriters( } func (w *consumerServiceWriterImpl) Write(rm *producer.RefCountedMessage) { - if rm.Accept(w.dataFilters) { + if rm.Accept(w.dataFilters, w.m.getFilterAcceptedGranularCounter, w.m.getFilterNotAcceptedGranularCounter) { w.shardWriters[rm.Shard()].Write(rm) w.m.filterAccepted.Inc(1) return From b7e606188ea8e2c6fbdaba0668b4a1dc89151c5d Mon Sep 17 00:00:00 2001 From: Adam Jean-Laurent Date: Tue, 24 Sep 2024 18:09:39 -0400 Subject: [PATCH 06/18] unit tests --- src/msg/producer/ref_counted_test.go | 17 +- .../writer/consumer_service_writer.go | 7 + .../writer/consumer_service_writer_test.go | 40 ++ src/msg/producer/writer/writer.go | 32 +- src/msg/producer/writer/writer_test.go | 497 ++++++++++++++++++ src/msg/topic/topic.go | 111 +++- src/msg/topic/topic_test.go | 35 +- src/msg/topic/types.go | 9 + 8 files changed, 695 insertions(+), 53 deletions(-) diff --git a/src/msg/producer/ref_counted_test.go b/src/msg/producer/ref_counted_test.go index 07d53b3c90..f8462d8c72 100644 --- a/src/msg/producer/ref_counted_test.go +++ b/src/msg/producer/ref_counted_test.go @@ -27,8 +27,17 @@ import ( "github.com/golang/mock/gomock" "github.com/stretchr/testify/require" + "github.com/uber-go/tally" ) +var filterAcceptFn = func(metadata FilterFuncMetadata) tally.Counter { + return tally.NoopScope.Counter("accept") +} + +var filterDenyFn = func(metadata FilterFuncMetadata) tally.Counter { + return tally.NoopScope.Counter("deny") +} + func TestRefCountedMessageConsume(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() @@ -142,16 +151,16 @@ func TestRefCountedMessageFilter(t *testing.T) { rm := NewRefCountedMessage(mm, nil) mm.EXPECT().Shard().Return(uint32(0)) - require.True(t, rm.Accept([]FilterFunc{filter})) + require.True(t, rm.Accept([]FilterFunc{filter}, filterAcceptFn, filterDenyFn)) mm.EXPECT().Shard().Return(uint32(1)) - require.False(t, rm.Accept([]FilterFunc{filter})) + require.False(t, rm.Accept([]FilterFunc{filter}, filterAcceptFn, filterDenyFn)) mm.EXPECT().Shard().Return(uint32(0)) mm.EXPECT().Size().Return(0) - require.True(t, rm.Accept([]FilterFunc{filter, sizeFilter})) + require.True(t, rm.Accept([]FilterFunc{filter, sizeFilter}, filterAcceptFn, filterDenyFn)) - require.False(t, rm.Accept([]FilterFunc{})) + require.False(t, rm.Accept([]FilterFunc{}, filterAcceptFn, filterDenyFn)) } func TestRefCountedMessageOnDropFn(t *testing.T) { diff --git a/src/msg/producer/writer/consumer_service_writer.go b/src/msg/producer/writer/consumer_service_writer.go index 459f1acb8a..0bea78f090 100644 --- a/src/msg/producer/writer/consumer_service_writer.go +++ b/src/msg/producer/writer/consumer_service_writer.go @@ -83,6 +83,9 @@ type consumerServiceWriter interface { // UnregisterFilters unregisters the filters for the consumer service. UnregisterFilters() + + // GetDataFilter returns the data filters on the consumer service writer. + GetDataFilters() []producer.FilterFunc } type ConsumerServiceWriterMetrics struct { @@ -239,6 +242,10 @@ func initShardWriters( return sws } +func (w *consumerServiceWriterImpl) GetDataFilters() []producer.FilterFunc { + return w.dataFilters +} + func (w *consumerServiceWriterImpl) Write(rm *producer.RefCountedMessage) { if rm.Accept(w.dataFilters, w.m.getFilterAcceptedGranularCounter, w.m.getFilterNotAcceptedGranularCounter) { w.shardWriters[rm.Shard()].Write(rm) diff --git a/src/msg/producer/writer/consumer_service_writer_test.go b/src/msg/producer/writer/consumer_service_writer_test.go index 3d0383b084..78c13c37e7 100644 --- a/src/msg/producer/writer/consumer_service_writer_test.go +++ b/src/msg/producer/writer/consumer_service_writer_test.go @@ -30,6 +30,7 @@ import ( "github.com/fortytw2/leaktest" "github.com/golang/mock/gomock" "github.com/stretchr/testify/require" + "github.com/uber-go/tally" "github.com/m3db/m3/src/cluster/kv" "github.com/m3db/m3/src/cluster/kv/mem" @@ -686,6 +687,45 @@ func TestConsumerServiceCloseShardWritersConcurrently(t *testing.T) { } } +func TestConsumerServiceWriterMetrics(t *testing.T) { + testScope := tally.NewTestScope("test", nil) + + acceptedMetadata := producer.FilterFuncMetadata{FilterType: producer.ShardSetFilter, SourceType: producer.DynamicConfig} + notAcceptedMetadata := producer.FilterFuncMetadata{FilterType: producer.PercentageFilter, SourceType: producer.DynamicConfig} + + m := newConsumerServiceWriterMetrics(testScope) + m.getFilterAcceptedGranularCounter(acceptedMetadata).Inc(1) + m.getFilterNotAcceptedGranularCounter(notAcceptedMetadata).Inc(1) + + accepetKey := m.getGranularFilterCounterMapKey(acceptedMetadata) + notAcceptKey := m.getGranularFilterCounterMapKey(notAcceptedMetadata) + + _, ok := m.filterAcceptedGranular[accepetKey] + require.True(t, ok) + + _, ok = m.filterAcceptedGranular[notAcceptKey] + require.True(t, ok) + + gotValue := false + snap := testScope.Snapshot() + for _, c := range snap.Counters() { + if c.Name() == "test.filter-accepted-granular" { + require.Equal(t, "config-source", c.Tags()["DynamicConfig"]) + require.Equal(t, "filter-type", c.Tags()["ShardSetFilter"]) + require.Equal(t, int64(1), c.Value()) + gotValue = true + } + + if c.Name() == "test.filter-not-accepted-granular" { + require.Equal(t, "config-source", c.Tags()["DynamicConfig"]) + require.Equal(t, "filter-type", c.Tags()["PercentageFilter"]) + require.Equal(t, int64(1), c.Value()) + gotValue = true + } + } + require.True(t, gotValue) +} + func testPlacementService(store kv.Store, sid services.ServiceID) placement.Service { return service.NewPlacementService( storage.NewPlacementStorage(store, sid.String(), placement.NewOptions()), diff --git a/src/msg/producer/writer/writer.go b/src/msg/producer/writer/writer.go index d5b2645257..2fcc37b7ee 100644 --- a/src/msg/producer/writer/writer.go +++ b/src/msg/producer/writer/writer.go @@ -184,21 +184,21 @@ func (w *writer) process(update interface{}) error { newConsumerServiceWriters[key] = csw if cs.DynamicFilterConfigs() != nil { + w.logger.Debug("registering dynamic filters", zap.String("consumer-service", cs.String())) // NOTE: do we need to delete the ones from the static config ???? + consumerServicesWithDynamicFilterConfig[key] = true err := RegisterDynamicFilters(csw, cs.DynamicFilterConfigs()) if err != nil { w.logger.Error("could not register dynamic filters", zap.String("writer", cs.String()), zap.Error(err)) - // NOTE: emit metric !! multiErr = multiErr.Add(err) } } continue } - // NOTE: we should be able to use this scope later when doing metrics !!! scope := iOpts.MetricsScope().Tagged(map[string]string{ "consumer-service-name": cs.ServiceID().Name(), "consumer-service-zone": cs.ServiceID().Zone(), @@ -208,13 +208,14 @@ func (w *writer) process(update interface{}) error { csw, err := newConsumerServiceWriter(cs, t.NumberOfShards(), w.opts.SetInstrumentOptions(iOpts.SetMetricsScope(scope))) if cs.DynamicFilterConfigs() != nil { + w.logger.Debug("registering dynamic filters", zap.Any("dynamic-filter-configs", cs.DynamicFilterConfigs())) + consumerServicesWithDynamicFilterConfig[key] = true err := RegisterDynamicFilters(csw, cs.DynamicFilterConfigs()) if err != nil { w.logger.Error("could not register dynamic filters", zap.String("writer", cs.String()), zap.Error(err)) - // NOTE: emit metric !! multiErr = multiErr.Add(err) } } @@ -352,16 +353,8 @@ func RegisterDynamicFilters(csw consumerServiceWriter, filterConfig topic.Filter } func RegisterShardSetFilterFromTopicUpdate(csw consumerServiceWriter, ssf topic.ShardSetFilter) error { - if ssf == nil { - return errors.New("nil shard set filter") - } - shardSetString := ssf.ShardSet() - if len(shardSetString) == 0 { - return errors.New("no shard set string specified in filter") - } - shardSet, err := sharding.ParseShardSet(shardSetString) if err != nil { @@ -374,16 +367,8 @@ func RegisterShardSetFilterFromTopicUpdate(csw consumerServiceWriter, ssf topic. } func RegisterStoragePolicyFilterFromTopicUpdate(csw consumerServiceWriter, spf topic.StoragePolicyFilter) error { - if spf == nil { - return errors.New("nil storage policy filter") - } - storagePolicies := spf.StoragePolicies() - if len(storagePolicies) == 0 { - return errors.New("no storage policies specified in filter") - } - parsedPolicies := make([]policy.StoragePolicy, len(storagePolicies)) for _, storagePolicyString := range storagePolicies { parsedPolicy, err := policy.ParseStoragePolicy(storagePolicyString) @@ -401,17 +386,8 @@ func RegisterStoragePolicyFilterFromTopicUpdate(csw consumerServiceWriter, spf t } func RegisterPercentageFilterFromFromTopicUpdate(csw consumerServiceWriter, pf topic.PercentageFilter) error { - - if pf == nil { - return errors.New("nil percentage filter") - } - percentage := pf.Percentage() - if percentage == 0 { - return errors.New("no percentage specified in filter") - } - csw.RegisterFilter(filter.NewPercentageFilter(percentage, producer.DynamicConfig)) return nil diff --git a/src/msg/producer/writer/writer_test.go b/src/msg/producer/writer/writer_test.go index 38dba8112a..de2b6a6ffd 100644 --- a/src/msg/producer/writer/writer_test.go +++ b/src/msg/producer/writer/writer_test.go @@ -850,3 +850,500 @@ func TestWriterNumShards(t *testing.T) { require.NoError(t, w.Init()) require.Equal(t, 2, int(w.NumShards())) } + +// Consumer service has no staticly configured filters, topic update comes in with dynamic filters. +// Expected: Dynamic filters override static and are the only filters present. +func TestTopicUpdateWithDynamicFiltersNoStaticFilters(t *testing.T) { + defer leaktest.Check(t)() + + ctrl := xtest.NewController(t) + defer ctrl.Finish() + + store := mem.NewStore() + cs := client.NewMockClient(ctrl) + cs.EXPECT().Store(gomock.Any()).Return(store, nil) + + ts, err := topic.NewService(topic.NewServiceOptions().SetConfigService(cs)) + require.NoError(t, err) + + opts := testOptions().SetTopicService(ts) + + sid1 := services.NewServiceID().SetName("s1") + + // NOTE: set these to actual values + percentageFilter := topic.NewPercentageFilter(50) + shardSetFilter := topic.NewShardSetFilter("") + storagePolicyFilter := topic.NewStoragePolicyFilter([]string{}) + + filterConfig := topic.NewFilterConfig().SetPercentageFilter(percentageFilter).SetShardSetFilter(shardSetFilter).SetStoragePolicyFilter(storagePolicyFilter) + + cs1 := topic.NewConsumerService().SetConsumptionType(topic.Replicated).SetServiceID(sid1).SetDynamicFilterConfigs(filterConfig) + + testTopic := topic.NewTopic(). + SetName(opts.TopicName()). + SetNumberOfShards(1). + SetConsumerServices([]topic.ConsumerService{cs1}) + _, err = ts.CheckAndSet(testTopic, kv.UninitializedVersion) + require.NoError(t, err) + + sd := services.NewMockServices(ctrl) + opts = opts.SetServiceDiscovery(sd) + ps1 := testPlacementService(store, sid1) + sd.EXPECT().PlacementService(sid1, gomock.Any()).Return(ps1, nil) + + p1 := placement.NewPlacement(). + SetInstances([]placement.Instance{ + placement.NewInstance(). + SetID("i1"). + SetEndpoint("i1"). + SetShards(shard.NewShards([]shard.Shard{ + shard.NewShard(0).SetState(shard.Available), + })), + }). + SetShards([]uint32{0}). + SetReplicaFactor(1). + SetIsSharded(true) + _, err = ps1.Set(p1) + require.NoError(t, err) + + w := NewWriter(opts).(*writer) + + called := atomic.NewInt32(0) + w.processFn = func(update interface{}) error { + called.Inc() + return w.process(update) + } + require.NoError(t, w.Init()) + require.Equal(t, 1, int(called.Load())) + require.Equal(t, 1, len(w.consumerServiceWriters)) + + csw, ok := w.consumerServiceWriters[cs1.ServiceID().String()] + require.True(t, ok) + + dataFilters := csw.GetDataFilters() + require.Equal(t, 4, len(dataFilters)) + + foundPercentageFilter := false + foundShardSetFilter := false + foundStoragePolicyFilter := false + foundAcceptAllFilter := false + + for _, filter := range dataFilters { + switch filter.Metadata.FilterType { + case producer.PercentageFilter: + foundPercentageFilter = true + require.Equal(t, filter.Metadata.SourceType, producer.DynamicConfig) + case producer.ShardSetFilter: + foundShardSetFilter = true + require.Equal(t, filter.Metadata.SourceType, producer.DynamicConfig) + case producer.StoragePolicyFilter: + foundStoragePolicyFilter = true + require.Equal(t, filter.Metadata.SourceType, producer.DynamicConfig) + case producer.AcceptAllFilter: + foundAcceptAllFilter = true + require.Equal(t, filter.Metadata.SourceType, producer.StaticConfig) + } + } + + require.True(t, foundPercentageFilter) + require.True(t, foundShardSetFilter) + require.True(t, foundStoragePolicyFilter) + require.True(t, foundAcceptAllFilter) + + defer csw.Close() + + w.Close() +} + +// Consumer service has staticly configured filters, topic update comes in with dynamic filters. +// Expected: Dynamic filters override static and are the only filters present. +func TestTopicUpdateWithDynamicFiltersHasStaticFilters(t *testing.T) { + defer leaktest.Check(t)() + + ctrl := xtest.NewController(t) + defer ctrl.Finish() + + store := mem.NewStore() + cs := client.NewMockClient(ctrl) + cs.EXPECT().Store(gomock.Any()).Return(store, nil) + + ts, err := topic.NewService(topic.NewServiceOptions().SetConfigService(cs)) + require.NoError(t, err) + + opts := testOptions().SetTopicService(ts) + + sid1 := services.NewServiceID().SetName("s1") + + // NOTE: set these to actual values + percentageFilter := topic.NewPercentageFilter(50) + shardSetFilter := topic.NewShardSetFilter("") + storagePolicyFilter := topic.NewStoragePolicyFilter([]string{}) + + filterConfig := topic.NewFilterConfig().SetPercentageFilter(percentageFilter).SetShardSetFilter(shardSetFilter).SetStoragePolicyFilter(storagePolicyFilter) + + cs1 := topic.NewConsumerService().SetConsumptionType(topic.Replicated).SetServiceID(sid1).SetDynamicFilterConfigs(filterConfig) + + testTopic := topic.NewTopic(). + SetName(opts.TopicName()). + SetNumberOfShards(1). + SetConsumerServices([]topic.ConsumerService{cs1}) + _, err = ts.CheckAndSet(testTopic, kv.UninitializedVersion) + require.NoError(t, err) + + sd := services.NewMockServices(ctrl) + opts = opts.SetServiceDiscovery(sd) + ps1 := testPlacementService(store, sid1) + sd.EXPECT().PlacementService(sid1, gomock.Any()).Return(ps1, nil) + + p1 := placement.NewPlacement(). + SetInstances([]placement.Instance{ + placement.NewInstance(). + SetID("i1"). + SetEndpoint("i1"). + SetShards(shard.NewShards([]shard.Shard{ + shard.NewShard(0).SetState(shard.Available), + })), + }). + SetShards([]uint32{0}). + SetReplicaFactor(1). + SetIsSharded(true) + _, err = ps1.Set(p1) + require.NoError(t, err) + + w := NewWriter(opts).(*writer) + + // Register static filters for the consumer service. + w.RegisterFilter(sid1, producer.NewFilterFunc(func(producer.Message) bool { return true }, producer.PercentageFilter, producer.StaticConfig)) + w.RegisterFilter(sid1, producer.NewFilterFunc(func(producer.Message) bool { return true }, producer.StoragePolicyFilter, producer.StaticConfig)) + w.RegisterFilter(sid1, producer.NewFilterFunc(func(producer.Message) bool { return true }, producer.ShardSetFilter, producer.StaticConfig)) + + called := atomic.NewInt32(0) + w.processFn = func(update interface{}) error { + called.Inc() + return w.process(update) + } + require.NoError(t, w.Init()) + require.Equal(t, 1, int(called.Load())) + require.Equal(t, 1, len(w.consumerServiceWriters)) + + csw, ok := w.consumerServiceWriters[cs1.ServiceID().String()] + require.True(t, ok) + + dataFilters := csw.GetDataFilters() + require.Equal(t, 4, len(dataFilters)) + + foundPercentageFilter := false + foundShardSetFilter := false + foundStoragePolicyFilter := false + foundAcceptAllFilter := false + + for _, filter := range dataFilters { + switch filter.Metadata.FilterType { + case producer.PercentageFilter: + foundPercentageFilter = true + require.Equal(t, filter.Metadata.SourceType, producer.DynamicConfig) + case producer.ShardSetFilter: + foundShardSetFilter = true + require.Equal(t, filter.Metadata.SourceType, producer.DynamicConfig) + case producer.StoragePolicyFilter: + foundStoragePolicyFilter = true + require.Equal(t, filter.Metadata.SourceType, producer.DynamicConfig) + case producer.AcceptAllFilter: + foundAcceptAllFilter = true + require.Equal(t, filter.Metadata.SourceType, producer.StaticConfig) + } + } + + require.True(t, foundPercentageFilter) + require.True(t, foundShardSetFilter) + require.True(t, foundStoragePolicyFilter) + require.True(t, foundAcceptAllFilter) + + defer csw.Close() + + w.Close() +} + +func TestTopicUpdateNoDynamicFiltersHasStaticFilters(t *testing.T) { + defer leaktest.Check(t)() + + ctrl := xtest.NewController(t) + defer ctrl.Finish() + + store := mem.NewStore() + cs := client.NewMockClient(ctrl) + cs.EXPECT().Store(gomock.Any()).Return(store, nil) + + ts, err := topic.NewService(topic.NewServiceOptions().SetConfigService(cs)) + require.NoError(t, err) + + opts := testOptions().SetTopicService(ts) + + sid1 := services.NewServiceID().SetName("s1") + + cs1 := topic.NewConsumerService().SetConsumptionType(topic.Replicated).SetServiceID(sid1) + + testTopic := topic.NewTopic(). + SetName(opts.TopicName()). + SetNumberOfShards(1). + SetConsumerServices([]topic.ConsumerService{cs1}) + _, err = ts.CheckAndSet(testTopic, kv.UninitializedVersion) + require.NoError(t, err) + + sd := services.NewMockServices(ctrl) + opts = opts.SetServiceDiscovery(sd) + ps1 := testPlacementService(store, sid1) + sd.EXPECT().PlacementService(sid1, gomock.Any()).Return(ps1, nil) + + p1 := placement.NewPlacement(). + SetInstances([]placement.Instance{ + placement.NewInstance(). + SetID("i1"). + SetEndpoint("i1"). + SetShards(shard.NewShards([]shard.Shard{ + shard.NewShard(0).SetState(shard.Available), + })), + }). + SetShards([]uint32{0}). + SetReplicaFactor(1). + SetIsSharded(true) + _, err = ps1.Set(p1) + require.NoError(t, err) + + w := NewWriter(opts).(*writer) + + // Register static filters for the consumer service. + w.RegisterFilter(sid1, producer.NewFilterFunc(func(producer.Message) bool { return true }, producer.PercentageFilter, producer.StaticConfig)) + w.RegisterFilter(sid1, producer.NewFilterFunc(func(producer.Message) bool { return true }, producer.StoragePolicyFilter, producer.StaticConfig)) + w.RegisterFilter(sid1, producer.NewFilterFunc(func(producer.Message) bool { return true }, producer.ShardSetFilter, producer.StaticConfig)) + + called := atomic.NewInt32(0) + w.processFn = func(update interface{}) error { + called.Inc() + return w.process(update) + } + require.NoError(t, w.Init()) + require.Equal(t, 1, int(called.Load())) + require.Equal(t, 1, len(w.consumerServiceWriters)) + + csw, ok := w.consumerServiceWriters[cs1.ServiceID().String()] + require.True(t, ok) + + dataFilters := csw.GetDataFilters() + require.Equal(t, 4, len(dataFilters)) + + foundPercentageFilter := false + foundShardSetFilter := false + foundStoragePolicyFilter := false + foundAcceptAllFilter := false + + for _, filter := range dataFilters { + switch filter.Metadata.FilterType { + case producer.PercentageFilter: + foundPercentageFilter = true + require.Equal(t, filter.Metadata.SourceType, producer.StaticConfig) + case producer.ShardSetFilter: + foundShardSetFilter = true + require.Equal(t, filter.Metadata.SourceType, producer.StaticConfig) + case producer.StoragePolicyFilter: + foundStoragePolicyFilter = true + require.Equal(t, filter.Metadata.SourceType, producer.StaticConfig) + case producer.AcceptAllFilter: + foundAcceptAllFilter = true + require.Equal(t, filter.Metadata.SourceType, producer.StaticConfig) + } + } + + require.True(t, foundPercentageFilter) + require.True(t, foundShardSetFilter) + require.True(t, foundStoragePolicyFilter) + require.True(t, foundAcceptAllFilter) + + defer csw.Close() + + w.Close() +} + +func TestTopicUpdateWithDynamicFilter2Updates(t *testing.T) { + defer leaktest.Check(t)() + + ctrl := xtest.NewController(t) + defer ctrl.Finish() + + store := mem.NewStore() + cs := client.NewMockClient(ctrl) + cs.EXPECT().Store(gomock.Any()).Return(store, nil) + + ts, err := topic.NewService(topic.NewServiceOptions().SetConfigService(cs)) + require.NoError(t, err) + + opts := testOptions().SetTopicService(ts) + + sid1 := services.NewServiceID().SetName("s1") + + // NOTE: set these to actual values + percentageFilter := topic.NewPercentageFilter(50) + shardSetFilter := topic.NewShardSetFilter("") + storagePolicyFilter := topic.NewStoragePolicyFilter([]string{}) + + filterConfig := topic.NewFilterConfig().SetPercentageFilter(percentageFilter).SetShardSetFilter(shardSetFilter).SetStoragePolicyFilter(storagePolicyFilter) + + cs1 := topic.NewConsumerService().SetConsumptionType(topic.Replicated).SetServiceID(sid1).SetDynamicFilterConfigs(filterConfig) + + testTopic := topic.NewTopic(). + SetName(opts.TopicName()). + SetNumberOfShards(1). + SetConsumerServices([]topic.ConsumerService{cs1}) + _, err = ts.CheckAndSet(testTopic, kv.UninitializedVersion) + require.NoError(t, err) + + sd := services.NewMockServices(ctrl) + opts = opts.SetServiceDiscovery(sd) + ps1 := testPlacementService(store, sid1) + sd.EXPECT().PlacementService(sid1, gomock.Any()).Return(ps1, nil) + + p1 := placement.NewPlacement(). + SetInstances([]placement.Instance{ + placement.NewInstance(). + SetID("i1"). + SetEndpoint("i1"). + SetShards(shard.NewShards([]shard.Shard{ + shard.NewShard(0).SetState(shard.Available), + })), + }). + SetShards([]uint32{0}). + SetReplicaFactor(1). + SetIsSharded(true) + _, err = ps1.Set(p1) + require.NoError(t, err) + + w := NewWriter(opts).(*writer) + + called := atomic.NewInt32(0) + w.processFn = func(update interface{}) error { + called.Inc() + return w.process(update) + } + require.NoError(t, w.Init()) + require.Equal(t, 1, int(called.Load())) + require.Equal(t, 1, len(w.consumerServiceWriters)) + + csw, ok := w.consumerServiceWriters[cs1.ServiceID().String()] + require.True(t, ok) + + dataFilters := csw.GetDataFilters() + require.Equal(t, 4, len(dataFilters)) + + foundPercentageFilter := false + foundShardSetFilter := false + foundStoragePolicyFilter := false + foundAcceptAllFilter := false + + for _, filter := range dataFilters { + switch filter.Metadata.FilterType { + case producer.PercentageFilter: + foundPercentageFilter = true + require.Equal(t, filter.Metadata.SourceType, producer.DynamicConfig) + case producer.ShardSetFilter: + foundShardSetFilter = true + require.Equal(t, filter.Metadata.SourceType, producer.DynamicConfig) + case producer.StoragePolicyFilter: + foundStoragePolicyFilter = true + require.Equal(t, filter.Metadata.SourceType, producer.DynamicConfig) + case producer.AcceptAllFilter: + foundAcceptAllFilter = true + require.Equal(t, filter.Metadata.SourceType, producer.StaticConfig) + } + } + + require.True(t, foundPercentageFilter) + require.True(t, foundShardSetFilter) + require.True(t, foundStoragePolicyFilter) + require.True(t, foundAcceptAllFilter) + + // change the dynamic filters for topic + cs1.SetDynamicFilterConfigs(topic.NewFilterConfig().SetPercentageFilter(topic.NewPercentageFilter(75))) + + testTopic = testTopic. + SetConsumerServices([]topic.ConsumerService{cs1}). + SetVersion(1) + _, err = ts.CheckAndSet(testTopic, 1) + require.NoError(t, err) + + for called.Load() != 2 { + time.Sleep(50 * time.Millisecond) + } + + csw, ok = w.consumerServiceWriters[cs1.ServiceID().String()] + require.True(t, ok) + + require.Equal(t, 1, len(dataFilters)) + require.Equal(t, producer.PercentageFilter, dataFilters[0].Metadata.FilterType) + require.Equal(t, producer.DynamicConfig, dataFilters[0].Metadata.SourceType) + + defer csw.Close() + + w.Close() +} + +func TestTopicUpdateWithDynamicFiltersInvalidFilter(t *testing.T) { + defer leaktest.Check(t)() + + ctrl := xtest.NewController(t) + defer ctrl.Finish() + + store := mem.NewStore() + cs := client.NewMockClient(ctrl) + cs.EXPECT().Store(gomock.Any()).Return(store, nil) + + ts, err := topic.NewService(topic.NewServiceOptions().SetConfigService(cs)) + require.NoError(t, err) + + opts := testOptions().SetTopicService(ts) + + sid1 := services.NewServiceID().SetName("s1") + + shardSetFilter := topic.NewShardSetFilter("") + + filterConfig := topic.NewFilterConfig().SetShardSetFilter(shardSetFilter) + + cs1 := topic.NewConsumerService().SetConsumptionType(topic.Replicated).SetServiceID(sid1).SetDynamicFilterConfigs(filterConfig) + + testTopic := topic.NewTopic(). + SetName(opts.TopicName()). + SetNumberOfShards(1). + SetConsumerServices([]topic.ConsumerService{cs1}) + _, err = ts.CheckAndSet(testTopic, kv.UninitializedVersion) + require.NoError(t, err) + + sd := services.NewMockServices(ctrl) + opts = opts.SetServiceDiscovery(sd) + ps1 := testPlacementService(store, sid1) + sd.EXPECT().PlacementService(sid1, gomock.Any()).Return(ps1, nil) + + p1 := placement.NewPlacement(). + SetInstances([]placement.Instance{ + placement.NewInstance(). + SetID("i1"). + SetEndpoint("i1"). + SetShards(shard.NewShards([]shard.Shard{ + shard.NewShard(0).SetState(shard.Available), + })), + }). + SetShards([]uint32{0}). + SetReplicaFactor(1). + SetIsSharded(true) + _, err = ps1.Set(p1) + require.NoError(t, err) + + w := NewWriter(opts).(*writer) + + called := atomic.NewInt32(0) + w.processFn = func(update interface{}) error { + called.Inc() + return w.process(update) + } + require.Error(t, w.Init()) + + w.Close() +} diff --git a/src/msg/topic/topic.go b/src/msg/topic/topic.go index e82f20d25b..4e026a1f4e 100644 --- a/src/msg/topic/topic.go +++ b/src/msg/topic/topic.go @@ -163,7 +163,6 @@ func (t *topic) String() string { buf.WriteString("\tconsumerServices: {\n") } for _, cs := range t.consumerServices { - // NOTE: update to print filters? buf.WriteString(fmt.Sprintf("\t\t%s\n", cs.String())) } if len(t.consumerServices) > 0 { @@ -187,7 +186,36 @@ func (t *topic) Validate() error { return fmt.Errorf("invalid topic: duplicated consumer %s", cs.ServiceID().String()) } uniqConsumers[cs.ServiceID().String()] = struct{}{} + + filterConfig := cs.DynamicFilterConfigs() + + if filterConfig != nil { + shardSetFilter := filterConfig.ShardSetFilter() + if shardSetFilter != nil { + shardSet := shardSetFilter.ShardSet() + if shardSet == "" { + return fmt.Errorf("invalid topic: empty shard set in filter for consumer %s", cs.ServiceID().String()) + } + } + + percentageFilter := filterConfig.PercentageFilter() + if percentageFilter != nil { + percentage := percentageFilter.Percentage() + if percentage < 0 || percentage > 1 { + return fmt.Errorf("invalid topic: invalid percentage in filter for consumer %s", cs.ServiceID().String()) + } + } + + storagePolicyFilter := filterConfig.StoragePolicyFilter() + if storagePolicyFilter != nil { + storagePolicies := storagePolicyFilter.StoragePolicies() + if len(storagePolicies) == 0 { + return fmt.Errorf("invalid topic: empty storage policy filter for consumer %s", cs.ServiceID().String()) + } + } + } } + return nil } @@ -209,20 +237,44 @@ func ToProto(t Topic) (*topicpb.Topic, error) { }, nil } -type percentageFilter struct { - percentage float64 +type filterConfig struct { + shardSetFilterConfig *shardSetFilter + percentageFilterConfig *percentageFilter + storagePolicyFilterConfig *storagePolicyFilter } -func (filter *percentageFilter) Percentage() float64 { - return filter.percentage +func NewFilterConfig() FilterConfig { + return new(filterConfig) } -type storagePolicyFilter struct { - storagePolicies []string +func (fc *filterConfig) ShardSetFilter() ShardSetFilter { + return fc.shardSetFilterConfig } -func (filter *storagePolicyFilter) StoragePolicies() []string { - return filter.storagePolicies +func (fc *filterConfig) SetShardSetFilter(value ShardSetFilter) FilterConfig { + newfc := *fc + newfc.shardSetFilterConfig = value.(*shardSetFilter) + return &newfc +} + +func (fc *filterConfig) PercentageFilter() PercentageFilter { + return fc.percentageFilterConfig +} + +func (fc *filterConfig) SetPercentageFilter(value PercentageFilter) FilterConfig { + newfc := *fc + newfc.percentageFilterConfig = value.(*percentageFilter) + return &newfc +} + +func (fc *filterConfig) StoragePolicyFilter() StoragePolicyFilter { + return fc.storagePolicyFilterConfig +} + +func (fc *filterConfig) SetStoragePolicyFilter(value StoragePolicyFilter) FilterConfig { + newfc := *fc + newfc.storagePolicyFilterConfig = value.(*storagePolicyFilter) + return &newfc } type shardSetFilter struct { @@ -233,22 +285,32 @@ func (filter *shardSetFilter) ShardSet() string { return filter.shardSet } -type filterConfig struct { - shardSetFilterConfig *shardSetFilter - percentageFilterConfig *percentageFilter - storagePolicyFilterConfig *storagePolicyFilter +func NewShardSetFilter(shardSet string) ShardSetFilter { + return &shardSetFilter{shardSet: shardSet} } -func (fc *filterConfig) ShardSetFilter() ShardSetFilter { - return fc.shardSetFilterConfig +type percentageFilter struct { + percentage float64 } -func (fc *filterConfig) PercentageFilter() PercentageFilter { - return fc.percentageFilterConfig +func NewPercentageFilter(percentage float64) PercentageFilter { + return &percentageFilter{percentage: percentage} } -func (fc *filterConfig) StoragePolicyFilter() StoragePolicyFilter { - return fc.storagePolicyFilterConfig +func (filter *percentageFilter) Percentage() float64 { + return filter.percentage +} + +type storagePolicyFilter struct { + storagePolicies []string +} + +func NewStoragePolicyFilter(storagePolicies []string) StoragePolicyFilter { + return &storagePolicyFilter{storagePolicies: storagePolicies} +} + +func (filter *storagePolicyFilter) StoragePolicies() []string { + return filter.storagePolicies } type consumerService struct { @@ -347,6 +409,17 @@ func (cs *consumerService) String() string { if cs.ttlNanos != 0 { buf.WriteString(fmt.Sprintf(", ttl: %v", time.Duration(cs.ttlNanos))) } + if cs.filterConfigs != nil { + if cs.filterConfigs.shardSetFilterConfig != nil { + buf.WriteString(fmt.Sprintf(", shard set filter: %s", cs.filterConfigs.shardSetFilterConfig.shardSet)) + } + if cs.filterConfigs.percentageFilterConfig != nil { + buf.WriteString(fmt.Sprintf(", percentage filter: %v", cs.filterConfigs.percentageFilterConfig.percentage)) + } + if cs.filterConfigs.storagePolicyFilterConfig != nil { + buf.WriteString(fmt.Sprintf(", storage policy filter: %v", cs.filterConfigs.storagePolicyFilterConfig.storagePolicies)) + } + } buf.WriteString("}") return buf.String() } diff --git a/src/msg/topic/topic_test.go b/src/msg/topic/topic_test.go index 776e2d9b9f..f6febbc4df 100644 --- a/src/msg/topic/topic_test.go +++ b/src/msg/topic/topic_test.go @@ -145,6 +145,12 @@ func TestTopicUpdateConsumer(t *testing.T) { } func TestTopicString(t *testing.T) { + percentageFilter := NewPercentageFilter(0.5) + shardSetFilter := NewShardSetFilter("[10..23]") + storagePolicyFilter := NewStoragePolicyFilter([]string{"1m:40d"}) + + filterConfig := NewFilterConfig().SetPercentageFilter(percentageFilter).SetShardSetFilter(shardSetFilter).SetStoragePolicyFilter(storagePolicyFilter) + cs1 := NewConsumerService(). SetConsumptionType(Shared). SetServiceID(services.NewServiceID(). @@ -159,7 +165,8 @@ func TestTopicString(t *testing.T) { SetEnvironment("env2"). SetZone("zone2"), ). - SetMessageTTLNanos(int64(time.Minute)) + SetMessageTTLNanos(int64(time.Minute)). + SetDynamicFilterConfigs(filterConfig) tpc := NewTopic(). SetName("testName"). SetNumberOfShards(1024). @@ -174,7 +181,7 @@ func TestTopicString(t *testing.T) { numOfShards: 1024 consumerServices: { {service: [name: s1, env: env1, zone: zone1], consumption type: shared} - {service: [name: s2, env: env2, zone: zone2], consumption type: shared, ttl: 1m0s} + {service: [name: s2, env: env2, zone: zone2], consumption type: shared, ttl: 1m0s, shard set filter: [10..23], storage policy filter: [1m:40d], percentage filter: 0.5} } } ` @@ -222,6 +229,30 @@ func TestTopicValidation(t *testing.T) { }) err = topic.Validate() require.NoError(t, err) + + topic = topic.SetConsumerServices([]ConsumerService{ + cs1.SetDynamicFilterConfigs(NewFilterConfig().SetPercentageFilter(NewPercentageFilter(0.4)).SetStoragePolicyFilter(NewStoragePolicyFilter([]string{"1m:40d"})).SetShardSetFilter(NewShardSetFilter("[10..23]"))), + }) + err = topic.Validate() + require.NoError(t, err) + + topic = topic.SetConsumerServices([]ConsumerService{ + cs1.SetDynamicFilterConfigs(NewFilterConfig().SetPercentageFilter(NewPercentageFilter(9999))), + }) + err = topic.Validate() + require.Contains(t, err.Error(), "invalid percentage") + + topic = topic.SetConsumerServices([]ConsumerService{ + cs1.SetDynamicFilterConfigs(NewFilterConfig().SetShardSetFilter(NewShardSetFilter(""))), + }) + err = topic.Validate() + require.Contains(t, err.Error(), "empty shard set") + + topic = topic.SetConsumerServices([]ConsumerService{ + cs1.SetDynamicFilterConfigs(NewFilterConfig().SetStoragePolicyFilter(NewStoragePolicyFilter([]string{}))), + }) + err = topic.Validate() + require.Contains(t, err.Error(), "empty storage policy") } func TestConsumerService(t *testing.T) { diff --git a/src/msg/topic/types.go b/src/msg/topic/types.go index a15322fb5d..0d8bb43f83 100644 --- a/src/msg/topic/types.go +++ b/src/msg/topic/types.go @@ -102,11 +102,20 @@ type FilterConfig interface { // StoragePolicyFilter returns the storage policy data filter of the consumer service. StoragePolicyFilter() StoragePolicyFilter + // SetStoragePolicyFilter sets the storage policy data filter of the consumer service. + SetStoragePolicyFilter(value StoragePolicyFilter) FilterConfig + // PercentageFilter returns the percentage data filter of the consumer service. PercentageFilter() PercentageFilter + // SetPercentageFilter sets the percentage data filter of the consumer service. + SetPercentageFilter(value PercentageFilter) FilterConfig + // ShardSetFilter returns the shard set data filter of the consumer service. ShardSetFilter() ShardSetFilter + + // SetShardSetFilter sets the shard set data filter of the consumer service. + SetShardSetFilter(value ShardSetFilter) FilterConfig } type PercentageFilter interface { From 906cd73f369f4b10aa44cbb00b144d1094eef8ee Mon Sep 17 00:00:00 2001 From: Adam Jean-Laurent Date: Wed, 25 Sep 2024 09:44:10 -0400 Subject: [PATCH 07/18] fix error in csw mock --- .../writer/consumer_service_writer_mock.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/msg/producer/writer/consumer_service_writer_mock.go b/src/msg/producer/writer/consumer_service_writer_mock.go index 49c8d5218f..9e6d2358b6 100644 --- a/src/msg/producer/writer/consumer_service_writer_mock.go +++ b/src/msg/producer/writer/consumer_service_writer_mock.go @@ -105,6 +105,20 @@ func (mr *MockconsumerServiceWriterMockRecorder) SetMessageTTLNanos(value interf return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetMessageTTLNanos", reflect.TypeOf((*MockconsumerServiceWriter)(nil).SetMessageTTLNanos), value) } +// GetDataFilter mocks base method. +func (m *MockconsumerServiceWriter) GetDataFilters() []producer.FilterFunc { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetDataFilters") + + ret0, _ := ret[0].([]producer.FilterFunc) + return ret0 +} + +// GetDataFilters indicates an expected call of GetDataFilters. +func (mr *MockconsumerServiceWriterMockRecorder) GetDataFilters() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetDataFilters", reflect.TypeOf((*MockconsumerServiceWriter)(nil).GetDataFilters())) +} // SetShardSet mocks base method. func (m *MockconsumerServiceWriter) SetShardSet(value string) { m.ctrl.T.Helper() From 709cf8e14da6c3f1befd14999d1d384932ed2ce0 Mon Sep 17 00:00:00 2001 From: Adam Jean-Laurent Date: Wed, 25 Sep 2024 10:28:24 -0400 Subject: [PATCH 08/18] cleanup nits --- scripts/proto-gen.sh | 3 --- src/aggregator/aggregator/handler/config.go | 1 - .../handler/filter/percentageFilter_test.go | 2 +- src/msg/producer/ref_counted.go | 2 +- .../writer/consumer_service_writer.go | 11 +------- .../writer/consumer_service_writer_mock.go | 13 +-------- .../writer/consumer_service_writer_test.go | 14 ++++++---- src/msg/producer/writer/writer.go | 4 +-- src/msg/producer/writer/writer_test.go | 27 ++++++++++--------- src/msg/topic/topic.go | 24 +++++++---------- src/msg/topic/topic_test.go | 8 +++++- 11 files changed, 47 insertions(+), 62 deletions(-) diff --git a/scripts/proto-gen.sh b/scripts/proto-gen.sh index 597ca817cb..d108b15fed 100755 --- a/scripts/proto-gen.sh +++ b/scripts/proto-gen.sh @@ -17,13 +17,10 @@ if [[ -n "$BUILDKITE" ]]; then fi PROTO_SRC=$1 -GOPATH="/home/user/Uber/gocode" for i in "${GOPATH}/src/${PROTO_SRC}"/*; do - echo $i if ! [ -d $i ]; then continue fi - echo $i if ls $i/*.proto > /dev/null 2>&1; then proto_files=$(ls $i/*.proto | sed -e "s@${GOPATH}@@g") diff --git a/src/aggregator/aggregator/handler/config.go b/src/aggregator/aggregator/handler/config.go index 943a19dd02..bcd40092cc 100644 --- a/src/aggregator/aggregator/handler/config.go +++ b/src/aggregator/aggregator/handler/config.go @@ -186,7 +186,6 @@ func (c *DynamicBackendConfiguration) newProtobufHandler( return nil, err } logger := instrumentOpts.Logger() - // NOTE: where we setup the static filters for _, filter := range c.ShardSetFilters { sid, f := filter.NewConsumerServiceFilter() p.RegisterFilter(sid, f) diff --git a/src/aggregator/aggregator/handler/filter/percentageFilter_test.go b/src/aggregator/aggregator/handler/filter/percentageFilter_test.go index c17c9397cd..3093ec210e 100644 --- a/src/aggregator/aggregator/handler/filter/percentageFilter_test.go +++ b/src/aggregator/aggregator/handler/filter/percentageFilter_test.go @@ -37,7 +37,7 @@ func TestPercentageFilter(t *testing.T) { f0 := NewPercentageFilter(0, producer.StaticConfig) require.False(t, f0.Function((mm))) - f1 := NewPercentageFilter(1, producer.StaticConfig) + f1 := NewPercentageFilter(1, producer.DynamicConfig) require.True(t, f1.Function(mm)) } diff --git a/src/msg/producer/ref_counted.go b/src/msg/producer/ref_counted.go index 474a3efa30..85679f529a 100644 --- a/src/msg/producer/ref_counted.go +++ b/src/msg/producer/ref_counted.go @@ -51,7 +51,7 @@ func NewRefCountedMessage(m Message, fn OnFinalizeFn) *RefCountedMessage { } } -// note: Not a fan at all of passing function pointers here, but passing ConsumerServiceWriterMetrics to RefCountedMessage.Accept would cause a cyclical depdenency. ¯\_(ツ)_/¯ +// Not a fan of passing function pointers here, but passing ConsumerServiceWriterMetrics to RefCountedMessage.Accept will cause a cyclical depdenency. ¯\_(ツ)_/¯ type filterAcceptCounterFn func(metadata FilterFuncMetadata) tally.Counter type filterDenyCounterFn func(metadata FilterFuncMetadata) tally.Counter diff --git a/src/msg/producer/writer/consumer_service_writer.go b/src/msg/producer/writer/consumer_service_writer.go index 0bea78f090..b1d58c5985 100644 --- a/src/msg/producer/writer/consumer_service_writer.go +++ b/src/msg/producer/writer/consumer_service_writer.go @@ -89,7 +89,6 @@ type consumerServiceWriter interface { } type ConsumerServiceWriterMetrics struct { - scope tally.Scope placementError tally.Counter placementUpdate tally.Counter queueSize tally.Gauge @@ -99,6 +98,7 @@ type ConsumerServiceWriterMetrics struct { filterAcceptedGranularLock sync.RWMutex filterNotAcceptedGranular map[string]tally.Counter filterNotAcceptedGranularLock sync.RWMutex + scope tally.Scope } func (cswm *ConsumerServiceWriterMetrics) getGranularFilterCounterMapKey(metadata producer.FilterFuncMetadata) string { @@ -178,7 +178,6 @@ type consumerServiceWriterImpl struct { wg sync.WaitGroup m ConsumerServiceWriterMetrics cm consumerWriterMetrics - shardSet string processFn watch.ProcessFn } @@ -394,14 +393,6 @@ func (w *consumerServiceWriterImpl) SetMessageTTLNanos(value int64) { } } -func (w *consumerServiceWriterImpl) GetShardSet() string { - return w.shardSet -} - -func (w *consumerServiceWriterImpl) SetShardSet(value string) { - w.shardSet = value -} - func (w *consumerServiceWriterImpl) RegisterFilter(filter producer.FilterFunc) { w.Lock() w.dataFilters = append(w.dataFilters, filter) diff --git a/src/msg/producer/writer/consumer_service_writer_mock.go b/src/msg/producer/writer/consumer_service_writer_mock.go index 9e6d2358b6..1a91d1a11e 100644 --- a/src/msg/producer/writer/consumer_service_writer_mock.go +++ b/src/msg/producer/writer/consumer_service_writer_mock.go @@ -114,22 +114,11 @@ func (m *MockconsumerServiceWriter) GetDataFilters() []producer.FilterFunc { return ret0 } -// GetDataFilters indicates an expected call of GetDataFilters. +// GetDataFilters indicates an expected call of GetDataFilters func (mr *MockconsumerServiceWriterMockRecorder) GetDataFilters() *gomock.Call { mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetDataFilters", reflect.TypeOf((*MockconsumerServiceWriter)(nil).GetDataFilters())) } -// SetShardSet mocks base method. -func (m *MockconsumerServiceWriter) SetShardSet(value string) { - m.ctrl.T.Helper() - m.ctrl.Call(m, "SetShardSet", value) -} - -// SetShardSet indicates an expected call of SetShardSet. -func (mr *MockconsumerServiceWriterMockRecorder) SetShardSet(value interface{}) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetMessageTTLNanos", reflect.TypeOf((*MockconsumerServiceWriter)(nil).SetShardSet), value) -} // UnregisterFilters mocks base method. func (m *MockconsumerServiceWriter) UnregisterFilters() { diff --git a/src/msg/producer/writer/consumer_service_writer_test.go b/src/msg/producer/writer/consumer_service_writer_test.go index 78c13c37e7..4a9a8237a0 100644 --- a/src/msg/producer/writer/consumer_service_writer_test.go +++ b/src/msg/producer/writer/consumer_service_writer_test.go @@ -703,27 +703,31 @@ func TestConsumerServiceWriterMetrics(t *testing.T) { _, ok := m.filterAcceptedGranular[accepetKey] require.True(t, ok) - _, ok = m.filterAcceptedGranular[notAcceptKey] + _, ok = m.filterNotAcceptedGranular[notAcceptKey] require.True(t, ok) - gotValue := false + gotAcceptedValue := false + gotNotAcceptedValue := false + snap := testScope.Snapshot() for _, c := range snap.Counters() { if c.Name() == "test.filter-accepted-granular" { require.Equal(t, "config-source", c.Tags()["DynamicConfig"]) require.Equal(t, "filter-type", c.Tags()["ShardSetFilter"]) require.Equal(t, int64(1), c.Value()) - gotValue = true + gotAcceptedValue = true } if c.Name() == "test.filter-not-accepted-granular" { require.Equal(t, "config-source", c.Tags()["DynamicConfig"]) require.Equal(t, "filter-type", c.Tags()["PercentageFilter"]) require.Equal(t, int64(1), c.Value()) - gotValue = true + gotNotAcceptedValue = true } } - require.True(t, gotValue) + + require.True(t, gotAcceptedValue) + require.True(t, gotNotAcceptedValue) } func testPlacementService(store kv.Store, sid services.ServiceID) placement.Service { diff --git a/src/msg/producer/writer/writer.go b/src/msg/producer/writer/writer.go index 2fcc37b7ee..dd76543d7d 100644 --- a/src/msg/producer/writer/writer.go +++ b/src/msg/producer/writer/writer.go @@ -181,7 +181,6 @@ func (w *writer) process(update interface{}) error { csw, ok := w.consumerServiceWriters[key] if ok { csw.SetMessageTTLNanos(cs.MessageTTLNanos()) - newConsumerServiceWriters[key] = csw if cs.DynamicFilterConfigs() != nil { w.logger.Debug("registering dynamic filters", zap.String("consumer-service", cs.String())) @@ -197,6 +196,8 @@ func (w *writer) process(update interface{}) error { } } + newConsumerServiceWriters[key] = csw + continue } scope := iOpts.MetricsScope().Tagged(map[string]string{ @@ -235,7 +236,6 @@ func (w *writer) process(update interface{}) error { continue } csw.SetMessageTTLNanos(cs.MessageTTLNanos()) - newConsumerServiceWriters[key] = csw w.logger.Info("initialized consumer service writer", zap.String("writer", cs.String())) } diff --git a/src/msg/producer/writer/writer_test.go b/src/msg/producer/writer/writer_test.go index de2b6a6ffd..6a735167a8 100644 --- a/src/msg/producer/writer/writer_test.go +++ b/src/msg/producer/writer/writer_test.go @@ -852,7 +852,7 @@ func TestWriterNumShards(t *testing.T) { } // Consumer service has no staticly configured filters, topic update comes in with dynamic filters. -// Expected: Dynamic filters override static and are the only filters present. +// Expected: Dynamic filters present on csw. func TestTopicUpdateWithDynamicFiltersNoStaticFilters(t *testing.T) { defer leaktest.Check(t)() @@ -870,10 +870,9 @@ func TestTopicUpdateWithDynamicFiltersNoStaticFilters(t *testing.T) { sid1 := services.NewServiceID().SetName("s1") - // NOTE: set these to actual values percentageFilter := topic.NewPercentageFilter(50) - shardSetFilter := topic.NewShardSetFilter("") - storagePolicyFilter := topic.NewStoragePolicyFilter([]string{}) + shardSetFilter := topic.NewShardSetFilter("[1..5]") + storagePolicyFilter := topic.NewStoragePolicyFilter([]string{"1m:40d"}) filterConfig := topic.NewFilterConfig().SetPercentageFilter(percentageFilter).SetShardSetFilter(shardSetFilter).SetStoragePolicyFilter(storagePolicyFilter) @@ -956,7 +955,7 @@ func TestTopicUpdateWithDynamicFiltersNoStaticFilters(t *testing.T) { } // Consumer service has staticly configured filters, topic update comes in with dynamic filters. -// Expected: Dynamic filters override static and are the only filters present. +// Expected: Dynamic filters override static and are the only filters present on csw. func TestTopicUpdateWithDynamicFiltersHasStaticFilters(t *testing.T) { defer leaktest.Check(t)() @@ -974,10 +973,9 @@ func TestTopicUpdateWithDynamicFiltersHasStaticFilters(t *testing.T) { sid1 := services.NewServiceID().SetName("s1") - // NOTE: set these to actual values percentageFilter := topic.NewPercentageFilter(50) - shardSetFilter := topic.NewShardSetFilter("") - storagePolicyFilter := topic.NewStoragePolicyFilter([]string{}) + shardSetFilter := topic.NewShardSetFilter("[1..5]") + storagePolicyFilter := topic.NewStoragePolicyFilter([]string{"1m:40d"}) filterConfig := topic.NewFilterConfig().SetPercentageFilter(percentageFilter).SetShardSetFilter(shardSetFilter).SetStoragePolicyFilter(storagePolicyFilter) @@ -1064,6 +1062,8 @@ func TestTopicUpdateWithDynamicFiltersHasStaticFilters(t *testing.T) { w.Close() } +// Consumer service has staticly configured filters, topic update comes in with no dynamic filters. +// Expected: Static filters remain on the csw. func TestTopicUpdateNoDynamicFiltersHasStaticFilters(t *testing.T) { defer leaktest.Check(t)() @@ -1164,6 +1164,8 @@ func TestTopicUpdateNoDynamicFiltersHasStaticFilters(t *testing.T) { w.Close() } +// Consumer service has no static filters, topic update comes adding dynamic filters. Another topic update comes in changing the dynamic filters. +// Expected: Dynamic filters are updated on csw. func TestTopicUpdateWithDynamicFilter2Updates(t *testing.T) { defer leaktest.Check(t)() @@ -1181,10 +1183,9 @@ func TestTopicUpdateWithDynamicFilter2Updates(t *testing.T) { sid1 := services.NewServiceID().SetName("s1") - // NOTE: set these to actual values percentageFilter := topic.NewPercentageFilter(50) - shardSetFilter := topic.NewShardSetFilter("") - storagePolicyFilter := topic.NewStoragePolicyFilter([]string{}) + shardSetFilter := topic.NewShardSetFilter("[1..5]") + storagePolicyFilter := topic.NewStoragePolicyFilter([]string{"1m:40d"}) filterConfig := topic.NewFilterConfig().SetPercentageFilter(percentageFilter).SetShardSetFilter(shardSetFilter).SetStoragePolicyFilter(storagePolicyFilter) @@ -1286,6 +1287,8 @@ func TestTopicUpdateWithDynamicFilter2Updates(t *testing.T) { w.Close() } +// Consumer service has no static filters, topic update comes adding dynamic filters that that incurs parsing error. +// Expected: Topic update fails. func TestTopicUpdateWithDynamicFiltersInvalidFilter(t *testing.T) { defer leaktest.Check(t)() @@ -1303,7 +1306,7 @@ func TestTopicUpdateWithDynamicFiltersInvalidFilter(t *testing.T) { sid1 := services.NewServiceID().SetName("s1") - shardSetFilter := topic.NewShardSetFilter("") + shardSetFilter := topic.NewShardSetFilter("randomstringstrinxyz123abc") filterConfig := topic.NewFilterConfig().SetShardSetFilter(shardSetFilter) diff --git a/src/msg/topic/topic.go b/src/msg/topic/topic.go index 4e026a1f4e..4be526ad30 100644 --- a/src/msg/topic/topic.go +++ b/src/msg/topic/topic.go @@ -201,7 +201,7 @@ func (t *topic) Validate() error { percentageFilter := filterConfig.PercentageFilter() if percentageFilter != nil { percentage := percentageFilter.Percentage() - if percentage < 0 || percentage > 1 { + if percentage < 0 || percentage > 100 { return fmt.Errorf("invalid topic: invalid percentage in filter for consumer %s", cs.ServiceID().String()) } } @@ -212,6 +212,12 @@ func (t *topic) Validate() error { if len(storagePolicies) == 0 { return fmt.Errorf("invalid topic: empty storage policy filter for consumer %s", cs.ServiceID().String()) } + + for _, storagePolicy := range storagePolicies { + if storagePolicy == "" { + return fmt.Errorf("invalid topic: empty storage policy in filter for consumer %s", cs.ServiceID().String()) + } + } } } } @@ -317,7 +323,6 @@ type consumerService struct { sid services.ServiceID ct ConsumptionType ttlNanos int64 - shardSet string filterConfigs *filterConfig } @@ -382,16 +387,6 @@ func (cs *consumerService) SetMessageTTLNanos(value int64) ConsumerService { return &newcs } -func (cs *consumerService) ShardSet() string { - return cs.shardSet -} - -func (cs *consumerService) SetShardSet(value string) ConsumerService { - newcs := *cs - newcs.shardSet = value - return &newcs -} - func (cs *consumerService) DynamicFilterConfigs() FilterConfig { return cs.filterConfigs } @@ -438,12 +433,13 @@ func ServiceIDToProto(sid services.ServiceID) *topicpb.ServiceID { } } -// NewDynamicFilterConfigFromProto creates filters from a proto. +// NewDynamicFilterConfigFromProto creates filter config from a proto. func NewDynamicFilterConfigFromProto(filterProto *topicpb.Filters) FilterConfig { if filterProto == nil { return nil } - var filter filterConfig + + filter := filterConfig{} if filterProto.ShardSetFilter != nil { filter.shardSetFilterConfig = &shardSetFilter{shardSet: filterProto.ShardSetFilter.ShardSet} } diff --git a/src/msg/topic/topic_test.go b/src/msg/topic/topic_test.go index f6febbc4df..0c34fc133d 100644 --- a/src/msg/topic/topic_test.go +++ b/src/msg/topic/topic_test.go @@ -181,7 +181,7 @@ func TestTopicString(t *testing.T) { numOfShards: 1024 consumerServices: { {service: [name: s1, env: env1, zone: zone1], consumption type: shared} - {service: [name: s2, env: env2, zone: zone2], consumption type: shared, ttl: 1m0s, shard set filter: [10..23], storage policy filter: [1m:40d], percentage filter: 0.5} + {service: [name: s2, env: env2, zone: zone2], consumption type: shared, ttl: 1m0s, shard set filter: [10..23], storage policy filter: [1m:40d], percentage filter: 0.5} } } ` @@ -253,6 +253,12 @@ func TestTopicValidation(t *testing.T) { }) err = topic.Validate() require.Contains(t, err.Error(), "empty storage policy") + + topic = topic.SetConsumerServices([]ConsumerService{ + cs1.SetDynamicFilterConfigs(NewFilterConfig().SetStoragePolicyFilter(NewStoragePolicyFilter([]string{""}))), + }) + err = topic.Validate() + require.Contains(t, err.Error(), "empty storage policy") } func TestConsumerService(t *testing.T) { From b6bd11e9382885fcf9fd1969062ac6bfe91c9ebf Mon Sep 17 00:00:00 2001 From: Adam Jean-Laurent Date: Wed, 25 Sep 2024 11:44:29 -0400 Subject: [PATCH 09/18] fix topic tests --- src/msg/topic/topic.go | 32 ++++++++++++++++++++++++++++---- src/msg/topic/topic_test.go | 2 +- 2 files changed, 29 insertions(+), 5 deletions(-) diff --git a/src/msg/topic/topic.go b/src/msg/topic/topic.go index 4be526ad30..3ff1f289a9 100644 --- a/src/msg/topic/topic.go +++ b/src/msg/topic/topic.go @@ -254,32 +254,50 @@ func NewFilterConfig() FilterConfig { } func (fc *filterConfig) ShardSetFilter() ShardSetFilter { + if fc.shardSetFilterConfig == nil { + return nil + } + return fc.shardSetFilterConfig } func (fc *filterConfig) SetShardSetFilter(value ShardSetFilter) FilterConfig { newfc := *fc - newfc.shardSetFilterConfig = value.(*shardSetFilter) + if value != nil { + newfc.shardSetFilterConfig = value.(*shardSetFilter) + } return &newfc } func (fc *filterConfig) PercentageFilter() PercentageFilter { + if fc.percentageFilterConfig == nil { + return nil + } + return fc.percentageFilterConfig } func (fc *filterConfig) SetPercentageFilter(value PercentageFilter) FilterConfig { newfc := *fc - newfc.percentageFilterConfig = value.(*percentageFilter) + if value != nil { + newfc.percentageFilterConfig = value.(*percentageFilter) + } return &newfc } func (fc *filterConfig) StoragePolicyFilter() StoragePolicyFilter { + if fc.storagePolicyFilterConfig == nil { + return nil + } + return fc.storagePolicyFilterConfig } func (fc *filterConfig) SetStoragePolicyFilter(value StoragePolicyFilter) FilterConfig { newfc := *fc - newfc.storagePolicyFilterConfig = value.(*storagePolicyFilter) + if value != nil { + newfc.storagePolicyFilterConfig = value.(*storagePolicyFilter) + } return &newfc } @@ -388,12 +406,18 @@ func (cs *consumerService) SetMessageTTLNanos(value int64) ConsumerService { } func (cs *consumerService) DynamicFilterConfigs() FilterConfig { + if cs.filterConfigs == nil { + return nil + } + return cs.filterConfigs } func (cs *consumerService) SetDynamicFilterConfigs(value FilterConfig) ConsumerService { newcs := *cs - newcs.filterConfigs = value.(*filterConfig) + if value != nil { + newcs.filterConfigs = value.(*filterConfig) + } return &newcs } diff --git a/src/msg/topic/topic_test.go b/src/msg/topic/topic_test.go index 0c34fc133d..b4aa527f14 100644 --- a/src/msg/topic/topic_test.go +++ b/src/msg/topic/topic_test.go @@ -181,7 +181,7 @@ func TestTopicString(t *testing.T) { numOfShards: 1024 consumerServices: { {service: [name: s1, env: env1, zone: zone1], consumption type: shared} - {service: [name: s2, env: env2, zone: zone2], consumption type: shared, ttl: 1m0s, shard set filter: [10..23], storage policy filter: [1m:40d], percentage filter: 0.5} + {service: [name: s2, env: env2, zone: zone2], consumption type: shared, ttl: 1m0s, shard set filter: [10..23], percentage filter: 0.5, storage policy filter: [1m:40d]} } } ` From dd67a1220bea1c1055e8b9403f8ea7b34a8001a2 Mon Sep 17 00:00:00 2001 From: Adam Jean-Laurent Date: Wed, 25 Sep 2024 18:09:58 -0400 Subject: [PATCH 10/18] write writer tests --- src/msg/producer/writer/writer.go | 11 +- src/msg/producer/writer/writer_test.go | 742 +++++++++---------------- src/msg/topic/topic.go | 39 +- src/msg/topic/topic_test.go | 2 +- 4 files changed, 307 insertions(+), 487 deletions(-) diff --git a/src/msg/producer/writer/writer.go b/src/msg/producer/writer/writer.go index dd76543d7d..06207f7b32 100644 --- a/src/msg/producer/writer/writer.go +++ b/src/msg/producer/writer/writer.go @@ -162,6 +162,7 @@ func (w *writer) process(update interface{}) error { if err := t.Validate(); err != nil { return err } + // We don't allow changing number of shards for topics, it will be // prevented on topic service side, but also being defensive here as well. numShards := w.NumShards() @@ -179,14 +180,14 @@ func (w *writer) process(update interface{}) error { for _, cs := range t.ConsumerServices() { key := cs.ServiceID().String() csw, ok := w.consumerServiceWriters[key] + if ok { csw.SetMessageTTLNanos(cs.MessageTTLNanos()) if cs.DynamicFilterConfigs() != nil { - w.logger.Debug("registering dynamic filters", zap.String("consumer-service", cs.String())) - // NOTE: do we need to delete the ones from the static config ???? - consumerServicesWithDynamicFilterConfig[key] = true + + csw.UnregisterFilters() err := RegisterDynamicFilters(csw, cs.DynamicFilterConfigs()) if err != nil { w.logger.Error("could not register dynamic filters", @@ -198,6 +199,8 @@ func (w *writer) process(update interface{}) error { newConsumerServiceWriters[key] = csw + w.logger.Debug("Updated consumer service writer", zap.String("consumer-service", cs.String())) + continue } scope := iOpts.MetricsScope().Tagged(map[string]string{ @@ -209,8 +212,6 @@ func (w *writer) process(update interface{}) error { csw, err := newConsumerServiceWriter(cs, t.NumberOfShards(), w.opts.SetInstrumentOptions(iOpts.SetMetricsScope(scope))) if cs.DynamicFilterConfigs() != nil { - w.logger.Debug("registering dynamic filters", zap.Any("dynamic-filter-configs", cs.DynamicFilterConfigs())) - consumerServicesWithDynamicFilterConfig[key] = true err := RegisterDynamicFilters(csw, cs.DynamicFilterConfigs()) if err != nil { diff --git a/src/msg/producer/writer/writer_test.go b/src/msg/producer/writer/writer_test.go index 6a735167a8..1725667ffb 100644 --- a/src/msg/producer/writer/writer_test.go +++ b/src/msg/producer/writer/writer_test.go @@ -851,502 +851,284 @@ func TestWriterNumShards(t *testing.T) { require.Equal(t, 2, int(w.NumShards())) } -// Consumer service has no staticly configured filters, topic update comes in with dynamic filters. -// Expected: Dynamic filters present on csw. -func TestTopicUpdateWithDynamicFiltersNoStaticFilters(t *testing.T) { - defer leaktest.Check(t)() - - ctrl := xtest.NewController(t) - defer ctrl.Finish() - - store := mem.NewStore() - cs := client.NewMockClient(ctrl) - cs.EXPECT().Store(gomock.Any()).Return(store, nil) - - ts, err := topic.NewService(topic.NewServiceOptions().SetConfigService(cs)) - require.NoError(t, err) - - opts := testOptions().SetTopicService(ts) - - sid1 := services.NewServiceID().SetName("s1") - - percentageFilter := topic.NewPercentageFilter(50) - shardSetFilter := topic.NewShardSetFilter("[1..5]") - storagePolicyFilter := topic.NewStoragePolicyFilter([]string{"1m:40d"}) - - filterConfig := topic.NewFilterConfig().SetPercentageFilter(percentageFilter).SetShardSetFilter(shardSetFilter).SetStoragePolicyFilter(storagePolicyFilter) - - cs1 := topic.NewConsumerService().SetConsumptionType(topic.Replicated).SetServiceID(sid1).SetDynamicFilterConfigs(filterConfig) - - testTopic := topic.NewTopic(). - SetName(opts.TopicName()). - SetNumberOfShards(1). - SetConsumerServices([]topic.ConsumerService{cs1}) - _, err = ts.CheckAndSet(testTopic, kv.UninitializedVersion) - require.NoError(t, err) - - sd := services.NewMockServices(ctrl) - opts = opts.SetServiceDiscovery(sd) - ps1 := testPlacementService(store, sid1) - sd.EXPECT().PlacementService(sid1, gomock.Any()).Return(ps1, nil) - - p1 := placement.NewPlacement(). - SetInstances([]placement.Instance{ - placement.NewInstance(). - SetID("i1"). - SetEndpoint("i1"). - SetShards(shard.NewShards([]shard.Shard{ - shard.NewShard(0).SetState(shard.Available), - })), - }). - SetShards([]uint32{0}). - SetReplicaFactor(1). - SetIsSharded(true) - _, err = ps1.Set(p1) - require.NoError(t, err) - - w := NewWriter(opts).(*writer) - - called := atomic.NewInt32(0) - w.processFn = func(update interface{}) error { - called.Inc() - return w.process(update) +func TestDynamicConsumerServiceWriterFilters(t *testing.T) { + testDynamicFilterConfig := topic.NewFilterConfig(). + SetPercentageFilter( + topic.NewPercentageFilter(50), + ). + SetShardSetFilter( + topic.NewShardSetFilter("1..5"), + ). + SetStoragePolicyFilter( + topic.NewStoragePolicyFilter([]string{"1m:40d"}), + ) + + type testTopicUpdate struct { + dynamicFilterConfig topic.FilterConfig + expectedDataFilters []producer.FilterFuncMetadata + expectError bool } - require.NoError(t, w.Init()) - require.Equal(t, 1, int(called.Load())) - require.Equal(t, 1, len(w.consumerServiceWriters)) - - csw, ok := w.consumerServiceWriters[cs1.ServiceID().String()] - require.True(t, ok) - dataFilters := csw.GetDataFilters() - require.Equal(t, 4, len(dataFilters)) - - foundPercentageFilter := false - foundShardSetFilter := false - foundStoragePolicyFilter := false - foundAcceptAllFilter := false - - for _, filter := range dataFilters { - switch filter.Metadata.FilterType { - case producer.PercentageFilter: - foundPercentageFilter = true - require.Equal(t, filter.Metadata.SourceType, producer.DynamicConfig) - case producer.ShardSetFilter: - foundShardSetFilter = true - require.Equal(t, filter.Metadata.SourceType, producer.DynamicConfig) - case producer.StoragePolicyFilter: - foundStoragePolicyFilter = true - require.Equal(t, filter.Metadata.SourceType, producer.DynamicConfig) - case producer.AcceptAllFilter: - foundAcceptAllFilter = true - require.Equal(t, filter.Metadata.SourceType, producer.StaticConfig) - } + type testCase struct { + name string + staticFilters []producer.FilterFuncType + topicUpdate1 testTopicUpdate + topicUpdate2 *testTopicUpdate } - require.True(t, foundPercentageFilter) - require.True(t, foundShardSetFilter) - require.True(t, foundStoragePolicyFilter) - require.True(t, foundAcceptAllFilter) - - defer csw.Close() - - w.Close() -} - -// Consumer service has staticly configured filters, topic update comes in with dynamic filters. -// Expected: Dynamic filters override static and are the only filters present on csw. -func TestTopicUpdateWithDynamicFiltersHasStaticFilters(t *testing.T) { - defer leaktest.Check(t)() - - ctrl := xtest.NewController(t) - defer ctrl.Finish() - - store := mem.NewStore() - cs := client.NewMockClient(ctrl) - cs.EXPECT().Store(gomock.Any()).Return(store, nil) - - ts, err := topic.NewService(topic.NewServiceOptions().SetConfigService(cs)) - require.NoError(t, err) - - opts := testOptions().SetTopicService(ts) - - sid1 := services.NewServiceID().SetName("s1") - - percentageFilter := topic.NewPercentageFilter(50) - shardSetFilter := topic.NewShardSetFilter("[1..5]") - storagePolicyFilter := topic.NewStoragePolicyFilter([]string{"1m:40d"}) - - filterConfig := topic.NewFilterConfig().SetPercentageFilter(percentageFilter).SetShardSetFilter(shardSetFilter).SetStoragePolicyFilter(storagePolicyFilter) - - cs1 := topic.NewConsumerService().SetConsumptionType(topic.Replicated).SetServiceID(sid1).SetDynamicFilterConfigs(filterConfig) - - testTopic := topic.NewTopic(). - SetName(opts.TopicName()). - SetNumberOfShards(1). - SetConsumerServices([]topic.ConsumerService{cs1}) - _, err = ts.CheckAndSet(testTopic, kv.UninitializedVersion) - require.NoError(t, err) - - sd := services.NewMockServices(ctrl) - opts = opts.SetServiceDiscovery(sd) - ps1 := testPlacementService(store, sid1) - sd.EXPECT().PlacementService(sid1, gomock.Any()).Return(ps1, nil) - - p1 := placement.NewPlacement(). - SetInstances([]placement.Instance{ - placement.NewInstance(). - SetID("i1"). - SetEndpoint("i1"). - SetShards(shard.NewShards([]shard.Shard{ - shard.NewShard(0).SetState(shard.Available), - })), - }). - SetShards([]uint32{0}). - SetReplicaFactor(1). - SetIsSharded(true) - _, err = ps1.Set(p1) - require.NoError(t, err) - - w := NewWriter(opts).(*writer) - - // Register static filters for the consumer service. - w.RegisterFilter(sid1, producer.NewFilterFunc(func(producer.Message) bool { return true }, producer.PercentageFilter, producer.StaticConfig)) - w.RegisterFilter(sid1, producer.NewFilterFunc(func(producer.Message) bool { return true }, producer.StoragePolicyFilter, producer.StaticConfig)) - w.RegisterFilter(sid1, producer.NewFilterFunc(func(producer.Message) bool { return true }, producer.ShardSetFilter, producer.StaticConfig)) - - called := atomic.NewInt32(0) - w.processFn = func(update interface{}) error { - called.Inc() - return w.process(update) + tests := []testCase{ + { + name: "No_Static_Filters_One_Topic_Update_With_Dynamic_Filters", + staticFilters: []producer.FilterFuncType{}, + topicUpdate1: testTopicUpdate{ + dynamicFilterConfig: testDynamicFilterConfig, + expectedDataFilters: []producer.FilterFuncMetadata{ + {FilterType: producer.PercentageFilter, SourceType: producer.DynamicConfig}, + {FilterType: producer.ShardSetFilter, SourceType: producer.DynamicConfig}, + {FilterType: producer.StoragePolicyFilter, SourceType: producer.DynamicConfig}, + {FilterType: producer.AcceptAllFilter, SourceType: producer.StaticConfig}, + }, + }, + topicUpdate2: nil, + }, + + { + name: "Has_Static_Filters_One_Topic_Update_With_Dynamic_Filters", + staticFilters: []producer.FilterFuncType{producer.PercentageFilter, producer.ShardSetFilter, producer.StoragePolicyFilter}, + topicUpdate1: testTopicUpdate{ + dynamicFilterConfig: testDynamicFilterConfig, + expectedDataFilters: []producer.FilterFuncMetadata{ + {FilterType: producer.PercentageFilter, SourceType: producer.DynamicConfig}, + {FilterType: producer.ShardSetFilter, SourceType: producer.DynamicConfig}, + {FilterType: producer.StoragePolicyFilter, SourceType: producer.DynamicConfig}, + {FilterType: producer.AcceptAllFilter, SourceType: producer.StaticConfig}, + }, + }, + topicUpdate2: nil, + }, + + { + name: "Has_Static_Filters_Two_Topic_Updates_With_No_Dynamic_Filters", + staticFilters: []producer.FilterFuncType{producer.PercentageFilter, producer.ShardSetFilter, producer.StoragePolicyFilter}, + topicUpdate1: testTopicUpdate{ + dynamicFilterConfig: nil, + expectedDataFilters: []producer.FilterFuncMetadata{ + {FilterType: producer.PercentageFilter, SourceType: producer.StaticConfig}, + {FilterType: producer.ShardSetFilter, SourceType: producer.StaticConfig}, + {FilterType: producer.StoragePolicyFilter, SourceType: producer.StaticConfig}, + {FilterType: producer.AcceptAllFilter, SourceType: producer.StaticConfig}, + }, + }, + topicUpdate2: &testTopicUpdate{ + dynamicFilterConfig: nil, + expectedDataFilters: []producer.FilterFuncMetadata{ + {FilterType: producer.PercentageFilter, SourceType: producer.StaticConfig}, + {FilterType: producer.ShardSetFilter, SourceType: producer.StaticConfig}, + {FilterType: producer.StoragePolicyFilter, SourceType: producer.StaticConfig}, + {FilterType: producer.AcceptAllFilter, SourceType: producer.StaticConfig}, + }, + }, + }, + + { + name: "No_Static_Config_Two_Topic_Updates_With_Different_Dynamic_Filters", + staticFilters: []producer.FilterFuncType{}, + topicUpdate1: testTopicUpdate{ + dynamicFilterConfig: testDynamicFilterConfig, + expectedDataFilters: []producer.FilterFuncMetadata{ + {FilterType: producer.PercentageFilter, SourceType: producer.DynamicConfig}, + {FilterType: producer.ShardSetFilter, SourceType: producer.DynamicConfig}, + {FilterType: producer.StoragePolicyFilter, SourceType: producer.DynamicConfig}, + {FilterType: producer.AcceptAllFilter, SourceType: producer.StaticConfig}, + }, + }, + topicUpdate2: &testTopicUpdate{ + dynamicFilterConfig: topic.NewFilterConfig().SetPercentageFilter(topic.NewPercentageFilter(75)), + expectedDataFilters: []producer.FilterFuncMetadata{ + {FilterType: producer.PercentageFilter, SourceType: producer.DynamicConfig}, + {FilterType: producer.AcceptAllFilter, SourceType: producer.StaticConfig}, + }, + }, + }, + + { + name: "No_Static_Config_One_Topic_Update_With_Invalid_Dynamic_Filter", + staticFilters: []producer.FilterFuncType{}, + topicUpdate1: testTopicUpdate{ + dynamicFilterConfig: topic.NewFilterConfig().SetShardSetFilter(topic.NewShardSetFilter("randomstringstrinxyz123abc")), + expectedDataFilters: nil, // NOTE: comeback to this + expectError: true, + }, + topicUpdate2: nil, + }, + + { + name: "No_Static_Config_Two_Topic_Updates_First_Update_Adds_Dynamic_Filters_Second_Update_Removes_Dynamic_Filters", + staticFilters: []producer.FilterFuncType{}, + topicUpdate1: testTopicUpdate{ + dynamicFilterConfig: testDynamicFilterConfig, + expectedDataFilters: []producer.FilterFuncMetadata{ + {FilterType: producer.PercentageFilter, SourceType: producer.DynamicConfig}, + {FilterType: producer.ShardSetFilter, SourceType: producer.DynamicConfig}, + {FilterType: producer.StoragePolicyFilter, SourceType: producer.DynamicConfig}, + {FilterType: producer.AcceptAllFilter, SourceType: producer.StaticConfig}, + }, + }, + topicUpdate2: &testTopicUpdate{ + dynamicFilterConfig: nil, + expectedDataFilters: []producer.FilterFuncMetadata{ + {FilterType: producer.AcceptAllFilter, SourceType: producer.StaticConfig}, + }, + }, + }, } - require.NoError(t, w.Init()) - require.Equal(t, 1, int(called.Load())) - require.Equal(t, 1, len(w.consumerServiceWriters)) - - csw, ok := w.consumerServiceWriters[cs1.ServiceID().String()] - require.True(t, ok) - dataFilters := csw.GetDataFilters() - require.Equal(t, 4, len(dataFilters)) - - foundPercentageFilter := false - foundShardSetFilter := false - foundStoragePolicyFilter := false - foundAcceptAllFilter := false - - for _, filter := range dataFilters { - switch filter.Metadata.FilterType { - case producer.PercentageFilter: - foundPercentageFilter = true - require.Equal(t, filter.Metadata.SourceType, producer.DynamicConfig) - case producer.ShardSetFilter: - foundShardSetFilter = true - require.Equal(t, filter.Metadata.SourceType, producer.DynamicConfig) - case producer.StoragePolicyFilter: - foundStoragePolicyFilter = true - require.Equal(t, filter.Metadata.SourceType, producer.DynamicConfig) - case producer.AcceptAllFilter: - foundAcceptAllFilter = true - require.Equal(t, filter.Metadata.SourceType, producer.StaticConfig) - } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + defer leaktest.Check(t)() + + ctrl := xtest.NewController(t) + defer ctrl.Finish() + + store := mem.NewStore() + cs := client.NewMockClient(ctrl) + cs.EXPECT().Store(gomock.Any()).Return(store, nil) + + ts, err := topic.NewService(topic.NewServiceOptions().SetConfigService(cs)) + require.NoError(t, err) + + opts := testOptions().SetTopicService(ts) + + sid1 := services.NewServiceID().SetName("s1") + + cs1 := topic.NewConsumerService().SetConsumptionType(topic.Replicated).SetServiceID(sid1) + + if test.topicUpdate1.dynamicFilterConfig != nil { + cs1 = cs1.SetDynamicFilterConfigs(test.topicUpdate1.dynamicFilterConfig) + } + + testTopic := topic.NewTopic(). + SetName(opts.TopicName()). + SetNumberOfShards(1). + SetConsumerServices([]topic.ConsumerService{cs1}) + _, err = ts.CheckAndSet(testTopic, kv.UninitializedVersion) + + if test.topicUpdate1.expectError { + require.Error(t, err) + } else { + require.NoError(t, err) + } + + sd := services.NewMockServices(ctrl) + opts = opts.SetServiceDiscovery(sd) + ps1 := testPlacementService(store, sid1) + sd.EXPECT().PlacementService(sid1, gomock.Any()).Return(ps1, nil) + + p1 := placement.NewPlacement(). + SetInstances([]placement.Instance{ + placement.NewInstance(). + SetID("i1"). + SetEndpoint("i1"). + SetShards(shard.NewShards([]shard.Shard{ + shard.NewShard(0).SetState(shard.Available), + })), + }). + SetShards([]uint32{0}). + SetReplicaFactor(1). + SetIsSharded(true) + _, err = ps1.Set(p1) + require.NoError(t, err) + + w := NewWriter(opts).(*writer) + + for _, filterType := range test.staticFilters { + w.RegisterFilter(sid1, producer.NewFilterFunc(func(producer.Message) bool { return true }, filterType, producer.StaticConfig)) + } + + called := atomic.NewInt32(0) + w.processFn = func(update interface{}) error { + called.Inc() + return w.process(update) + } + + require.NoError(t, w.Init()) + require.Equal(t, 1, int(called.Load())) + require.Equal(t, 1, len(w.consumerServiceWriters)) + + csw, ok := w.consumerServiceWriters[cs1.ServiceID().String()] + require.True(t, ok) + + actualDataFilterFuncs := csw.GetDataFilters() + actualDataFilterMetadatas := make([]producer.FilterFuncMetadata, len(actualDataFilterFuncs)) + for _, filter := range actualDataFilterFuncs { + actualDataFilterMetadatas = append(actualDataFilterMetadatas, filter.Metadata) + } + + require.True(t, testAreFilterFuncMetadataSlicesEqual(actualDataFilterMetadatas, test.topicUpdate1.expectedDataFilters)) + + if test.topicUpdate2 != nil { + cs1.SetDynamicFilterConfigs(nil) + + if test.topicUpdate2.dynamicFilterConfig != nil { + cs1 = cs1.SetDynamicFilterConfigs(test.topicUpdate2.dynamicFilterConfig) + } + + testTopic = testTopic. + SetConsumerServices([]topic.ConsumerService{cs1}). + SetVersion(1) + _, err = ts.CheckAndSet(testTopic, 1) + + if test.topicUpdate2.expectError { + require.Error(t, err) + } else { + require.NoError(t, err) + } + + for called.Load() != 2 { + time.Sleep(50 * time.Millisecond) + } + + csw, ok = w.consumerServiceWriters[cs1.ServiceID().String()] + require.True(t, ok) + + actualDataFilterFuncs = csw.GetDataFilters() + actualDataFilterMetadatas = make([]producer.FilterFuncMetadata, len(actualDataFilterFuncs)) + for _, filter := range actualDataFilterFuncs { + actualDataFilterMetadatas = append(actualDataFilterMetadatas, filter.Metadata) + } + + require.True(t, testAreFilterFuncMetadataSlicesEqual(actualDataFilterMetadatas, test.topicUpdate2.expectedDataFilters)) + } + + defer csw.Close() + + w.Close() + }) } - - require.True(t, foundPercentageFilter) - require.True(t, foundShardSetFilter) - require.True(t, foundStoragePolicyFilter) - require.True(t, foundAcceptAllFilter) - - defer csw.Close() - - w.Close() } -// Consumer service has staticly configured filters, topic update comes in with no dynamic filters. -// Expected: Static filters remain on the csw. -func TestTopicUpdateNoDynamicFiltersHasStaticFilters(t *testing.T) { - defer leaktest.Check(t)() - - ctrl := xtest.NewController(t) - defer ctrl.Finish() - - store := mem.NewStore() - cs := client.NewMockClient(ctrl) - cs.EXPECT().Store(gomock.Any()).Return(store, nil) - - ts, err := topic.NewService(topic.NewServiceOptions().SetConfigService(cs)) - require.NoError(t, err) - - opts := testOptions().SetTopicService(ts) - - sid1 := services.NewServiceID().SetName("s1") - - cs1 := topic.NewConsumerService().SetConsumptionType(topic.Replicated).SetServiceID(sid1) - - testTopic := topic.NewTopic(). - SetName(opts.TopicName()). - SetNumberOfShards(1). - SetConsumerServices([]topic.ConsumerService{cs1}) - _, err = ts.CheckAndSet(testTopic, kv.UninitializedVersion) - require.NoError(t, err) - - sd := services.NewMockServices(ctrl) - opts = opts.SetServiceDiscovery(sd) - ps1 := testPlacementService(store, sid1) - sd.EXPECT().PlacementService(sid1, gomock.Any()).Return(ps1, nil) - - p1 := placement.NewPlacement(). - SetInstances([]placement.Instance{ - placement.NewInstance(). - SetID("i1"). - SetEndpoint("i1"). - SetShards(shard.NewShards([]shard.Shard{ - shard.NewShard(0).SetState(shard.Available), - })), - }). - SetShards([]uint32{0}). - SetReplicaFactor(1). - SetIsSharded(true) - _, err = ps1.Set(p1) - require.NoError(t, err) - - w := NewWriter(opts).(*writer) - - // Register static filters for the consumer service. - w.RegisterFilter(sid1, producer.NewFilterFunc(func(producer.Message) bool { return true }, producer.PercentageFilter, producer.StaticConfig)) - w.RegisterFilter(sid1, producer.NewFilterFunc(func(producer.Message) bool { return true }, producer.StoragePolicyFilter, producer.StaticConfig)) - w.RegisterFilter(sid1, producer.NewFilterFunc(func(producer.Message) bool { return true }, producer.ShardSetFilter, producer.StaticConfig)) - - called := atomic.NewInt32(0) - w.processFn = func(update interface{}) error { - called.Inc() - return w.process(update) +func testAreFilterFuncMetadataSlicesEqual(slice1, slice2 []producer.FilterFuncMetadata) bool { + if len(slice1) != len(slice2) { + return false } - require.NoError(t, w.Init()) - require.Equal(t, 1, int(called.Load())) - require.Equal(t, 1, len(w.consumerServiceWriters)) - - csw, ok := w.consumerServiceWriters[cs1.ServiceID().String()] - require.True(t, ok) - dataFilters := csw.GetDataFilters() - require.Equal(t, 4, len(dataFilters)) - - foundPercentageFilter := false - foundShardSetFilter := false - foundStoragePolicyFilter := false - foundAcceptAllFilter := false - - for _, filter := range dataFilters { - switch filter.Metadata.FilterType { - case producer.PercentageFilter: - foundPercentageFilter = true - require.Equal(t, filter.Metadata.SourceType, producer.StaticConfig) - case producer.ShardSetFilter: - foundShardSetFilter = true - require.Equal(t, filter.Metadata.SourceType, producer.StaticConfig) - case producer.StoragePolicyFilter: - foundStoragePolicyFilter = true - require.Equal(t, filter.Metadata.SourceType, producer.StaticConfig) - case producer.AcceptAllFilter: - foundAcceptAllFilter = true - require.Equal(t, filter.Metadata.SourceType, producer.StaticConfig) - } + countMap := make(map[producer.FilterFuncMetadata]int) + for _, item := range slice1 { + countMap[item]++ } - require.True(t, foundPercentageFilter) - require.True(t, foundShardSetFilter) - require.True(t, foundStoragePolicyFilter) - require.True(t, foundAcceptAllFilter) - - defer csw.Close() - - w.Close() -} - -// Consumer service has no static filters, topic update comes adding dynamic filters. Another topic update comes in changing the dynamic filters. -// Expected: Dynamic filters are updated on csw. -func TestTopicUpdateWithDynamicFilter2Updates(t *testing.T) { - defer leaktest.Check(t)() - - ctrl := xtest.NewController(t) - defer ctrl.Finish() - - store := mem.NewStore() - cs := client.NewMockClient(ctrl) - cs.EXPECT().Store(gomock.Any()).Return(store, nil) - - ts, err := topic.NewService(topic.NewServiceOptions().SetConfigService(cs)) - require.NoError(t, err) - - opts := testOptions().SetTopicService(ts) - - sid1 := services.NewServiceID().SetName("s1") - - percentageFilter := topic.NewPercentageFilter(50) - shardSetFilter := topic.NewShardSetFilter("[1..5]") - storagePolicyFilter := topic.NewStoragePolicyFilter([]string{"1m:40d"}) - - filterConfig := topic.NewFilterConfig().SetPercentageFilter(percentageFilter).SetShardSetFilter(shardSetFilter).SetStoragePolicyFilter(storagePolicyFilter) - - cs1 := topic.NewConsumerService().SetConsumptionType(topic.Replicated).SetServiceID(sid1).SetDynamicFilterConfigs(filterConfig) - - testTopic := topic.NewTopic(). - SetName(opts.TopicName()). - SetNumberOfShards(1). - SetConsumerServices([]topic.ConsumerService{cs1}) - _, err = ts.CheckAndSet(testTopic, kv.UninitializedVersion) - require.NoError(t, err) - - sd := services.NewMockServices(ctrl) - opts = opts.SetServiceDiscovery(sd) - ps1 := testPlacementService(store, sid1) - sd.EXPECT().PlacementService(sid1, gomock.Any()).Return(ps1, nil) - - p1 := placement.NewPlacement(). - SetInstances([]placement.Instance{ - placement.NewInstance(). - SetID("i1"). - SetEndpoint("i1"). - SetShards(shard.NewShards([]shard.Shard{ - shard.NewShard(0).SetState(shard.Available), - })), - }). - SetShards([]uint32{0}). - SetReplicaFactor(1). - SetIsSharded(true) - _, err = ps1.Set(p1) - require.NoError(t, err) - - w := NewWriter(opts).(*writer) - - called := atomic.NewInt32(0) - w.processFn = func(update interface{}) error { - called.Inc() - return w.process(update) - } - require.NoError(t, w.Init()) - require.Equal(t, 1, int(called.Load())) - require.Equal(t, 1, len(w.consumerServiceWriters)) - - csw, ok := w.consumerServiceWriters[cs1.ServiceID().String()] - require.True(t, ok) - - dataFilters := csw.GetDataFilters() - require.Equal(t, 4, len(dataFilters)) - - foundPercentageFilter := false - foundShardSetFilter := false - foundStoragePolicyFilter := false - foundAcceptAllFilter := false - - for _, filter := range dataFilters { - switch filter.Metadata.FilterType { - case producer.PercentageFilter: - foundPercentageFilter = true - require.Equal(t, filter.Metadata.SourceType, producer.DynamicConfig) - case producer.ShardSetFilter: - foundShardSetFilter = true - require.Equal(t, filter.Metadata.SourceType, producer.DynamicConfig) - case producer.StoragePolicyFilter: - foundStoragePolicyFilter = true - require.Equal(t, filter.Metadata.SourceType, producer.DynamicConfig) - case producer.AcceptAllFilter: - foundAcceptAllFilter = true - require.Equal(t, filter.Metadata.SourceType, producer.StaticConfig) + for _, item := range slice2 { + if countMap[item] == 0 { + return false } + countMap[item]-- } - require.True(t, foundPercentageFilter) - require.True(t, foundShardSetFilter) - require.True(t, foundStoragePolicyFilter) - require.True(t, foundAcceptAllFilter) - - // change the dynamic filters for topic - cs1.SetDynamicFilterConfigs(topic.NewFilterConfig().SetPercentageFilter(topic.NewPercentageFilter(75))) - - testTopic = testTopic. - SetConsumerServices([]topic.ConsumerService{cs1}). - SetVersion(1) - _, err = ts.CheckAndSet(testTopic, 1) - require.NoError(t, err) - - for called.Load() != 2 { - time.Sleep(50 * time.Millisecond) - } - - csw, ok = w.consumerServiceWriters[cs1.ServiceID().String()] - require.True(t, ok) - - require.Equal(t, 1, len(dataFilters)) - require.Equal(t, producer.PercentageFilter, dataFilters[0].Metadata.FilterType) - require.Equal(t, producer.DynamicConfig, dataFilters[0].Metadata.SourceType) - - defer csw.Close() - - w.Close() -} - -// Consumer service has no static filters, topic update comes adding dynamic filters that that incurs parsing error. -// Expected: Topic update fails. -func TestTopicUpdateWithDynamicFiltersInvalidFilter(t *testing.T) { - defer leaktest.Check(t)() - - ctrl := xtest.NewController(t) - defer ctrl.Finish() - - store := mem.NewStore() - cs := client.NewMockClient(ctrl) - cs.EXPECT().Store(gomock.Any()).Return(store, nil) - - ts, err := topic.NewService(topic.NewServiceOptions().SetConfigService(cs)) - require.NoError(t, err) - - opts := testOptions().SetTopicService(ts) - - sid1 := services.NewServiceID().SetName("s1") - - shardSetFilter := topic.NewShardSetFilter("randomstringstrinxyz123abc") - - filterConfig := topic.NewFilterConfig().SetShardSetFilter(shardSetFilter) - - cs1 := topic.NewConsumerService().SetConsumptionType(topic.Replicated).SetServiceID(sid1).SetDynamicFilterConfigs(filterConfig) - - testTopic := topic.NewTopic(). - SetName(opts.TopicName()). - SetNumberOfShards(1). - SetConsumerServices([]topic.ConsumerService{cs1}) - _, err = ts.CheckAndSet(testTopic, kv.UninitializedVersion) - require.NoError(t, err) - - sd := services.NewMockServices(ctrl) - opts = opts.SetServiceDiscovery(sd) - ps1 := testPlacementService(store, sid1) - sd.EXPECT().PlacementService(sid1, gomock.Any()).Return(ps1, nil) - - p1 := placement.NewPlacement(). - SetInstances([]placement.Instance{ - placement.NewInstance(). - SetID("i1"). - SetEndpoint("i1"). - SetShards(shard.NewShards([]shard.Shard{ - shard.NewShard(0).SetState(shard.Available), - })), - }). - SetShards([]uint32{0}). - SetReplicaFactor(1). - SetIsSharded(true) - _, err = ps1.Set(p1) - require.NoError(t, err) - - w := NewWriter(opts).(*writer) - - called := atomic.NewInt32(0) - w.processFn = func(update interface{}) error { - called.Inc() - return w.process(update) + for _, count := range countMap { + if count != 0 { + return false + } } - require.Error(t, w.Init()) - w.Close() + return true } diff --git a/src/msg/topic/topic.go b/src/msg/topic/topic.go index 3ff1f289a9..276c6b33ef 100644 --- a/src/msg/topic/topic.go +++ b/src/msg/topic/topic.go @@ -201,7 +201,7 @@ func (t *topic) Validate() error { percentageFilter := filterConfig.PercentageFilter() if percentageFilter != nil { percentage := percentageFilter.Percentage() - if percentage < 0 || percentage > 100 { + if percentage <= 0 || percentage >= 100 { return fmt.Errorf("invalid topic: invalid percentage in filter for consumer %s", cs.ServiceID().String()) } } @@ -372,6 +372,7 @@ func ConsumerServiceToProto(cs ConsumerService) (*topicpb.ConsumerService, error ConsumptionType: ct, ServiceId: ServiceIDToProto(cs.ServiceID()), MessageTtlNanos: cs.MessageTTLNanos(), + Filters: DynamicFilterConfigToProto(cs.DynamicFilterConfigs()), }, nil } @@ -476,3 +477,39 @@ func NewDynamicFilterConfigFromProto(filterProto *topicpb.Filters) FilterConfig return &filter } + +func DynamicFilterConfigToProto(filter FilterConfig) *topicpb.Filters { + if filter == nil { + return nil + } + + return &topicpb.Filters{ + ShardSetFilter: ShardSetFilterToProto(filter.ShardSetFilter()), + PercentageFilter: PercentageFilterToProto(filter.PercentageFilter()), + StoragePolicyFilter: StoragePolicyFilterToProto(filter.StoragePolicyFilter()), + } +} + +func ShardSetFilterToProto(filter ShardSetFilter) *topicpb.ShardSetFilter { + if filter == nil { + return nil + } + + return &topicpb.ShardSetFilter{ShardSet: filter.ShardSet()} +} + +func PercentageFilterToProto(filter PercentageFilter) *topicpb.PercentageFilter { + if filter == nil { + return nil + } + + return &topicpb.PercentageFilter{Percentage: filter.Percentage()} +} + +func StoragePolicyFilterToProto(filter StoragePolicyFilter) *topicpb.StoragePolicyFilter { + if filter == nil { + return nil + } + + return &topicpb.StoragePolicyFilter{StoragePolicies: filter.StoragePolicies()} +} diff --git a/src/msg/topic/topic_test.go b/src/msg/topic/topic_test.go index b4aa527f14..dbc762a833 100644 --- a/src/msg/topic/topic_test.go +++ b/src/msg/topic/topic_test.go @@ -146,7 +146,7 @@ func TestTopicUpdateConsumer(t *testing.T) { func TestTopicString(t *testing.T) { percentageFilter := NewPercentageFilter(0.5) - shardSetFilter := NewShardSetFilter("[10..23]") + shardSetFilter := NewShardSetFilter("10..23") storagePolicyFilter := NewStoragePolicyFilter([]string{"1m:40d"}) filterConfig := NewFilterConfig().SetPercentageFilter(percentageFilter).SetShardSetFilter(shardSetFilter).SetStoragePolicyFilter(storagePolicyFilter) From b1ab2bb29f228e5dc3abc49244c210ecab9f41d9 Mon Sep 17 00:00:00 2001 From: Adam Jean-Laurent Date: Thu, 26 Sep 2024 12:26:42 -0400 Subject: [PATCH 11/18] unit tests passing --- .../writer/consumer_service_writer.go | 14 +- src/msg/producer/writer/writer.go | 139 +++++++++++------- src/msg/producer/writer/writer_test.go | 132 ++++++++++++----- src/msg/topic/topic.go | 2 + 4 files changed, 193 insertions(+), 94 deletions(-) diff --git a/src/msg/producer/writer/consumer_service_writer.go b/src/msg/producer/writer/consumer_service_writer.go index b1d58c5985..5961f87081 100644 --- a/src/msg/producer/writer/consumer_service_writer.go +++ b/src/msg/producer/writer/consumer_service_writer.go @@ -88,7 +88,7 @@ type consumerServiceWriter interface { GetDataFilters() []producer.FilterFunc } -type ConsumerServiceWriterMetrics struct { +type consumerServiceWriterMetrics struct { placementError tally.Counter placementUpdate tally.Counter queueSize tally.Gauge @@ -101,11 +101,11 @@ type ConsumerServiceWriterMetrics struct { scope tally.Scope } -func (cswm *ConsumerServiceWriterMetrics) getGranularFilterCounterMapKey(metadata producer.FilterFuncMetadata) string { +func (cswm *consumerServiceWriterMetrics) getGranularFilterCounterMapKey(metadata producer.FilterFuncMetadata) string { return fmt.Sprintf("%s::%s", metadata.FilterType.String(), metadata.SourceType.String()) } -func (cswm *ConsumerServiceWriterMetrics) getFilterAcceptedGranularCounter(metadata producer.FilterFuncMetadata) tally.Counter { +func (cswm *consumerServiceWriterMetrics) getFilterAcceptedGranularCounter(metadata producer.FilterFuncMetadata) tally.Counter { key := cswm.getGranularFilterCounterMapKey(metadata) cswm.filterAcceptedGranularLock.RLock() @@ -126,7 +126,7 @@ func (cswm *ConsumerServiceWriterMetrics) getFilterAcceptedGranularCounter(metad return val } -func (cswm *ConsumerServiceWriterMetrics) getFilterNotAcceptedGranularCounter(metadata producer.FilterFuncMetadata) tally.Counter { +func (cswm *consumerServiceWriterMetrics) getFilterNotAcceptedGranularCounter(metadata producer.FilterFuncMetadata) tally.Counter { key := cswm.getGranularFilterCounterMapKey(metadata) cswm.filterNotAcceptedGranularLock.RLock() @@ -147,8 +147,8 @@ func (cswm *ConsumerServiceWriterMetrics) getFilterNotAcceptedGranularCounter(me return val } -func newConsumerServiceWriterMetrics(scope tally.Scope) ConsumerServiceWriterMetrics { - return ConsumerServiceWriterMetrics{ +func newConsumerServiceWriterMetrics(scope tally.Scope) consumerServiceWriterMetrics { + return consumerServiceWriterMetrics{ placementUpdate: scope.Counter("placement-update"), placementError: scope.Counter("placement-error"), filterAccepted: scope.Counter("filter-accepted"), @@ -176,7 +176,7 @@ type consumerServiceWriterImpl struct { closed bool doneCh chan struct{} wg sync.WaitGroup - m ConsumerServiceWriterMetrics + m consumerServiceWriterMetrics cm consumerWriterMetrics processFn watch.ProcessFn diff --git a/src/msg/producer/writer/writer.go b/src/msg/producer/writer/writer.go index 06207f7b32..a096ffbff6 100644 --- a/src/msg/producer/writer/writer.go +++ b/src/msg/producer/writer/writer.go @@ -171,35 +171,48 @@ func (w *writer) process(update interface{}) error { return fmt.Errorf("invalid topic update with %d shards, expecting %d", t.NumberOfShards(), numShards) } var ( - iOpts = w.opts.InstrumentOptions() - newConsumerServiceWriters = make(map[string]consumerServiceWriter, len(t.ConsumerServices())) - toBeClosed []consumerServiceWriter - multiErr xerrors.MultiError - consumerServicesWithDynamicFilterConfig = make(map[string]bool, len(t.ConsumerServices())) + iOpts = w.opts.InstrumentOptions() + newConsumerServiceWriters = make(map[string]consumerServiceWriter, len(t.ConsumerServices())) + toBeClosed []consumerServiceWriter + multiErr xerrors.MultiError ) for _, cs := range t.ConsumerServices() { key := cs.ServiceID().String() csw, ok := w.consumerServiceWriters[key] if ok { + // update existing consumer service writer + csw.SetMessageTTLNanos(cs.MessageTTLNanos()) if cs.DynamicFilterConfigs() != nil { - consumerServicesWithDynamicFilterConfig[key] = true + dynamicFilters, err := ParseDynamicFilters(csw, cs.DynamicFilterConfigs()) - csw.UnregisterFilters() - err := RegisterDynamicFilters(csw, cs.DynamicFilterConfigs()) if err != nil { - w.logger.Error("could not register dynamic filters", + w.logger.Error("could not update dynamic filters on consumer service writer, error registering dynamic filters", zap.String("writer", cs.String()), zap.Error(err)) multiErr = multiErr.Add(err) + } else { + // unregister all filters and register the new ones + csw.UnregisterFilters() + for _, dynamicFilter := range dynamicFilters { + csw.RegisterFilter(dynamicFilter) + } + } + } else { + // sending no dynamic filters means we should remove all filters, unless there are static filters + // if there are no static filters, remove all filters + _, ok := w.filterRegistry[key] + + if !ok { + csw.UnregisterFilters() } } newConsumerServiceWriters[key] = csw - w.logger.Debug("Updated consumer service writer", zap.String("consumer-service", cs.String())) + w.logger.Info("Updated consumer service writer", zap.String("consumer-service", cs.String())) continue } @@ -209,25 +222,42 @@ func (w *writer) process(update interface{}) error { "consumer-service-env": cs.ServiceID().Environment(), "consumption-type": cs.ConsumptionType().String(), }) + + // create new consumer service writer csw, err := newConsumerServiceWriter(cs, t.NumberOfShards(), w.opts.SetInstrumentOptions(iOpts.SetMetricsScope(scope))) + if err != nil { + w.logger.Error("could not create consumer service writer", + zap.String("writer", cs.String()), zap.Error(err)) + multiErr = multiErr.Add(err) + continue + } + + // if there are dynamicly configured filters, they are the source of truth if cs.DynamicFilterConfigs() != nil { - consumerServicesWithDynamicFilterConfig[key] = true - err := RegisterDynamicFilters(csw, cs.DynamicFilterConfigs()) + dynamicFilters, err := ParseDynamicFilters(csw, cs.DynamicFilterConfigs()) + if err != nil { - w.logger.Error("could not register dynamic filters", + w.logger.Error("could not create consumer service writer, error registering dynamic filters", zap.String("writer", cs.String()), zap.Error(err)) multiErr = multiErr.Add(err) + continue + } else { + for _, dynamicFilter := range dynamicFilters { + csw.RegisterFilter(dynamicFilter) + } } - } - if err != nil { - w.logger.Error("could not create consumer service writer", - zap.String("writer", cs.String()), zap.Error(err)) - multiErr = multiErr.Add(err) - continue + } else { + // if there are no dynamicly configured filters, static filters are the source of truth + if staticFilters, ok := w.filterRegistry[key]; ok { + for _, staticFilter := range staticFilters { + csw.RegisterFilter(staticFilter) + } + } } + if err = csw.Init(w.initType); err != nil { w.logger.Error("could not init consumer service writer", zap.String("writer", cs.String()), zap.Error(err)) @@ -257,17 +287,6 @@ func (w *writer) process(update interface{}) error { // Apply the new consumer service writers. w.Lock() - for key, csw := range newConsumerServiceWriters { - _, usingDynamicConfigForFilters := consumerServicesWithDynamicFilterConfig[key] - if !usingDynamicConfigForFilters { - if filters, ok := w.filterRegistry[key]; ok { - for _, filter := range filters { - csw.RegisterFilter(filter) - } - } - } - } - w.consumerServiceWriters = newConsumerServiceWriters w.numShards = t.NumberOfShards() w.Unlock() @@ -327,47 +346,65 @@ func (w *writer) UnregisterFilters(sid services.ServiceID) { } } -func RegisterDynamicFilters(csw consumerServiceWriter, filterConfig topic.FilterConfig) error { +func ParseDynamicFilters(csw consumerServiceWriter, filterConfig topic.FilterConfig) ([]producer.FilterFunc, error) { + filterFuncs := []producer.FilterFunc{} + if filterConfig == nil { - return errors.New("nil filter config") + return filterFuncs, errors.New("nil filter config") } if filterConfig.ShardSetFilter() != nil { - if err := RegisterShardSetFilterFromTopicUpdate(csw, filterConfig.ShardSetFilter()); err != nil { - return fmt.Errorf("Error registering shard set filter: %v", err) + shardSetFilterFunc, err := ParseShardSetFilterFromTopicUpdate(csw, filterConfig.ShardSetFilter()) + + if err != nil { + return filterFuncs, fmt.Errorf("Error registering shard set filter: %v", err) } + + filterFuncs = append(filterFuncs, shardSetFilterFunc) } if filterConfig.StoragePolicyFilter() != nil { - if err := RegisterStoragePolicyFilterFromTopicUpdate(csw, filterConfig.StoragePolicyFilter()); err != nil { - return fmt.Errorf("Error registering storage policy filter: %v", err) + storagePolicyFilterFunc, err := ParseStoragePolicyFilterFromTopicUpdate(csw, filterConfig.StoragePolicyFilter()) + + if err != nil { + return filterFuncs, fmt.Errorf("Error registering storage policy filter: %v", err) } + + filterFuncs = append(filterFuncs, storagePolicyFilterFunc) } if filterConfig.PercentageFilter() != nil { - if err := RegisterPercentageFilterFromFromTopicUpdate(csw, filterConfig.PercentageFilter()); err != nil { - return fmt.Errorf("Error registering percentage filter: %v", err) + percentageFilterFunc, err := ParsePercentageFilterFromFromTopicUpdate(csw, filterConfig.PercentageFilter()) + + if err != nil { + return filterFuncs, fmt.Errorf("Error registering percentage filter: %v", err) } + + filterFuncs = append(filterFuncs, percentageFilterFunc) } - return nil + return filterFuncs, nil } -func RegisterShardSetFilterFromTopicUpdate(csw consumerServiceWriter, ssf topic.ShardSetFilter) error { +func ParseShardSetFilterFromTopicUpdate(csw consumerServiceWriter, ssf topic.ShardSetFilter) (producer.FilterFunc, error) { + filterFunc := producer.FilterFunc{} + shardSetString := ssf.ShardSet() shardSet, err := sharding.ParseShardSet(shardSetString) if err != nil { - return errors.New("Error parsing shard set") + return filterFunc, errors.New("Error parsing shard set") } - csw.RegisterFilter(filter.NewShardSetFilter(shardSet, producer.DynamicConfig)) + filterFunc = filter.NewShardSetFilter(shardSet, producer.DynamicConfig) - return nil + return filterFunc, nil } -func RegisterStoragePolicyFilterFromTopicUpdate(csw consumerServiceWriter, spf topic.StoragePolicyFilter) error { +func ParseStoragePolicyFilterFromTopicUpdate(csw consumerServiceWriter, spf topic.StoragePolicyFilter) (producer.FilterFunc, error) { + filterFunc := producer.FilterFunc{} + storagePolicies := spf.StoragePolicies() parsedPolicies := make([]policy.StoragePolicy, len(storagePolicies)) @@ -375,21 +412,23 @@ func RegisterStoragePolicyFilterFromTopicUpdate(csw consumerServiceWriter, spf t parsedPolicy, err := policy.ParseStoragePolicy(storagePolicyString) if err != nil { - return fmt.Errorf("Error parsing storage policy: %v", err) + return filterFunc, fmt.Errorf("Error parsing storage policy: %v", err) } parsedPolicies = append(parsedPolicies, parsedPolicy) - csw.RegisterFilter(handlerWriter.NewStoragePolicyFilter(parsedPolicies, producer.DynamicConfig)) + filterFunc = handlerWriter.NewStoragePolicyFilter(parsedPolicies, producer.DynamicConfig) } - return nil + return filterFunc, nil } -func RegisterPercentageFilterFromFromTopicUpdate(csw consumerServiceWriter, pf topic.PercentageFilter) error { +func ParsePercentageFilterFromFromTopicUpdate(csw consumerServiceWriter, pf topic.PercentageFilter) (producer.FilterFunc, error) { + filterFunc := producer.FilterFunc{} + percentage := pf.Percentage() - csw.RegisterFilter(filter.NewPercentageFilter(percentage, producer.DynamicConfig)) + filterFunc = filter.NewPercentageFilter(percentage, producer.DynamicConfig) - return nil + return filterFunc, nil } diff --git a/src/msg/producer/writer/writer_test.go b/src/msg/producer/writer/writer_test.go index 1725667ffb..92d8ae91ca 100644 --- a/src/msg/producer/writer/writer_test.go +++ b/src/msg/producer/writer/writer_test.go @@ -864,9 +864,11 @@ func TestDynamicConsumerServiceWriterFilters(t *testing.T) { ) type testTopicUpdate struct { - dynamicFilterConfig topic.FilterConfig - expectedDataFilters []producer.FilterFuncMetadata - expectError bool + dynamicFilterConfig topic.FilterConfig + expectedDataFilters []producer.FilterFuncMetadata + expectTopicUpdateError bool + expectTopicValidationError bool + expectedCswCount int } type testCase struct { @@ -888,6 +890,7 @@ func TestDynamicConsumerServiceWriterFilters(t *testing.T) { {FilterType: producer.StoragePolicyFilter, SourceType: producer.DynamicConfig}, {FilterType: producer.AcceptAllFilter, SourceType: producer.StaticConfig}, }, + expectedCswCount: 1, }, topicUpdate2: nil, }, @@ -903,6 +906,7 @@ func TestDynamicConsumerServiceWriterFilters(t *testing.T) { {FilterType: producer.StoragePolicyFilter, SourceType: producer.DynamicConfig}, {FilterType: producer.AcceptAllFilter, SourceType: producer.StaticConfig}, }, + expectedCswCount: 1, }, topicUpdate2: nil, }, @@ -918,6 +922,7 @@ func TestDynamicConsumerServiceWriterFilters(t *testing.T) { {FilterType: producer.StoragePolicyFilter, SourceType: producer.StaticConfig}, {FilterType: producer.AcceptAllFilter, SourceType: producer.StaticConfig}, }, + expectedCswCount: 1, }, topicUpdate2: &testTopicUpdate{ dynamicFilterConfig: nil, @@ -927,6 +932,7 @@ func TestDynamicConsumerServiceWriterFilters(t *testing.T) { {FilterType: producer.StoragePolicyFilter, SourceType: producer.StaticConfig}, {FilterType: producer.AcceptAllFilter, SourceType: producer.StaticConfig}, }, + expectedCswCount: 1, }, }, @@ -941,6 +947,7 @@ func TestDynamicConsumerServiceWriterFilters(t *testing.T) { {FilterType: producer.StoragePolicyFilter, SourceType: producer.DynamicConfig}, {FilterType: producer.AcceptAllFilter, SourceType: producer.StaticConfig}, }, + expectedCswCount: 1, }, topicUpdate2: &testTopicUpdate{ dynamicFilterConfig: topic.NewFilterConfig().SetPercentageFilter(topic.NewPercentageFilter(75)), @@ -948,20 +955,55 @@ func TestDynamicConsumerServiceWriterFilters(t *testing.T) { {FilterType: producer.PercentageFilter, SourceType: producer.DynamicConfig}, {FilterType: producer.AcceptAllFilter, SourceType: producer.StaticConfig}, }, + expectedCswCount: 1, }, }, { - name: "No_Static_Config_One_Topic_Update_With_Invalid_Dynamic_Filter", + name: "No_Static_Config_One_Topic_Update_With_Invalid_Dynamic_Shard_Set_Filter", staticFilters: []producer.FilterFuncType{}, topicUpdate1: testTopicUpdate{ - dynamicFilterConfig: topic.NewFilterConfig().SetShardSetFilter(topic.NewShardSetFilter("randomstringstrinxyz123abc")), - expectedDataFilters: nil, // NOTE: comeback to this - expectError: true, + dynamicFilterConfig: topic.NewFilterConfig().SetShardSetFilter(topic.NewShardSetFilter("randomstringstrinxyz123abc")), + expectedDataFilters: []producer.FilterFuncMetadata{}, + expectTopicUpdateError: true, + expectedCswCount: 0, }, topicUpdate2: nil, }, + { + name: "No_Static_Config_One_Topic_Update_With_Invalid_Dynamic_Percentage_Filter", + staticFilters: []producer.FilterFuncType{}, + topicUpdate1: testTopicUpdate{ + dynamicFilterConfig: topic.NewFilterConfig().SetPercentageFilter(topic.NewPercentageFilter(99999)), + expectedDataFilters: []producer.FilterFuncMetadata{}, + expectTopicValidationError: true, + expectedCswCount: 0, + }, + topicUpdate2: nil, + }, + + { + name: "No_Static_Config_First_Topic_Update_With_Dynamic_Storage_Policy_Filter_Second_Topic_Update_With_Invalid_Dynamic_Shard_Set_Filter", + staticFilters: []producer.FilterFuncType{}, + topicUpdate1: testTopicUpdate{ + dynamicFilterConfig: topic.NewFilterConfig().SetStoragePolicyFilter(topic.NewStoragePolicyFilter([]string{"1m:40d"})), + expectedDataFilters: []producer.FilterFuncMetadata{ + {FilterType: producer.StoragePolicyFilter, SourceType: producer.DynamicConfig}, + {FilterType: producer.AcceptAllFilter, SourceType: producer.StaticConfig}, + }, + expectedCswCount: 1, + }, + topicUpdate2: &testTopicUpdate{ + dynamicFilterConfig: topic.NewFilterConfig().SetShardSetFilter(topic.NewShardSetFilter("randomstringstrinxyz123abc")), + expectedDataFilters: []producer.FilterFuncMetadata{ + {FilterType: producer.AcceptAllFilter, SourceType: producer.StaticConfig}, + {FilterType: producer.StoragePolicyFilter, SourceType: producer.DynamicConfig}, // second update should not be applied + }, + expectedCswCount: 1, + }, + }, + { name: "No_Static_Config_Two_Topic_Updates_First_Update_Adds_Dynamic_Filters_Second_Update_Removes_Dynamic_Filters", staticFilters: []producer.FilterFuncType{}, @@ -973,12 +1015,14 @@ func TestDynamicConsumerServiceWriterFilters(t *testing.T) { {FilterType: producer.StoragePolicyFilter, SourceType: producer.DynamicConfig}, {FilterType: producer.AcceptAllFilter, SourceType: producer.StaticConfig}, }, + expectedCswCount: 1, }, topicUpdate2: &testTopicUpdate{ dynamicFilterConfig: nil, expectedDataFilters: []producer.FilterFuncMetadata{ {FilterType: producer.AcceptAllFilter, SourceType: producer.StaticConfig}, }, + expectedCswCount: 1, }, }, } @@ -995,7 +1039,7 @@ func TestDynamicConsumerServiceWriterFilters(t *testing.T) { cs.EXPECT().Store(gomock.Any()).Return(store, nil) ts, err := topic.NewService(topic.NewServiceOptions().SetConfigService(cs)) - require.NoError(t, err) + require.NoError(t, err, "expect no error after reading topic service") opts := testOptions().SetTopicService(ts) @@ -1013,10 +1057,11 @@ func TestDynamicConsumerServiceWriterFilters(t *testing.T) { SetConsumerServices([]topic.ConsumerService{cs1}) _, err = ts.CheckAndSet(testTopic, kv.UninitializedVersion) - if test.topicUpdate1.expectError { - require.Error(t, err) + if test.topicUpdate1.expectTopicValidationError { + require.Error(t, err, "expect error after setting topic") + return } else { - require.NoError(t, err) + require.NoError(t, err, "expect no error after setting topic") } sd := services.NewMockServices(ctrl) @@ -1037,7 +1082,7 @@ func TestDynamicConsumerServiceWriterFilters(t *testing.T) { SetReplicaFactor(1). SetIsSharded(true) _, err = ps1.Set(p1) - require.NoError(t, err) + require.NoError(t, err, "expect no error after setting placement") w := NewWriter(opts).(*writer) @@ -1051,23 +1096,36 @@ func TestDynamicConsumerServiceWriterFilters(t *testing.T) { return w.process(update) } - require.NoError(t, w.Init()) - require.Equal(t, 1, int(called.Load())) - require.Equal(t, 1, len(w.consumerServiceWriters)) + err = w.Init() - csw, ok := w.consumerServiceWriters[cs1.ServiceID().String()] - require.True(t, ok) - - actualDataFilterFuncs := csw.GetDataFilters() - actualDataFilterMetadatas := make([]producer.FilterFuncMetadata, len(actualDataFilterFuncs)) - for _, filter := range actualDataFilterFuncs { - actualDataFilterMetadatas = append(actualDataFilterMetadatas, filter.Metadata) + if test.topicUpdate1.expectTopicUpdateError { + require.Error(t, err, "expect error after writer init") + } else { + require.NoError(t, err, "expect no error after writer init") } - require.True(t, testAreFilterFuncMetadataSlicesEqual(actualDataFilterMetadatas, test.topicUpdate1.expectedDataFilters)) + require.Equal(t, 1, int(called.Load()), "expect processFn to be called once") + require.Equal(t, test.topicUpdate1.expectedCswCount, len(w.consumerServiceWriters), "expect csw count to match after 1st topic update") + + if test.topicUpdate1.expectedCswCount > 0 { + csw, ok := w.consumerServiceWriters[cs1.ServiceID().String()] + require.True(t, ok, "expect csw to exist after 1st topic update") + + actualDataFilterFuncs := csw.GetDataFilters() + actualDataFilterMetadatas := []producer.FilterFuncMetadata{} + for _, filter := range actualDataFilterFuncs { + actualDataFilterMetadatas = append(actualDataFilterMetadatas, filter.Metadata) + } + + require.True(t, testAreFilterFuncMetadataSlicesEqual(actualDataFilterMetadatas, test.topicUpdate1.expectedDataFilters), "expect data filters to match after 1st topic update", actualDataFilterMetadatas, test.topicUpdate1.expectedDataFilters) + + if test.topicUpdate2 == nil { + csw.Close() + } + } if test.topicUpdate2 != nil { - cs1.SetDynamicFilterConfigs(nil) + cs1 = cs1.SetDynamicFilterConfigs(nil) if test.topicUpdate2.dynamicFilterConfig != nil { cs1 = cs1.SetDynamicFilterConfigs(test.topicUpdate2.dynamicFilterConfig) @@ -1078,29 +1136,29 @@ func TestDynamicConsumerServiceWriterFilters(t *testing.T) { SetVersion(1) _, err = ts.CheckAndSet(testTopic, 1) - if test.topicUpdate2.expectError { - require.Error(t, err) - } else { - require.NoError(t, err) - } - for called.Load() != 2 { time.Sleep(50 * time.Millisecond) } - csw, ok = w.consumerServiceWriters[cs1.ServiceID().String()] - require.True(t, ok) + require.Equal(t, test.topicUpdate1.expectedCswCount, len(w.consumerServiceWriters), "expect csw count to match after 2nd topic update") - actualDataFilterFuncs = csw.GetDataFilters() - actualDataFilterMetadatas = make([]producer.FilterFuncMetadata, len(actualDataFilterFuncs)) + if test.topicUpdate2.expectedCswCount == 0 { + return + } + + csw, ok := w.consumerServiceWriters[cs1.ServiceID().String()] + require.True(t, ok, "expect csw to exist after 2nd topic update") + + actualDataFilterFuncs := csw.GetDataFilters() + actualDataFilterMetadatas := []producer.FilterFuncMetadata{} for _, filter := range actualDataFilterFuncs { actualDataFilterMetadatas = append(actualDataFilterMetadatas, filter.Metadata) } - require.True(t, testAreFilterFuncMetadataSlicesEqual(actualDataFilterMetadatas, test.topicUpdate2.expectedDataFilters)) - } + require.True(t, testAreFilterFuncMetadataSlicesEqual(actualDataFilterMetadatas, test.topicUpdate2.expectedDataFilters), "expect data filters to match after 2nd topic update", actualDataFilterMetadatas, test.topicUpdate2.expectedDataFilters) - defer csw.Close() + csw.Close() + } w.Close() }) diff --git a/src/msg/topic/topic.go b/src/msg/topic/topic.go index 276c6b33ef..b6a5d0cd6a 100644 --- a/src/msg/topic/topic.go +++ b/src/msg/topic/topic.go @@ -418,6 +418,8 @@ func (cs *consumerService) SetDynamicFilterConfigs(value FilterConfig) ConsumerS newcs := *cs if value != nil { newcs.filterConfigs = value.(*filterConfig) + } else { + newcs.filterConfigs = nil } return &newcs } From 211f3921163343e37036d6539d6d456192c83450 Mon Sep 17 00:00:00 2001 From: Adam Jean-Laurent Date: Thu, 26 Sep 2024 12:42:10 -0400 Subject: [PATCH 12/18] fix csw metrics tests --- src/msg/producer/writer/consumer_service_writer_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/msg/producer/writer/consumer_service_writer_test.go b/src/msg/producer/writer/consumer_service_writer_test.go index 4a9a8237a0..340db6671f 100644 --- a/src/msg/producer/writer/consumer_service_writer_test.go +++ b/src/msg/producer/writer/consumer_service_writer_test.go @@ -712,15 +712,15 @@ func TestConsumerServiceWriterMetrics(t *testing.T) { snap := testScope.Snapshot() for _, c := range snap.Counters() { if c.Name() == "test.filter-accepted-granular" { - require.Equal(t, "config-source", c.Tags()["DynamicConfig"]) - require.Equal(t, "filter-type", c.Tags()["ShardSetFilter"]) + require.Equal(t, "DynamicConfig", c.Tags()["config-source"]) + require.Equal(t, "ShardSetFilter", c.Tags()["filter-type"]) require.Equal(t, int64(1), c.Value()) gotAcceptedValue = true } if c.Name() == "test.filter-not-accepted-granular" { - require.Equal(t, "config-source", c.Tags()["DynamicConfig"]) - require.Equal(t, "filter-type", c.Tags()["PercentageFilter"]) + require.Equal(t, "DynamicConfig", c.Tags()["config-source"]) + require.Equal(t, "PercentageFilter", c.Tags()["filter-type"]) require.Equal(t, int64(1), c.Value()) gotNotAcceptedValue = true } From 7f18604b797d25afe30fc68980be141dc9392787 Mon Sep 17 00:00:00 2001 From: Adam Jean-Laurent Date: Thu, 26 Sep 2024 12:58:41 -0400 Subject: [PATCH 13/18] fix 2 topic updates test --- src/msg/producer/writer/writer_test.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/msg/producer/writer/writer_test.go b/src/msg/producer/writer/writer_test.go index 92d8ae91ca..fae13b5b76 100644 --- a/src/msg/producer/writer/writer_test.go +++ b/src/msg/producer/writer/writer_test.go @@ -236,7 +236,6 @@ func TestWriterRegisterFilter(t *testing.T) { csw1.EXPECT().RegisterFilter(gomock.Any()) w.RegisterFilter(sid1, filter) - csw1.EXPECT().RegisterFilter(gomock.Any()) csw1.EXPECT().SetMessageTTLNanos(int64(0)) testTopic := topic.NewTopic(). SetName(opts.TopicName()). @@ -454,6 +453,9 @@ func TestTopicUpdateWithSameConsumerServicesButDifferentOrder(t *testing.T) { w.consumerServiceWriters[cs2.ServiceID().String()] = cswMock2 defer csw.Close() + cswMock1.EXPECT().UnregisterFilters() + cswMock2.EXPECT().UnregisterFilters() + cswMock1.EXPECT().SetMessageTTLNanos(int64(0)) cswMock2.EXPECT().SetMessageTTLNanos(int64(500)) testTopic = testTopic. From f5059228d834cbaa834ee0571416e85cbdd08c97 Mon Sep 17 00:00:00 2001 From: Adam Jean-Laurent Date: Thu, 26 Sep 2024 14:26:34 -0400 Subject: [PATCH 14/18] fix topic string test --- src/msg/topic/topic_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/msg/topic/topic_test.go b/src/msg/topic/topic_test.go index dbc762a833..532ce8d26a 100644 --- a/src/msg/topic/topic_test.go +++ b/src/msg/topic/topic_test.go @@ -181,7 +181,7 @@ func TestTopicString(t *testing.T) { numOfShards: 1024 consumerServices: { {service: [name: s1, env: env1, zone: zone1], consumption type: shared} - {service: [name: s2, env: env2, zone: zone2], consumption type: shared, ttl: 1m0s, shard set filter: [10..23], percentage filter: 0.5, storage policy filter: [1m:40d]} + {service: [name: s2, env: env2, zone: zone2], consumption type: shared, ttl: 1m0s, shard set filter: 10..23, percentage filter: 0.5, storage policy filter: [1m:40d]} } } ` From 66040b400f80a7d93cd44bb411bbdd1b371cb869 Mon Sep 17 00:00:00 2001 From: Adam Jean-Laurent Date: Thu, 26 Sep 2024 15:48:14 -0400 Subject: [PATCH 15/18] lint --- .../aggregator/handler/writer/protobuf.go | 9 ++- src/msg/producer/ref_counted.go | 9 ++- src/msg/producer/types.go | 11 ++++ .../writer/consumer_service_writer.go | 2 + .../writer/consumer_service_writer_test.go | 19 ++++-- src/msg/producer/writer/writer.go | 14 +++-- src/msg/producer/writer/writer_test.go | 59 ++++++++++++++----- src/msg/topic/topic.go | 11 +++- src/msg/topic/topic_test.go | 12 +++- src/msg/topic/types.go | 4 ++ 10 files changed, 120 insertions(+), 30 deletions(-) diff --git a/src/aggregator/aggregator/handler/writer/protobuf.go b/src/aggregator/aggregator/handler/writer/protobuf.go index 4e22b91e0a..429df31bfa 100644 --- a/src/aggregator/aggregator/handler/writer/protobuf.go +++ b/src/aggregator/aggregator/handler/writer/protobuf.go @@ -168,8 +168,13 @@ type storagePolicyFilter struct { } // NewStoragePolicyFilter creates a new storage policy based filter. -func NewStoragePolicyFilter(acceptedStoragePolicies []policy.StoragePolicy, configSource producer.FilterFuncConfigSourceType) producer.FilterFunc { - return producer.NewFilterFunc(storagePolicyFilter{acceptedStoragePolicies}.Filter, producer.StoragePolicyFilter, configSource) +func NewStoragePolicyFilter( + acceptedStoragePolicies []policy.StoragePolicy, + configSource producer.FilterFuncConfigSourceType) producer.FilterFunc { + return producer.NewFilterFunc( + storagePolicyFilter{acceptedStoragePolicies}.Filter, + producer.StoragePolicyFilter, + configSource) } func (f storagePolicyFilter) Filter(m producer.Message) bool { diff --git a/src/msg/producer/ref_counted.go b/src/msg/producer/ref_counted.go index 85679f529a..598b1a6619 100644 --- a/src/msg/producer/ref_counted.go +++ b/src/msg/producer/ref_counted.go @@ -51,12 +51,17 @@ func NewRefCountedMessage(m Message, fn OnFinalizeFn) *RefCountedMessage { } } -// Not a fan of passing function pointers here, but passing ConsumerServiceWriterMetrics to RefCountedMessage.Accept will cause a cyclical depdenency. ¯\_(ツ)_/¯ +// Not a fan of passing function pointers here, +// but passing ConsumerServiceWriterMetrics +// to RefCountedMessage.Accept will cause a cyclical depdenency. ¯\_(ツ)_/¯ type filterAcceptCounterFn func(metadata FilterFuncMetadata) tally.Counter type filterDenyCounterFn func(metadata FilterFuncMetadata) tally.Counter // Accept returns true if the message can be accepted by the filter. -func (rm *RefCountedMessage) Accept(fn []FilterFunc, acceptCounterFn filterAcceptCounterFn, denyCounterFn filterDenyCounterFn) bool { +func (rm *RefCountedMessage) Accept( + fn []FilterFunc, + acceptCounterFn filterAcceptCounterFn, + denyCounterFn filterDenyCounterFn) bool { if len(fn) == 0 { return false } diff --git a/src/msg/producer/types.go b/src/msg/producer/types.go index 07c4c1b966..d7c8b7a6e1 100644 --- a/src/msg/producer/types.go +++ b/src/msg/producer/types.go @@ -85,13 +85,19 @@ type Producer interface { Close(ct CloseType) } +// FilterFuncType specifies the type of filter function. type FilterFuncType uint8 const ( + // ShardSetFilter filters messages based on a shard set. ShardSetFilter FilterFuncType = iota + // StoragePolicyFilter filters messages based on a storage policy. StoragePolicyFilter + // PercentageFilter filters messages on a sampling percentage. PercentageFilter + // AcceptAllFilter accepts all messages. AcceptAllFilter + // UnspecifiedFilter is any filter that is not one of the well known types. UnspecifiedFilter ) @@ -113,10 +119,13 @@ func (f FilterFuncType) String() string { return "Unknown" } +// FilterFuncConfigSourceType specifies the configuration source of the filter function. type FilterFuncConfigSourceType uint8 const ( + // StaticConfig is static configuration that is applied once at service startup. StaticConfig FilterFuncConfigSourceType = iota + // DynamicConfig is dynamic configuration that can be updated at runtime. DynamicConfig ) @@ -137,6 +146,7 @@ type FilterFuncMetadata struct { SourceType FilterFuncConfigSourceType } +// NewFilterFuncMetadata creates a new filter function metadata. func NewFilterFuncMetadata(filterType FilterFuncType, sourceType FilterFuncConfigSourceType) FilterFuncMetadata { return FilterFuncMetadata{ FilterType: filterType, @@ -150,6 +160,7 @@ type FilterFunc struct { Metadata FilterFuncMetadata } +// NewFilterFunc creates a new filter function. func NewFilterFunc(function func(m Message) bool, filterType FilterFuncType, sourceType FilterFuncConfigSourceType) FilterFunc { return FilterFunc{ Function: function, diff --git a/src/msg/producer/writer/consumer_service_writer.go b/src/msg/producer/writer/consumer_service_writer.go index 5961f87081..298ac1bb48 100644 --- a/src/msg/producer/writer/consumer_service_writer.go +++ b/src/msg/producer/writer/consumer_service_writer.go @@ -105,6 +105,7 @@ func (cswm *consumerServiceWriterMetrics) getGranularFilterCounterMapKey(metadat return fmt.Sprintf("%s::%s", metadata.FilterType.String(), metadata.SourceType.String()) } +//nolint:dupl func (cswm *consumerServiceWriterMetrics) getFilterAcceptedGranularCounter(metadata producer.FilterFuncMetadata) tally.Counter { key := cswm.getGranularFilterCounterMapKey(metadata) @@ -126,6 +127,7 @@ func (cswm *consumerServiceWriterMetrics) getFilterAcceptedGranularCounter(metad return val } +//nolint:dupl func (cswm *consumerServiceWriterMetrics) getFilterNotAcceptedGranularCounter(metadata producer.FilterFuncMetadata) tally.Counter { key := cswm.getGranularFilterCounterMapKey(metadata) diff --git a/src/msg/producer/writer/consumer_service_writer_test.go b/src/msg/producer/writer/consumer_service_writer_test.go index 340db6671f..e3a5c675f6 100644 --- a/src/msg/producer/writer/consumer_service_writer_test.go +++ b/src/msg/producer/writer/consumer_service_writer_test.go @@ -522,7 +522,10 @@ func TestConsumerServiceWriterFilter(t *testing.T) { sw1.EXPECT().Write(gomock.Any()) csw.Write(producer.NewRefCountedMessage(mm1, nil)) - csw.RegisterFilter(producer.NewFilterFunc(func(m producer.Message) bool { return m.Shard() == uint32(0) }, producer.UnspecifiedFilter, producer.StaticConfig)) + csw.RegisterFilter(producer.NewFilterFunc( + func(m producer.Message) bool { return m.Shard() == uint32(0) }, + producer.UnspecifiedFilter, + producer.StaticConfig)) // Write is not expected due to mm1 shard != 0 csw.Write(producer.NewRefCountedMessage(mm1, nil)) @@ -530,7 +533,10 @@ func TestConsumerServiceWriterFilter(t *testing.T) { // Write is expected due to mm0 shard == 0 csw.Write(producer.NewRefCountedMessage(mm0, nil)) - csw.RegisterFilter(producer.NewFilterFunc(func(m producer.Message) bool { return m.Size() == 3 }, producer.UnspecifiedFilter, producer.StaticConfig)) + csw.RegisterFilter(producer.NewFilterFunc( + func(m producer.Message) bool { return m.Size() == 3 }, + producer.UnspecifiedFilter, + producer.StaticConfig)) sw0.EXPECT().Write(gomock.Any()) // Write is expected because to mm0 shard == 0 and mm0 size == 3 csw.Write(producer.NewRefCountedMessage(mm0, nil)) @@ -690,8 +696,13 @@ func TestConsumerServiceCloseShardWritersConcurrently(t *testing.T) { func TestConsumerServiceWriterMetrics(t *testing.T) { testScope := tally.NewTestScope("test", nil) - acceptedMetadata := producer.FilterFuncMetadata{FilterType: producer.ShardSetFilter, SourceType: producer.DynamicConfig} - notAcceptedMetadata := producer.FilterFuncMetadata{FilterType: producer.PercentageFilter, SourceType: producer.DynamicConfig} + acceptedMetadata := producer.FilterFuncMetadata{ + FilterType: producer.ShardSetFilter, + SourceType: producer.DynamicConfig} + + notAcceptedMetadata := producer.FilterFuncMetadata{ + FilterType: producer.PercentageFilter, + SourceType: producer.DynamicConfig} m := newConsumerServiceWriterMetrics(testScope) m.getFilterAcceptedGranularCounter(acceptedMetadata).Inc(1) diff --git a/src/msg/producer/writer/writer.go b/src/msg/producer/writer/writer.go index a096ffbff6..9d59db7c5d 100644 --- a/src/msg/producer/writer/writer.go +++ b/src/msg/producer/writer/writer.go @@ -346,6 +346,7 @@ func (w *writer) UnregisterFilters(sid services.ServiceID) { } } +// ParseDynamicFilters parses the dynamic filters for a consumer service from a topic update. func ParseDynamicFilters(csw consumerServiceWriter, filterConfig topic.FilterConfig) ([]producer.FilterFunc, error) { filterFuncs := []producer.FilterFunc{} @@ -357,7 +358,7 @@ func ParseDynamicFilters(csw consumerServiceWriter, filterConfig topic.FilterCon shardSetFilterFunc, err := ParseShardSetFilterFromTopicUpdate(csw, filterConfig.ShardSetFilter()) if err != nil { - return filterFuncs, fmt.Errorf("Error registering shard set filter: %v", err) + return filterFuncs, fmt.Errorf("Error registering shard set filter: %w", err) } filterFuncs = append(filterFuncs, shardSetFilterFunc) @@ -367,7 +368,7 @@ func ParseDynamicFilters(csw consumerServiceWriter, filterConfig topic.FilterCon storagePolicyFilterFunc, err := ParseStoragePolicyFilterFromTopicUpdate(csw, filterConfig.StoragePolicyFilter()) if err != nil { - return filterFuncs, fmt.Errorf("Error registering storage policy filter: %v", err) + return filterFuncs, fmt.Errorf("Error registering storage policy filter: %w", err) } filterFuncs = append(filterFuncs, storagePolicyFilterFunc) @@ -377,7 +378,7 @@ func ParseDynamicFilters(csw consumerServiceWriter, filterConfig topic.FilterCon percentageFilterFunc, err := ParsePercentageFilterFromFromTopicUpdate(csw, filterConfig.PercentageFilter()) if err != nil { - return filterFuncs, fmt.Errorf("Error registering percentage filter: %v", err) + return filterFuncs, fmt.Errorf("Error registering percentage filter: %w", err) } filterFuncs = append(filterFuncs, percentageFilterFunc) @@ -386,8 +387,9 @@ func ParseDynamicFilters(csw consumerServiceWriter, filterConfig topic.FilterCon return filterFuncs, nil } +// ParseShardSetFilterFromTopicUpdate parses a shard set filter from a topic update. func ParseShardSetFilterFromTopicUpdate(csw consumerServiceWriter, ssf topic.ShardSetFilter) (producer.FilterFunc, error) { - filterFunc := producer.FilterFunc{} + var filterFunc producer.FilterFunc shardSetString := ssf.ShardSet() @@ -402,8 +404,9 @@ func ParseShardSetFilterFromTopicUpdate(csw consumerServiceWriter, ssf topic.Sha return filterFunc, nil } +// ParseStoragePolicyFilterFromTopicUpdate parses a storage policy filter from a topic update. func ParseStoragePolicyFilterFromTopicUpdate(csw consumerServiceWriter, spf topic.StoragePolicyFilter) (producer.FilterFunc, error) { - filterFunc := producer.FilterFunc{} + var filterFunc producer.FilterFunc storagePolicies := spf.StoragePolicies() @@ -423,6 +426,7 @@ func ParseStoragePolicyFilterFromTopicUpdate(csw consumerServiceWriter, spf topi return filterFunc, nil } +// ParsePercentageFilterFromFromTopicUpdate parses a percentage filter from a topic update. func ParsePercentageFilterFromFromTopicUpdate(csw consumerServiceWriter, pf topic.PercentageFilter) (producer.FilterFunc, error) { filterFunc := producer.FilterFunc{} diff --git a/src/msg/producer/writer/writer_test.go b/src/msg/producer/writer/writer_test.go index fae13b5b76..cb5e05de36 100644 --- a/src/msg/producer/writer/writer_test.go +++ b/src/msg/producer/writer/writer_test.go @@ -211,8 +211,15 @@ func TestWriterRegisterFilter(t *testing.T) { csw1 := NewMockconsumerServiceWriter(ctrl) sid2 := services.NewServiceID().SetName("s2") - filter := producer.NewFilterFunc(func(producer.Message) bool { return false }, producer.UnspecifiedFilter, producer.StaticConfig) - filter2 := producer.NewFilterFunc(func(producer.Message) bool { return true }, producer.UnspecifiedFilter, producer.StaticConfig) + filter := producer.NewFilterFunc( + func(producer.Message) bool { return false }, + producer.UnspecifiedFilter, + producer.StaticConfig) + + filter2 := producer.NewFilterFunc( + func(producer.Message) bool { return true }, + producer.UnspecifiedFilter, + producer.StaticConfig) w := NewWriter(opts).(*writer) w.consumerServiceWriters[cs1.ServiceID().String()] = csw1 @@ -898,8 +905,12 @@ func TestDynamicConsumerServiceWriterFilters(t *testing.T) { }, { - name: "Has_Static_Filters_One_Topic_Update_With_Dynamic_Filters", - staticFilters: []producer.FilterFuncType{producer.PercentageFilter, producer.ShardSetFilter, producer.StoragePolicyFilter}, + name: "Has_Static_Filters_One_Topic_Update_With_Dynamic_Filters", + staticFilters: []producer.FilterFuncType{ + producer.PercentageFilter, + producer.ShardSetFilter, + producer.StoragePolicyFilter}, + topicUpdate1: testTopicUpdate{ dynamicFilterConfig: testDynamicFilterConfig, expectedDataFilters: []producer.FilterFuncMetadata{ @@ -914,8 +925,11 @@ func TestDynamicConsumerServiceWriterFilters(t *testing.T) { }, { - name: "Has_Static_Filters_Two_Topic_Updates_With_No_Dynamic_Filters", - staticFilters: []producer.FilterFuncType{producer.PercentageFilter, producer.ShardSetFilter, producer.StoragePolicyFilter}, + name: "Has_Static_Filters_Two_Topic_Updates_With_No_Dynamic_Filters", + staticFilters: []producer.FilterFuncType{ + producer.PercentageFilter, + producer.ShardSetFilter, + producer.StoragePolicyFilter}, topicUpdate1: testTopicUpdate{ dynamicFilterConfig: nil, expectedDataFilters: []producer.FilterFuncMetadata{ @@ -965,7 +979,8 @@ func TestDynamicConsumerServiceWriterFilters(t *testing.T) { name: "No_Static_Config_One_Topic_Update_With_Invalid_Dynamic_Shard_Set_Filter", staticFilters: []producer.FilterFuncType{}, topicUpdate1: testTopicUpdate{ - dynamicFilterConfig: topic.NewFilterConfig().SetShardSetFilter(topic.NewShardSetFilter("randomstringstrinxyz123abc")), + dynamicFilterConfig: topic.NewFilterConfig(). + SetShardSetFilter(topic.NewShardSetFilter("randomstringstrinxyz123abc")), expectedDataFilters: []producer.FilterFuncMetadata{}, expectTopicUpdateError: true, expectedCswCount: 0, @@ -986,10 +1001,12 @@ func TestDynamicConsumerServiceWriterFilters(t *testing.T) { }, { + // nolint:lll name: "No_Static_Config_First_Topic_Update_With_Dynamic_Storage_Policy_Filter_Second_Topic_Update_With_Invalid_Dynamic_Shard_Set_Filter", staticFilters: []producer.FilterFuncType{}, topicUpdate1: testTopicUpdate{ - dynamicFilterConfig: topic.NewFilterConfig().SetStoragePolicyFilter(topic.NewStoragePolicyFilter([]string{"1m:40d"})), + dynamicFilterConfig: topic.NewFilterConfig(). + SetStoragePolicyFilter(topic.NewStoragePolicyFilter([]string{"1m:40d"})), expectedDataFilters: []producer.FilterFuncMetadata{ {FilterType: producer.StoragePolicyFilter, SourceType: producer.DynamicConfig}, {FilterType: producer.AcceptAllFilter, SourceType: producer.StaticConfig}, @@ -997,7 +1014,8 @@ func TestDynamicConsumerServiceWriterFilters(t *testing.T) { expectedCswCount: 1, }, topicUpdate2: &testTopicUpdate{ - dynamicFilterConfig: topic.NewFilterConfig().SetShardSetFilter(topic.NewShardSetFilter("randomstringstrinxyz123abc")), + dynamicFilterConfig: topic.NewFilterConfig(). + SetShardSetFilter(topic.NewShardSetFilter("randomstringstrinxyz123abc")), expectedDataFilters: []producer.FilterFuncMetadata{ {FilterType: producer.AcceptAllFilter, SourceType: producer.StaticConfig}, {FilterType: producer.StoragePolicyFilter, SourceType: producer.DynamicConfig}, // second update should not be applied @@ -1007,6 +1025,7 @@ func TestDynamicConsumerServiceWriterFilters(t *testing.T) { }, { + // nolint:lll name: "No_Static_Config_Two_Topic_Updates_First_Update_Adds_Dynamic_Filters_Second_Update_Removes_Dynamic_Filters", staticFilters: []producer.FilterFuncType{}, topicUpdate1: testTopicUpdate{ @@ -1062,9 +1081,8 @@ func TestDynamicConsumerServiceWriterFilters(t *testing.T) { if test.topicUpdate1.expectTopicValidationError { require.Error(t, err, "expect error after setting topic") return - } else { - require.NoError(t, err, "expect no error after setting topic") } + require.NoError(t, err, "expect no error after setting topic") sd := services.NewMockServices(ctrl) opts = opts.SetServiceDiscovery(sd) @@ -1089,7 +1107,9 @@ func TestDynamicConsumerServiceWriterFilters(t *testing.T) { w := NewWriter(opts).(*writer) for _, filterType := range test.staticFilters { - w.RegisterFilter(sid1, producer.NewFilterFunc(func(producer.Message) bool { return true }, filterType, producer.StaticConfig)) + w.RegisterFilter( + sid1, + producer.NewFilterFunc(func(producer.Message) bool { return true }, filterType, producer.StaticConfig)) } called := atomic.NewInt32(0) @@ -1107,7 +1127,11 @@ func TestDynamicConsumerServiceWriterFilters(t *testing.T) { } require.Equal(t, 1, int(called.Load()), "expect processFn to be called once") - require.Equal(t, test.topicUpdate1.expectedCswCount, len(w.consumerServiceWriters), "expect csw count to match after 1st topic update") + require.Equal( + t, + test.topicUpdate1.expectedCswCount, + len(w.consumerServiceWriters), + "expect csw count to match after 1st topic update") if test.topicUpdate1.expectedCswCount > 0 { csw, ok := w.consumerServiceWriters[cs1.ServiceID().String()] @@ -1119,7 +1143,13 @@ func TestDynamicConsumerServiceWriterFilters(t *testing.T) { actualDataFilterMetadatas = append(actualDataFilterMetadatas, filter.Metadata) } - require.True(t, testAreFilterFuncMetadataSlicesEqual(actualDataFilterMetadatas, test.topicUpdate1.expectedDataFilters), "expect data filters to match after 1st topic update", actualDataFilterMetadatas, test.topicUpdate1.expectedDataFilters) + require.True(t, + testAreFilterFuncMetadataSlicesEqual( + actualDataFilterMetadatas, + test.topicUpdate1.expectedDataFilters), + "expect data filters to match after 1st topic update", + actualDataFilterMetadatas, + test.topicUpdate1.expectedDataFilters) if test.topicUpdate2 == nil { csw.Close() @@ -1137,6 +1167,7 @@ func TestDynamicConsumerServiceWriterFilters(t *testing.T) { SetConsumerServices([]topic.ConsumerService{cs1}). SetVersion(1) _, err = ts.CheckAndSet(testTopic, 1) + require.NoError(t, err, "expect no error after 2nd topic update") for called.Load() != 2 { time.Sleep(50 * time.Millisecond) diff --git a/src/msg/topic/topic.go b/src/msg/topic/topic.go index b6a5d0cd6a..cb9b263e7b 100644 --- a/src/msg/topic/topic.go +++ b/src/msg/topic/topic.go @@ -249,6 +249,7 @@ type filterConfig struct { storagePolicyFilterConfig *storagePolicyFilter } +// NewFilterConfig creates a new filter config. func NewFilterConfig() FilterConfig { return new(filterConfig) } @@ -309,6 +310,7 @@ func (filter *shardSetFilter) ShardSet() string { return filter.shardSet } +// NewShardSetFilter creates a new shard set filter. func NewShardSetFilter(shardSet string) ShardSetFilter { return &shardSetFilter{shardSet: shardSet} } @@ -317,6 +319,7 @@ type percentageFilter struct { percentage float64 } +// NewPercentageFilter creates a new percentage filter. func NewPercentageFilter(percentage float64) PercentageFilter { return &percentageFilter{percentage: percentage} } @@ -329,6 +332,7 @@ type storagePolicyFilter struct { storagePolicies []string } +// NewStoragePolicyFilter creates a new storage policy filter. func NewStoragePolicyFilter(storagePolicies []string) StoragePolicyFilter { return &storagePolicyFilter{storagePolicies: storagePolicies} } @@ -439,7 +443,8 @@ func (cs *consumerService) String() string { buf.WriteString(fmt.Sprintf(", percentage filter: %v", cs.filterConfigs.percentageFilterConfig.percentage)) } if cs.filterConfigs.storagePolicyFilterConfig != nil { - buf.WriteString(fmt.Sprintf(", storage policy filter: %v", cs.filterConfigs.storagePolicyFilterConfig.storagePolicies)) + buf.WriteString( + fmt.Sprintf(", storage policy filter: %v", cs.filterConfigs.storagePolicyFilterConfig.storagePolicies)) } } buf.WriteString("}") @@ -480,6 +485,7 @@ func NewDynamicFilterConfigFromProto(filterProto *topicpb.Filters) FilterConfig return &filter } +// DynamicFilterConfigToProto creates proto from a filter config. func DynamicFilterConfigToProto(filter FilterConfig) *topicpb.Filters { if filter == nil { return nil @@ -492,6 +498,7 @@ func DynamicFilterConfigToProto(filter FilterConfig) *topicpb.Filters { } } +// ShardSetFilterToProto creates proto from a shard set filter. func ShardSetFilterToProto(filter ShardSetFilter) *topicpb.ShardSetFilter { if filter == nil { return nil @@ -500,6 +507,7 @@ func ShardSetFilterToProto(filter ShardSetFilter) *topicpb.ShardSetFilter { return &topicpb.ShardSetFilter{ShardSet: filter.ShardSet()} } +// PercentageFilterToProto creates proto from a percentage filter. func PercentageFilterToProto(filter PercentageFilter) *topicpb.PercentageFilter { if filter == nil { return nil @@ -508,6 +516,7 @@ func PercentageFilterToProto(filter PercentageFilter) *topicpb.PercentageFilter return &topicpb.PercentageFilter{Percentage: filter.Percentage()} } +// StoragePolicyFilterToProto creates proto from a storage policy filter. func StoragePolicyFilterToProto(filter StoragePolicyFilter) *topicpb.StoragePolicyFilter { if filter == nil { return nil diff --git a/src/msg/topic/topic_test.go b/src/msg/topic/topic_test.go index 532ce8d26a..e58777a3bb 100644 --- a/src/msg/topic/topic_test.go +++ b/src/msg/topic/topic_test.go @@ -149,7 +149,10 @@ func TestTopicString(t *testing.T) { shardSetFilter := NewShardSetFilter("10..23") storagePolicyFilter := NewStoragePolicyFilter([]string{"1m:40d"}) - filterConfig := NewFilterConfig().SetPercentageFilter(percentageFilter).SetShardSetFilter(shardSetFilter).SetStoragePolicyFilter(storagePolicyFilter) + filterConfig := NewFilterConfig(). + SetPercentageFilter(percentageFilter). + SetShardSetFilter(shardSetFilter). + SetStoragePolicyFilter(storagePolicyFilter) cs1 := NewConsumerService(). SetConsumptionType(Shared). @@ -174,6 +177,7 @@ func TestTopicString(t *testing.T) { SetConsumerServices( []ConsumerService{cs1, cs2}, ) + //nolint:lll str := ` { version: 5 @@ -231,7 +235,11 @@ func TestTopicValidation(t *testing.T) { require.NoError(t, err) topic = topic.SetConsumerServices([]ConsumerService{ - cs1.SetDynamicFilterConfigs(NewFilterConfig().SetPercentageFilter(NewPercentageFilter(0.4)).SetStoragePolicyFilter(NewStoragePolicyFilter([]string{"1m:40d"})).SetShardSetFilter(NewShardSetFilter("[10..23]"))), + cs1.SetDynamicFilterConfigs( + NewFilterConfig(). + SetPercentageFilter(NewPercentageFilter(0.4)). + SetStoragePolicyFilter(NewStoragePolicyFilter([]string{"1m:40d"})). + SetShardSetFilter(NewShardSetFilter("[10..23]"))), }) err = topic.Validate() require.NoError(t, err) diff --git a/src/msg/topic/types.go b/src/msg/topic/types.go index 0d8bb43f83..2796f4d40b 100644 --- a/src/msg/topic/types.go +++ b/src/msg/topic/types.go @@ -98,6 +98,7 @@ type ConsumerService interface { String() string } +// FilterConfig is the filter configuration for a consumer service. type FilterConfig interface { // StoragePolicyFilter returns the storage policy data filter of the consumer service. StoragePolicyFilter() StoragePolicyFilter @@ -118,14 +119,17 @@ type FilterConfig interface { SetShardSetFilter(value ShardSetFilter) FilterConfig } +// PercentageFilter is sampling percentage filter for a consumer service. type PercentageFilter interface { Percentage() float64 } +// StoragePolicyFilter is the storage policy filter for a consumer service, filters metrics based on their storage policies. type StoragePolicyFilter interface { StoragePolicies() []string } +// ShardSetFilter is the shard set filter for a consumer service, filters metrics based on specified shard sets. type ShardSetFilter interface { ShardSet() string } From e913a7da73cfcf6d979ffbf0febadf5cf93b3b42 Mon Sep 17 00:00:00 2001 From: Adam Jean-Laurent Date: Thu, 26 Sep 2024 15:59:02 -0400 Subject: [PATCH 16/18] lint 2 --- src/msg/producer/types.go | 10 ++++++++-- .../producer/writer/consumer_service_writer.go | 6 ++++-- src/msg/producer/writer/writer.go | 18 ++++++++++++------ src/msg/producer/writer/writer_test.go | 17 ++++++++++++++--- src/msg/topic/topic.go | 3 ++- src/msg/topic/types.go | 3 ++- 6 files changed, 42 insertions(+), 15 deletions(-) diff --git a/src/msg/producer/types.go b/src/msg/producer/types.go index d7c8b7a6e1..4771fd2809 100644 --- a/src/msg/producer/types.go +++ b/src/msg/producer/types.go @@ -141,13 +141,16 @@ func (f FilterFuncConfigSourceType) String() string { return "Unknown" } +// FilterFuncMetadata contains metadata about a filter function. type FilterFuncMetadata struct { FilterType FilterFuncType SourceType FilterFuncConfigSourceType } // NewFilterFuncMetadata creates a new filter function metadata. -func NewFilterFuncMetadata(filterType FilterFuncType, sourceType FilterFuncConfigSourceType) FilterFuncMetadata { +func NewFilterFuncMetadata( + filterType FilterFuncType, + sourceType FilterFuncConfigSourceType) FilterFuncMetadata { return FilterFuncMetadata{ FilterType: filterType, SourceType: sourceType, @@ -161,7 +164,10 @@ type FilterFunc struct { } // NewFilterFunc creates a new filter function. -func NewFilterFunc(function func(m Message) bool, filterType FilterFuncType, sourceType FilterFuncConfigSourceType) FilterFunc { +func NewFilterFunc( + function func(m Message) bool, + filterType FilterFuncType, + sourceType FilterFuncConfigSourceType) FilterFunc { return FilterFunc{ Function: function, Metadata: NewFilterFuncMetadata(filterType, sourceType), diff --git a/src/msg/producer/writer/consumer_service_writer.go b/src/msg/producer/writer/consumer_service_writer.go index 298ac1bb48..77dc62b699 100644 --- a/src/msg/producer/writer/consumer_service_writer.go +++ b/src/msg/producer/writer/consumer_service_writer.go @@ -106,7 +106,8 @@ func (cswm *consumerServiceWriterMetrics) getGranularFilterCounterMapKey(metadat } //nolint:dupl -func (cswm *consumerServiceWriterMetrics) getFilterAcceptedGranularCounter(metadata producer.FilterFuncMetadata) tally.Counter { +func (cswm *consumerServiceWriterMetrics) getFilterAcceptedGranularCounter( + metadata producer.FilterFuncMetadata) tally.Counter { key := cswm.getGranularFilterCounterMapKey(metadata) cswm.filterAcceptedGranularLock.RLock() @@ -128,7 +129,8 @@ func (cswm *consumerServiceWriterMetrics) getFilterAcceptedGranularCounter(metad } //nolint:dupl -func (cswm *consumerServiceWriterMetrics) getFilterNotAcceptedGranularCounter(metadata producer.FilterFuncMetadata) tally.Counter { +func (cswm *consumerServiceWriterMetrics) getFilterNotAcceptedGranularCounter( + metadata producer.FilterFuncMetadata) tally.Counter { key := cswm.getGranularFilterCounterMapKey(metadata) cswm.filterNotAcceptedGranularLock.RLock() diff --git a/src/msg/producer/writer/writer.go b/src/msg/producer/writer/writer.go index 9d59db7c5d..8280b80a79 100644 --- a/src/msg/producer/writer/writer.go +++ b/src/msg/producer/writer/writer.go @@ -388,7 +388,9 @@ func ParseDynamicFilters(csw consumerServiceWriter, filterConfig topic.FilterCon } // ParseShardSetFilterFromTopicUpdate parses a shard set filter from a topic update. -func ParseShardSetFilterFromTopicUpdate(csw consumerServiceWriter, ssf topic.ShardSetFilter) (producer.FilterFunc, error) { +func ParseShardSetFilterFromTopicUpdate( + csw consumerServiceWriter, + ssf topic.ShardSetFilter) (producer.FilterFunc, error) { var filterFunc producer.FilterFunc shardSetString := ssf.ShardSet() @@ -405,17 +407,19 @@ func ParseShardSetFilterFromTopicUpdate(csw consumerServiceWriter, ssf topic.Sha } // ParseStoragePolicyFilterFromTopicUpdate parses a storage policy filter from a topic update. -func ParseStoragePolicyFilterFromTopicUpdate(csw consumerServiceWriter, spf topic.StoragePolicyFilter) (producer.FilterFunc, error) { +func ParseStoragePolicyFilterFromTopicUpdate( + csw consumerServiceWriter, + spf topic.StoragePolicyFilter) (producer.FilterFunc, error) { var filterFunc producer.FilterFunc storagePolicies := spf.StoragePolicies() - parsedPolicies := make([]policy.StoragePolicy, len(storagePolicies)) + parsedPolicies := []policy.StoragePolicy{} for _, storagePolicyString := range storagePolicies { parsedPolicy, err := policy.ParseStoragePolicy(storagePolicyString) if err != nil { - return filterFunc, fmt.Errorf("Error parsing storage policy: %v", err) + return filterFunc, fmt.Errorf("Error parsing storage policy: %w", err) } parsedPolicies = append(parsedPolicies, parsedPolicy) @@ -427,8 +431,10 @@ func ParseStoragePolicyFilterFromTopicUpdate(csw consumerServiceWriter, spf topi } // ParsePercentageFilterFromFromTopicUpdate parses a percentage filter from a topic update. -func ParsePercentageFilterFromFromTopicUpdate(csw consumerServiceWriter, pf topic.PercentageFilter) (producer.FilterFunc, error) { - filterFunc := producer.FilterFunc{} +func ParsePercentageFilterFromFromTopicUpdate( + csw consumerServiceWriter, + pf topic.PercentageFilter) (producer.FilterFunc, error) { + var filterFunc producer.FilterFunc percentage := pf.Percentage() diff --git a/src/msg/producer/writer/writer_test.go b/src/msg/producer/writer/writer_test.go index cb5e05de36..91fe51e1ec 100644 --- a/src/msg/producer/writer/writer_test.go +++ b/src/msg/producer/writer/writer_test.go @@ -1018,7 +1018,8 @@ func TestDynamicConsumerServiceWriterFilters(t *testing.T) { SetShardSetFilter(topic.NewShardSetFilter("randomstringstrinxyz123abc")), expectedDataFilters: []producer.FilterFuncMetadata{ {FilterType: producer.AcceptAllFilter, SourceType: producer.StaticConfig}, - {FilterType: producer.StoragePolicyFilter, SourceType: producer.DynamicConfig}, // second update should not be applied + // second update should not be applied + {FilterType: producer.StoragePolicyFilter, SourceType: producer.DynamicConfig}, }, expectedCswCount: 1, }, @@ -1173,7 +1174,11 @@ func TestDynamicConsumerServiceWriterFilters(t *testing.T) { time.Sleep(50 * time.Millisecond) } - require.Equal(t, test.topicUpdate1.expectedCswCount, len(w.consumerServiceWriters), "expect csw count to match after 2nd topic update") + require.Equal( + t, + test.topicUpdate1.expectedCswCount, + len(w.consumerServiceWriters), + "expect csw count to match after 2nd topic update") if test.topicUpdate2.expectedCswCount == 0 { return @@ -1188,7 +1193,13 @@ func TestDynamicConsumerServiceWriterFilters(t *testing.T) { actualDataFilterMetadatas = append(actualDataFilterMetadatas, filter.Metadata) } - require.True(t, testAreFilterFuncMetadataSlicesEqual(actualDataFilterMetadatas, test.topicUpdate2.expectedDataFilters), "expect data filters to match after 2nd topic update", actualDataFilterMetadatas, test.topicUpdate2.expectedDataFilters) + require.True(t, + testAreFilterFuncMetadataSlicesEqual( + actualDataFilterMetadatas, + test.topicUpdate2.expectedDataFilters), + "expect data filters to match after 2nd topic update", + actualDataFilterMetadatas, + test.topicUpdate2.expectedDataFilters) csw.Close() } diff --git a/src/msg/topic/topic.go b/src/msg/topic/topic.go index cb9b263e7b..5a5b8d7345 100644 --- a/src/msg/topic/topic.go +++ b/src/msg/topic/topic.go @@ -479,7 +479,8 @@ func NewDynamicFilterConfigFromProto(filterProto *topicpb.Filters) FilterConfig filter.percentageFilterConfig = &percentageFilter{percentage: filterProto.PercentageFilter.Percentage} } if filterProto.StoragePolicyFilter != nil { - filter.storagePolicyFilterConfig = &storagePolicyFilter{storagePolicies: filterProto.StoragePolicyFilter.StoragePolicies} + filter.storagePolicyFilterConfig = &storagePolicyFilter{ + storagePolicies: filterProto.StoragePolicyFilter.StoragePolicies} } return &filter diff --git a/src/msg/topic/types.go b/src/msg/topic/types.go index 2796f4d40b..3125cc54a8 100644 --- a/src/msg/topic/types.go +++ b/src/msg/topic/types.go @@ -124,7 +124,8 @@ type PercentageFilter interface { Percentage() float64 } -// StoragePolicyFilter is the storage policy filter for a consumer service, filters metrics based on their storage policies. +// StoragePolicyFilter is the storage policy filter for a consumer service, +// filters metrics based on their storage policies. type StoragePolicyFilter interface { StoragePolicies() []string } From 6d404be7f1dee2a25bf80bf989544dd042efe4b6 Mon Sep 17 00:00:00 2001 From: Adam Jean-Laurent Date: Fri, 27 Sep 2024 10:02:14 -0400 Subject: [PATCH 17/18] fix race conditon in tests --- src/msg/producer/writer/writer_test.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/msg/producer/writer/writer_test.go b/src/msg/producer/writer/writer_test.go index 91fe51e1ec..737cb1b176 100644 --- a/src/msg/producer/writer/writer_test.go +++ b/src/msg/producer/writer/writer_test.go @@ -1171,15 +1171,19 @@ func TestDynamicConsumerServiceWriterFilters(t *testing.T) { require.NoError(t, err, "expect no error after 2nd topic update") for called.Load() != 2 { - time.Sleep(50 * time.Millisecond) + time.Sleep(100 * time.Millisecond) } + w.RLock() + require.Equal( t, test.topicUpdate1.expectedCswCount, len(w.consumerServiceWriters), "expect csw count to match after 2nd topic update") + w.RUnlock() + if test.topicUpdate2.expectedCswCount == 0 { return } From b9eef68cbc42b52fdef7975d457b9afea32bfb40 Mon Sep 17 00:00:00 2001 From: Adam Jean-Laurent Date: Fri, 27 Sep 2024 11:41:52 -0400 Subject: [PATCH 18/18] add locks --- src/msg/producer/writer/writer.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/msg/producer/writer/writer.go b/src/msg/producer/writer/writer.go index 8280b80a79..fbc82a1172 100644 --- a/src/msg/producer/writer/writer.go +++ b/src/msg/producer/writer/writer.go @@ -195,10 +195,14 @@ func (w *writer) process(update interface{}) error { multiErr = multiErr.Add(err) } else { // unregister all filters and register the new ones + w.Lock() + csw.UnregisterFilters() for _, dynamicFilter := range dynamicFilters { csw.RegisterFilter(dynamicFilter) } + + w.Unlock() } } else { // sending no dynamic filters means we should remove all filters, unless there are static filters @@ -206,7 +210,9 @@ func (w *writer) process(update interface{}) error { _, ok := w.filterRegistry[key] if !ok { + w.Lock() csw.UnregisterFilters() + w.Unlock() } } @@ -244,18 +250,26 @@ func (w *writer) process(update interface{}) error { multiErr = multiErr.Add(err) continue } else { + w.Lock() + for _, dynamicFilter := range dynamicFilters { csw.RegisterFilter(dynamicFilter) } + + w.Unlock() } } else { + w.Lock() + // if there are no dynamicly configured filters, static filters are the source of truth if staticFilters, ok := w.filterRegistry[key]; ok { for _, staticFilter := range staticFilters { csw.RegisterFilter(staticFilter) } } + + w.Unlock() } if err = csw.Init(w.initType); err != nil {