diff --git a/internal/conf/conf.go b/internal/conf/conf.go index d844818b5f..24f13d312c 100644 --- a/internal/conf/conf.go +++ b/internal/conf/conf.go @@ -27,7 +27,7 @@ import ( "time" "github.com/sirupsen/logrus" - "github.com/yisaer/file-rotatelogs" + rotatelogs "github.com/yisaer/file-rotatelogs" "github.com/lf-edge/ekuiper/v2/internal/conf/logger" "github.com/lf-edge/ekuiper/v2/internal/pkg/def" @@ -290,6 +290,15 @@ func ValidateRuleOption(option *def.RuleOption) error { if err := schedule.ValidateRanges(option.CronDatetimeRange); err != nil { errs = errors.Join(errs, fmt.Errorf("validate cronDatetimeRange failed, err:%v", err)) } + if option.Qos >= def.AtLeastOnce { + if option.DisableBufferFullDiscard == nil { + t := true + option.DisableBufferFullDiscard = &t + } else if !*option.DisableBufferFullDiscard { + Log.Warnf("QoS is %d but disableBufferFullDiscard is explicitly set to false, this may lead to silent data loss during congestion.", option.Qos) + errs = errors.Join(errs, fmt.Errorf("invalidDisableBufferFullDiscard:disableBufferFullDiscard must be true when qos is %d or higher", def.AtLeastOnce)) + } + } return errs } diff --git a/internal/conf/conf_test.go b/internal/conf/conf_test.go index e1803d600c..00d25af174 100644 --- a/internal/conf/conf_test.go +++ b/internal/conf/conf_test.go @@ -107,6 +107,26 @@ func TestRuleOptionValidate(t *testing.T) { }, }, }, + { + s: &def.RuleOption{ + Qos: def.AtLeastOnce, + }, + e: &def.RuleOption{ + Qos: def.AtLeastOnce, + DisableBufferFullDiscard: func() *bool { b := true; return &b }(), + }, + }, + { + s: &def.RuleOption{ + Qos: def.AtLeastOnce, + DisableBufferFullDiscard: func() *bool { b := false; return &b }(), + }, + e: &def.RuleOption{ + Qos: def.AtLeastOnce, + DisableBufferFullDiscard: func() *bool { b := false; return &b }(), + }, + err: "invalidDisableBufferFullDiscard:disableBufferFullDiscard must be true when qos is 1 or higher", + }, } fmt.Printf("The test bucket size is %d.\n\n", len(tests)) for i, tt := range tests { diff --git a/internal/pkg/def/rule.go b/internal/pkg/def/rule.go index 7e3221a288..7a88be2f51 100644 --- a/internal/pkg/def/rule.go +++ b/internal/pkg/def/rule.go @@ -42,7 +42,7 @@ type RuleOption struct { CronDatetimeRange []schedule.DatetimeRange `json:"cronDatetimeRange,omitempty" yaml:"cronDatetimeRange,omitempty"` PlanOptimizeStrategy *PlanOptimizeStrategy `json:"planOptimizeStrategy,omitempty" yaml:"planOptimizeStrategy,omitempty"` NotifySub bool `json:"notifySub,omitempty" yaml:"notifySub,omitempty"` - DisableBufferFullDiscard bool `json:"disableBufferFullDiscard,omitempty" yaml:"disableBufferFullDiscard,omitempty"` + DisableBufferFullDiscard *bool `json:"disableBufferFullDiscard,omitempty" yaml:"disableBufferFullDiscard,omitempty"` EnableSaveStateBeforeStop bool `json:"enableSaveStateBeforeStop,omitempty" yaml:"enableSaveStateBeforeStop,omitempty"` ForceExitTimeout cast.DurationConf `json:"forceExitTimeout,omitempty" yaml:"forceExitTimeout,omitempty"` Experiment *ExpOpts `json:"experiment,omitempty" yaml:"experiment,omitempty"` diff --git a/internal/processor/rule_test.go b/internal/processor/rule_test.go index 241d6c6272..423e89e562 100644 --- a/internal/processor/rule_test.go +++ b/internal/processor/rule_test.go @@ -143,14 +143,15 @@ func TestRuleActionParse_Apply(t *testing.T) { }, }, Options: &def.RuleOption{ - IsEventTime: true, - LateTol: cast.DurationConf(time.Second), - Concurrency: 1, - BufferLength: 10240, - SendMetaToSink: false, - Qos: def.ExactlyOnce, - CheckpointInterval: cast.DurationConf(time.Minute), - SendError: false, + IsEventTime: true, + LateTol: cast.DurationConf(time.Second), + Concurrency: 1, + BufferLength: 10240, + SendMetaToSink: false, + Qos: def.ExactlyOnce, + CheckpointInterval: cast.DurationConf(time.Minute), + SendError: false, + DisableBufferFullDiscard: boolPtr(true), RestartStrategy: &def.RestartStrategy{ Attempts: 0, }, @@ -257,3 +258,7 @@ func TestAllRules(t *testing.T) { } assert.Equal(t, expected, all) } + +func boolPtr(b bool) *bool { + return &b +} diff --git a/internal/topo/node/node.go b/internal/topo/node/node.go index ae68327f69..c86947a37e 100644 --- a/internal/topo/node/node.go +++ b/internal/topo/node/node.go @@ -66,7 +66,7 @@ func newDefaultNode(name string, options *def.RuleOption) *defaultNode { outputs: make(map[string]chan any), concurrency: c, sendError: options.SendError, - disableBufferFullDiscard: options.DisableBufferFullDiscard, + disableBufferFullDiscard: options.DisableBufferFullDiscard != nil && *options.DisableBufferFullDiscard, } } @@ -98,6 +98,9 @@ func (o *defaultNode) GetName() string { func (o *defaultNode) SetQos(qos def.Qos) { o.qos = qos + if qos >= def.AtLeastOnce { + o.disableBufferFullDiscard = true + } } func (o *defaultNode) GetMetrics() []any { diff --git a/internal/topo/node/node_test.go b/internal/topo/node/node_test.go index 4f9ee8b856..cba1c6fa48 100644 --- a/internal/topo/node/node_test.go +++ b/internal/topo/node/node_test.go @@ -113,3 +113,19 @@ func TestMultipleOutputsBroadcast(t *testing.T) { }) } } + +func TestSetQos(t *testing.T) { + tests := []struct { + qos def.Qos + expectedDisableBufferFullDiscard bool + }{ + {qos: def.AtMostOnce, expectedDisableBufferFullDiscard: false}, + {qos: def.AtLeastOnce, expectedDisableBufferFullDiscard: true}, + {qos: def.ExactlyOnce, expectedDisableBufferFullDiscard: true}, + } + for _, tt := range tests { + n := newDefaultNode("test", &def.RuleOption{}) + n.SetQos(tt.qos) + assert.Equal(t, tt.expectedDisableBufferFullDiscard, n.disableBufferFullDiscard) + } +} diff --git a/internal/topo/planner/planner.go b/internal/topo/planner/planner.go index 5b97dc83c8..9a564bd074 100644 --- a/internal/topo/planner/planner.go +++ b/internal/topo/planner/planner.go @@ -506,7 +506,7 @@ func CreateLogicalPlan(stmt *ast.SelectStatement, opt *def.RuleOption, store kv. } func checkSharedSourceOption(streams []*streamInfo, opt *def.RuleOption) error { - if !opt.DisableBufferFullDiscard { + if opt.DisableBufferFullDiscard == nil || !*opt.DisableBufferFullDiscard { return nil } for _, stream := range streams { diff --git a/internal/topo/planner/planner_test.go b/internal/topo/planner/planner_test.go index 9da9d5b43e..6215b131c5 100644 --- a/internal/topo/planner/planner_test.go +++ b/internal/topo/planner/planner_test.go @@ -4672,8 +4672,9 @@ func TestCheckSharedSourceOption(t *testing.T) { }, }, } + b := true r1 := &def.RuleOption{ - DisableBufferFullDiscard: true, + DisableBufferFullDiscard: &b, } require.Error(t, checkSharedSourceOption(s1, r1)) } diff --git a/internal/topo/topotest/rule_test.go b/internal/topo/topotest/rule_test.go index 6e1a3dfec5..83d7ba836a 100644 --- a/internal/topo/topotest/rule_test.go +++ b/internal/topo/topotest/rule_test.go @@ -1245,6 +1245,7 @@ func TestSingleSQL(t *testing.T) { }, } HandleStream(true, streamList, t) + bTrue := true options := []*def.RuleOption{ { BufferLength: 100, @@ -1252,7 +1253,7 @@ func TestSingleSQL(t *testing.T) { PlanOptimizeStrategy: &def.PlanOptimizeStrategy{ EnableIncrementalWindow: true, }, - DisableBufferFullDiscard: true, + DisableBufferFullDiscard: &bTrue, }, { BufferLength: 100, @@ -1262,7 +1263,7 @@ func TestSingleSQL(t *testing.T) { PlanOptimizeStrategy: &def.PlanOptimizeStrategy{ EnableIncrementalWindow: true, }, - DisableBufferFullDiscard: true, + DisableBufferFullDiscard: &bTrue, }, { BufferLength: 100, @@ -1272,7 +1273,7 @@ func TestSingleSQL(t *testing.T) { PlanOptimizeStrategy: &def.PlanOptimizeStrategy{ EnableIncrementalWindow: true, }, - DisableBufferFullDiscard: true, + DisableBufferFullDiscard: &bTrue, }, } for _, opt := range options {