Skip to content
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions dm/metrics/alertmanager/dm_worker.rules.yml
Original file line number Diff line number Diff line change
Expand Up @@ -125,12 +125,12 @@ groups:
summary: dm relay binlog file not catch up master server exceed 10 min

- alert: DM_binlog_file_gap_between_master_syncer
expr: dm_syncer_binlog_file{node="master"} - ON(instance, task, job) dm_syncer_binlog_file{node="syncer"} > 1
expr: (dm_syncer_binlog_file{node="master"} - ON(instance, task, job) dm_syncer_binlog_file{node="syncer"} > 1) and ON(instance, task, job) (dm_worker_task_state == 2)
for: 10m
labels:
env: ENV_LABELS_ENV
level: critical
expr: dm_syncer_binlog_file{node="master"} - ON(instance, task, job) dm_syncer_binlog_file{node="syncer"} > 1
expr: (dm_syncer_binlog_file{node="master"} - ON(instance, task, job) dm_syncer_binlog_file{node="syncer"} > 1) and ON(instance, task, job) (dm_worker_task_state == 2)
annotations:
description: 'cluster: ENV_LABELS_ENV, instance: {{ $labels.instance }}, task: {{ $labels.task }}, values: {{ $value }}'
value: '{{ $value }}'
Expand Down
99 changes: 99 additions & 0 deletions dm/metrics/alertmanager/dm_worker_rules_test.go

@D3Hunter D3Hunter Jul 31, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is not the right place to write tests, and the test seems not that necessary

Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
// Copyright 2026 PingCAP, Inc.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// See the License for the specific language governing permissions and
// limitations under the License.

package alertmanager

import (
"os"
"testing"
"time"

"github.com/prometheus/prometheus/promql"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ [Major] Test-only PromQL coverage changes repository-wide production dependencies

Why
The new alert-rule test imports the full Prometheus PromQL engine from the root module, which also upgrades dependencies used by production TiFlow code outside DM. Go's module selection is repository-wide rather than test-only, so production binaries will compile against automemlimit v0.5.0 instead of v0.2.4 and golang-lru v0.6.0 instead of v0.5.1. Those releases change cgroup v1/hybrid/no-limit detection and move LRU eviction callbacks outside the cache lock, making a monitoring-only fix alter unrelated runtime behavior.

Scope
dm/metrics/alertmanager/dm_worker_rules_test.go:21; go.mod:11; go.mod:56; go.mod:82; pkg/util/memory.go:34; cdc/sink/dmlsink/mq/dmlproducer/pulsar_dml_producer.go:99; cdc/sink/ddlsink/mq/ddlproducer/pulsar_ddl_producer.go:121; cdc/sink/dmlsink/txn/mysql/mysql.go:158

Risk if unchanged
DM and TiCDC binaries can regress in memory-limit selection on cgroup v1 or hybrid hosts and in eviction or close concurrency for Pulsar producers, prepared statements, and other sink caches. These behavior changes would ship as part of a two-line alert fix even though the new validation exercises only PromQL, expanding the compatibility surface and making failures difficult to attribute.

Evidence
The only new Prometheus import is the test import on this line, while the root go.mod changes 27 requirement lines, adds roughly 20 other module entries, and go.sum changes 384 lines. Production code calls memlimit.FromCgroup from pkg/util/memory.go, whose result controls the redo applier memory cap, and several cdc/sink packages use the upgraded LRU library. No cgroup-mode, redo memory-limit, Pulsar eviction, prepared-statement eviction, or concurrency test is changed in this PR.

Change request
Please keep the production dependency graph unchanged by running the rule test through isolated Prometheus tooling, such as the existing promtool rule-test format or a dedicated test module. If the runtime dependency upgrades are intentional, split them into a dedicated change and add focused coverage for cgroup v1, v2, hybrid, and no-limit detection as well as the affected LRU eviction and sink-cache concurrency paths.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’ll remove PromQL evaluation and keep lightweight YAML assertions. I won’t add promtool tests because downloading promtool adds over a minute to test execution.

"github.com/stretchr/testify/require"
"gopkg.in/yaml.v2"
)

const (
binlogFileGapAlertName = "DM_binlog_file_gap_between_master_syncer"
binlogFileGapAlertExpr = `(dm_syncer_binlog_file{node="master"} - ON(instance, task, job) dm_syncer_binlog_file{node="syncer"} > 1) and ON(instance, task, job) (dm_worker_task_state == 2)`
)

type alertRule struct {
Alert string `yaml:"alert"`
Expr string `yaml:"expr"`
For string `yaml:"for"`
Labels map[string]string `yaml:"labels"`
Annotations map[string]string `yaml:"annotations"`
}

type alertRulesFile struct {
Groups []struct {
Name string `yaml:"name"`
Rules []alertRule `yaml:"rules"`
} `yaml:"groups"`
}

func TestBinlogFileGapAlertRuleRequiresRunningTask(t *testing.T) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this test test anything except that the yaml contains xxx

rule := loadAlertRule(t, "dm_worker.rules.yml", binlogFileGapAlertName)

require.Equal(t, binlogFileGapAlertExpr, rule.Expr)
require.Equal(t, binlogFileGapAlertExpr, rule.Labels["expr"])
require.Equal(t, "10m", rule.For)

engine := promql.NewEngine(promql.EngineOpts{
MaxSamples: 10000,
Timeout: time.Second,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 [Minor] One-second wall-clock query deadline makes the rule test load-sensitive

Why
The tested alert expression has no timing semantics, but the engine is configured with a one-second real-time deadline. PromQL applies this setting through context.WithTimeout during query execution, so a constrained or race-enabled CI worker can fail the test because of host speed rather than an incorrect rule.

Scope
dm/metrics/alertmanager/dm_worker_rules_test.go:55

Risk if unchanged
The new regression test can fail intermittently under CPU contention, reducing confidence in the alert-rule signal and creating non-reproducible CI failures.

Evidence
EngineOpts.Timeout is set to time.Second; the upstream PromQL test setup in the selected v0.50.1 dependency uses 10-100 second safety timeouts for comparable engine-backed tests. The fixture itself is deterministic and does not require a one-second behavioral boundary.

Change request
Please use a generous safety timeout consistent with the upstream PromQL tests so this case fails on expression behavior, not machine scheduling speed.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PromQL evaluation and one-second timeout will be removed, so this issue no longer applies.

EnableAtModifier: true,
EnableNegativeOffset: true,
NoStepSubqueryIntervalFn: func(int64) int64 {
return time.Minute.Milliseconds()
},
})

promql.RunTest(t, `
load 1m
dm_syncer_binlog_file{instance="worker-1",job="dm_worker",node="master",source_id="mysql-01",task="running"} 12
dm_syncer_binlog_file{instance="worker-1",job="dm_worker",node="syncer",source_id="mysql-01",task="running"} 10
dm_worker_task_state{instance="worker-1",job="dm_worker",source_id="mysql-01",task="running",worker="worker-1"} 2
dm_syncer_binlog_file{instance="worker-2",job="dm_worker",node="master",source_id="mysql-02",task="paused"} 12
dm_syncer_binlog_file{instance="worker-2",job="dm_worker",node="syncer",source_id="mysql-02",task="paused"} 10
dm_worker_task_state{instance="worker-2",job="dm_worker",source_id="mysql-02",task="paused",worker="worker-2"} 3
dm_syncer_binlog_file{instance="worker-3",job="dm_worker",node="master",source_id="mysql-03",task="stopped"} 12
dm_syncer_binlog_file{instance="worker-3",job="dm_worker",node="syncer",source_id="mysql-03",task="stopped"} 10

eval instant at 0m `+rule.Expr+`
{instance="worker-1",job="dm_worker",task="running"} 2
`, engine)
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

func loadAlertRule(t *testing.T, filename, alertName string) alertRule {
t.Helper()

content, err := os.ReadFile(filename)
require.NoError(t, err)

var ruleFile alertRulesFile
require.NoError(t, yaml.Unmarshal(content, &ruleFile))

for _, group := range ruleFile.Groups {
for _, rule := range group.Rules {
if rule.Alert == alertName {
require.NotEmpty(t, rule.Expr)
return rule
}
}
}

t.Fatalf("alert rule %q not found in %s", alertName, filename)
return alertRule{}
}
27 changes: 22 additions & 5 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ require (
github.com/BurntSushi/toml v1.6.0
github.com/DATA-DOG/go-sqlmock v1.5.0
github.com/IBM/sarama v1.41.2
github.com/KimMachineGun/automemlimit v0.2.4
github.com/KimMachineGun/automemlimit v0.5.0
github.com/VividCortex/mysqlerr v1.0.0
github.com/apache/pulsar-client-go v0.13.0
github.com/aws/aws-sdk-go v1.55.7
Expand Down Expand Up @@ -53,7 +53,7 @@ require (
github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0
github.com/grpc-ecosystem/grpc-gateway v1.16.0
github.com/grpc-ecosystem/grpc-gateway/v2 v2.27.1
github.com/hashicorp/golang-lru v0.5.1
github.com/hashicorp/golang-lru v0.6.0
github.com/imdario/mergo v0.3.16
github.com/integralist/go-findroot v0.0.0-20160518114804-ac90681525dc
github.com/jarcoal/httpmock v1.2.0
Expand All @@ -79,6 +79,7 @@ require (
github.com/pingcap/tidb/pkg/parser v0.0.0-20260616034257-3adc8ec71ef3
github.com/prometheus/client_golang v1.23.0
github.com/prometheus/client_model v0.6.2
github.com/prometheus/prometheus v0.50.1
github.com/r3labs/diff v1.1.0
github.com/rcrowley/go-metrics v0.0.0-20250401214520-65e299d6c5c9
github.com/robfig/cron v1.2.0
Expand Down Expand Up @@ -142,6 +143,7 @@ require (
github.com/GoogleCloudPlatform/opentelemetry-operations-go/detectors/gcp v1.30.0 // indirect
github.com/GoogleCloudPlatform/opentelemetry-operations-go/exporter/metric v0.51.0 // indirect
github.com/GoogleCloudPlatform/opentelemetry-operations-go/internal/resourcemapping v0.51.0 // indirect
github.com/alecthomas/units v0.0.0-20231202071711-9a357b53e9c9 // indirect
github.com/alibabacloud-go/alibabacloud-gateway-spi v0.0.5 // indirect
github.com/alibabacloud-go/darabonba-openapi/v2 v2.0.11 // indirect
github.com/alibabacloud-go/debug v1.0.1 // indirect
Expand Down Expand Up @@ -174,22 +176,28 @@ require (
github.com/aws/aws-sdk-go-v2/service/sso v1.30.5 // indirect
github.com/aws/aws-sdk-go-v2/service/ssooidc v1.35.10 // indirect
github.com/aws/aws-sdk-go-v2/service/sts v1.41.2 // indirect
github.com/bboreham/go-loser v0.0.0-20230920113527-fcc2c21820a3 // indirect
github.com/bytedance/sonic v1.9.1 // indirect
github.com/chenzhuoyu/base64x v0.0.0-20221115062448-fe3a3abad311 // indirect
github.com/clbanning/mxj/v2 v2.7.0 // indirect
github.com/cncf/xds/go v0.0.0-20251210132809-ee656c7534f5 // indirect
github.com/cockroachdb/fifo v0.0.0-20240606204812-0bbfbd93a7ce // indirect
github.com/cockroachdb/tokenbucket v0.0.0-20230807174530-cc333fc44b06 // indirect
github.com/containerd/cgroups/v3 v3.0.3 // indirect
github.com/dennwc/varint v1.0.0 // indirect
github.com/eapache/go-xerial-snappy v0.0.0-20230731223053-c322873962e3 // indirect
github.com/edsrzf/mmap-go v1.1.0 // indirect
github.com/envoyproxy/go-control-plane/envoy v1.36.0 // indirect
github.com/envoyproxy/protoc-gen-validate v1.3.0 // indirect
github.com/facette/natsort v0.0.0-20181210072756-2cd4dd1e2dcb // indirect
github.com/felixge/httpsnoop v1.0.4 // indirect
github.com/fsnotify/fsnotify v1.7.0 // indirect
github.com/gabriel-vasile/mimetype v1.4.2 // indirect
github.com/getsentry/sentry-go v0.27.0 // indirect
github.com/go-asn1-ber/asn1-ber v1.5.4 // indirect
github.com/go-jose/go-jose/v4 v4.1.3 // indirect
github.com/go-kit/log v0.2.1 // indirect
github.com/go-ldap/ldap/v3 v3.4.4 // indirect
github.com/go-logfmt/logfmt v0.6.0 // indirect
github.com/go-logr/logr v1.4.3 // indirect
github.com/go-logr/stdr v1.2.2 // indirect
github.com/go-resty/resty/v2 v2.11.0 // indirect
Expand All @@ -198,26 +206,35 @@ require (
github.com/google/flatbuffers v25.9.23+incompatible // indirect
github.com/google/gofuzz v1.2.0 // indirect
github.com/google/s2a-go v0.1.9 // indirect
github.com/grafana/regexp v0.0.0-20221122212121-6b5c0a4cb7fd // indirect
github.com/hamba/avro/v2 v2.30.0 // indirect
github.com/influxdata/tdigest v0.0.1 // indirect
github.com/jellydator/ttlcache/v3 v3.0.1 // indirect
github.com/jfcg/sixb v1.3.8 // indirect
github.com/jfcg/sorty/v2 v2.1.0 // indirect
github.com/joomcode/errorx v1.0.1 // indirect
github.com/jpillora/backoff v1.0.0 // indirect
github.com/klauspost/asmfmt v1.3.2 // indirect
github.com/klauspost/cpuid/v2 v2.3.0 // indirect
github.com/ks3sdklib/aws-sdk-go v1.2.9 // indirect
github.com/kylelemons/godebug v1.1.0 // indirect
github.com/minio/asm2plan9s v0.0.0-20200509001527-cdd76441f9d8 // indirect
github.com/minio/c2goasm v0.0.0-20190812172519-36a3d3bbc4f3 // indirect
github.com/moby/patternmatcher v0.6.1 // indirect
github.com/moby/sys/sequential v0.7.0 // indirect
github.com/moby/sys/user v0.4.1 // indirect
github.com/mohae/deepcopy v0.0.0-20170929034955-c48cc78d4826 // indirect
github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect
github.com/mwitkow/go-conntrack v0.0.0-20190716064945-2f068394615f // indirect
github.com/ncruces/go-strftime v0.1.9 // indirect
github.com/oasdiff/yaml v0.0.0-20250309154309-f31be36b4037 // indirect
github.com/oasdiff/yaml3 v0.0.0-20250309153720-d2182401db90 // indirect
github.com/oklog/ulid v1.3.1 // indirect
github.com/pbnjay/memory v0.0.0-20210728143218-7b4eea64cf58 // indirect
github.com/perimeterx/marshmallow v1.1.5 // indirect
github.com/pingcap/metering_sdk v0.0.0-20260324055927-14fead745f1d // indirect
github.com/planetscale/vtprotobuf v0.6.1-0.20240319094008-0393e58bdf10 // indirect
github.com/prometheus/common/sigv4 v0.1.0 // indirect
github.com/qri-io/jsonpointer v0.1.1 // indirect
github.com/qri-io/jsonschema v0.2.1 // indirect
github.com/robfig/cron/v3 v3.0.1 // indirect
Expand Down Expand Up @@ -252,6 +269,7 @@ require (
gopkg.in/ini.v1 v1.67.0 // indirect
k8s.io/api v0.29.11 // indirect
k8s.io/apimachinery v0.29.11 // indirect
k8s.io/client-go v0.29.11 // indirect
k8s.io/klog/v2 v2.120.1 // indirect
k8s.io/utils v0.0.0-20230726121419-3b25d923346b // indirect
sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect
Expand Down Expand Up @@ -284,12 +302,11 @@ require (
github.com/carlmjohnson/flagext v0.21.0 // indirect
github.com/cespare/xxhash/v2 v2.3.0 // indirect
github.com/cheggaaa/pb/v3 v3.0.8 // indirect
github.com/cilium/ebpf v0.4.0 // indirect
github.com/cilium/ebpf v0.11.0 // indirect
github.com/cloudfoundry/gosigar v1.3.6 // indirect
github.com/cockroachdb/errors v1.11.3 // indirect
github.com/cockroachdb/logtags v0.0.0-20230118201751-21c54148d20b // indirect
github.com/cockroachdb/redact v1.1.5 // indirect
github.com/containerd/cgroups v1.0.4 // indirect
github.com/coocood/bbloom v0.0.0-20190830030839-58deb6228d64 // indirect
github.com/coocood/freecache v1.2.1 // indirect
github.com/coocood/rtutil v0.0.0-20190304133409-c84515f646f2 // indirect
Expand Down
Loading
Loading