diff --git a/AGENTS.md b/AGENTS.md index 470f8569e6..cd3a8bed80 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1,5 +1,37 @@ # Repository Guidelines +## Purpose & Boundaries + +- Help implement, review, and document focused changes in this repository. +- Follow the user's task, keep changes minimal, and avoid unrelated refactors or cleanup. +- If requirements conflict or are unclear, surface the ambiguity before changing code. +- Do not commit, branch, or publish changes unless explicitly asked. + +## Agent Change Discipline + +- Make surgical changes: touch only files required by the task. +- Do not refactor adjacent code, rename symbols, or reformat unrelated files unless the task requires it. +- Prefer the simplest solution that solves the problem; do not add speculative abstractions, features, or configurability. +- Match existing style and patterns. Remove only unused imports, variables, functions, or files introduced by your change. +- Validate changes with targeted tests or checks when practical. + +## Before Changing Code + +- Identify the objective, non-objectives, touched module, and expected validation. +- Check the relevant detailed guide before changing errors, logs, tests, generated files, or cross-component behavior. +- Prefer package-owned changes and tests before adding shared abstractions or broad integration coverage. + +## Detailed Guides + +Read these only when relevant to the task: + +- Repository map: use [docs/agents/repository-map.md](docs/agents/repository-map.md) to choose the owning module and nearby tests. +- Validation: use [docs/agents/validation.md](docs/agents/validation.md) to choose the narrowest sufficient build, test, or lint command. +- Generated code: use [docs/agents/generated-code.md](docs/agents/generated-code.md) before changing protobufs, mocks, dashboards, or generated files. +- Error handling: use predefined repository errors; see [docs/agents/error-handling.md](docs/agents/error-handling.md) before changing error creation, wrapping, or propagation. +- Logging: logs are operational signals; see [docs/agents/logging.md](docs/agents/logging.md) before adding, removing, or rewriting logs. +- Testing: prefer focused deterministic tests; see [docs/agents/testing.md](docs/agents/testing.md) before adding or changing tests. + ## Project Structure & Module Organization - `cmd/`: buildable binaries (e.g. `cmd/cdc`, `cmd/kafka-consumer`). @@ -17,23 +49,24 @@ - `make check`: pre-submit checks (fmt, tidy, codegen, dashboards, Makefile formatting). - `make unit_test`: unit tests with race + failpoints enabled (uses `--tags=intest`). - `make unit_test_pkg PKG=./pkg/sink/...`: narrow unit test scope. +- `make generate_mock`: regenerate gomock-based mocks via `scripts/generate-mock.sh`. - `make integration_test_kafka CASE=` (and `*_mysql|*_storage|*_pulsar`): run integration suites; requires binaries in `bin/` (`make check_third_party_binary`). -## Coding Style & Naming Conventions +## Go Coding Basics -- Go: keep `gofmt` clean; use `make fmt` before pushing. +- Formatting: keep `gofmt` clean; use `make fmt` before pushing. - Naming: - Functions: use camel case and **do not** include `_` (e.g. `getPartitionNum`, not `get_partition_num`). - Variables: use lowerCamelCase (e.g. `flushInterval`, not `flush_interval`). -- Logging: structured logs via `github.com/pingcap/log` + `zap` fields; message strings should **not** include function names and should avoid `-` (use spaces instead). -- Errors: when an error comes from a third party/library call, wrap it immediately with `errors.Trace(err)` or `errors.WrapError(...)` to attach a stack trace; upstream callers should propagate wrapped errors without wrapping again. Avoid using `errors.New` to create error objects; instead, utilize the predefined objects available in the `cerrors` package. - -## Testing Guidelines - -- Unit tests: `*_test.go`, favor deterministic tests; use `testify/require`. -- Failpoints: `make unit_test` enables/disables automatically. If you enable manually, disable before committing to avoid a dirty tree. +- Imports: do not rename imports unless required to resolve a package name conflict or to follow an existing local convention. ## Commit & Pull Request Guidelines - Commit/PR title format (see `CONTRIBUTING.md`): `[,subsystem2]: ` or `*: `. Subject ≤70 chars; wrap body at ~80. - PRs should follow `.github/pull_request_template.md` (include `Issue Number:` line, select tests, and fill the `release-note` block). + +## Final Response + +- Summarize changed files and behavior. +- List validation commands run and results. +- State skipped checks with a short reason, especially for documentation-only changes. diff --git a/docs/agents/error-handling.md b/docs/agents/error-handling.md new file mode 100644 index 0000000000..178d9dd992 --- /dev/null +++ b/docs/agents/error-handling.md @@ -0,0 +1,11 @@ +# Error Handling Guidelines + +Read this before adding or changing error creation, wrapping, or propagation. + +- Use predefined errors from the repository error package; keep the local import name consistent with surrounding code. +- When an error comes from a third-party or library call, wrap it immediately at the boundary with `errors.WrapError(predefinedError, err, args...)`. +- For new or changed code, do not use `errors.Trace` as the initial wrapper for third-party or library errors. If no predefined error fits, choose an existing predefined error or add an appropriate one, then use `errors.WrapError`. +- After an error has been wrapped with `errors.WrapError`, propagate it directly; do not call `errors.Trace` on later paths. +- When creating a TiCDC error, use `GenWithStack...` or `GenWithStackByArgs...` on a predefined error and pass concrete details through arguments when needed. +- Decide whether a newly generated error needs stack information. If a stack is unnecessary, especially on hot paths, use `FastGen...` or `FastGenByArgs...`. +- Avoid other error creation or wrapping styles in new or changed code, including `errors.New`, bare `fmt.Errorf`, and adding stack information multiple times. diff --git a/docs/agents/generated-code.md b/docs/agents/generated-code.md new file mode 100644 index 0000000000..86100f6b50 --- /dev/null +++ b/docs/agents/generated-code.md @@ -0,0 +1,25 @@ +# Generated Code Guidelines + +Read this before changing protobufs, mocks, generated Go files, dashboards, or files produced by `go generate`. + +## Principles + +- Prefer editing the source definition and regenerating output over manually patching generated files. +- Keep generated diffs paired with the source change that requires them. +- If generation produces unrelated churn, stop and inspect before including it. +- Do not commit local tool binaries, build outputs, coverage files, or temporary artifacts. + +## Generation Commands + +- Protobufs: run `make generate-protobuf` after changing `eventpb/**/*.proto`, `heartbeatpb/**/*.proto`, or `logservice/logservicepb/**/*.proto`. +- Mocks: run `make generate_mock` after changing interfaces listed in `scripts/generate-mock.sh` or adding a mock to that flow. +- Go generate: run `make go-generate` after changing files whose generated output is controlled by `//go:generate`. +- Next generation Grafana dashboards: run `make generate-next-gen-grafana` after changing inputs consumed by `scripts/generate-next-gen-metrics.sh`. +- Full pre-submit generation check: run `make check` when a change may affect formatting, generated files, dashboards, Makefile formatting, or module tidiness. + +## Review Checklist + +- Confirm generated files are deterministic and limited to the intended source change. +- Confirm generated files are not manually edited without a source-of-truth update. +- Confirm newly required generated files are included in the diff. +- Confirm no tool downloads, binaries, or temporary files are included. diff --git a/docs/agents/logging.md b/docs/agents/logging.md new file mode 100644 index 0000000000..45d52618f5 --- /dev/null +++ b/docs/agents/logging.md @@ -0,0 +1,19 @@ +# Logging Guidelines + +Read this before adding, removing, or rewriting logs. + +- Use structured logs via `github.com/pingcap/log` with `zap` fields. +- Treat logs as operational signals, not control-flow comments. Keep normal paths quiet. +- Default `INFO`/`WARN` logs should record high-value lifecycle events, state changes, external dependency abnormalities, or invariant violations. +- Choose log levels by required action: + - `ERROR`: correctness, availability, or key progress is affected and needs attention. + - `WARN`: the system is abnormal but can continue through recovery, retry, fallback, or degraded behavior. + - `INFO`: key lifecycle events, important state changes, important configuration, or summary information. + - `DEBUG`: bounded, low-frequency diagnostics with clear troubleshooting value. +- Do not add `DEBUG` logs by default. Delete low-value logs instead of moving them to `DEBUG`. +- Keep `message` stable and concise: summarize what happened, why it happened, and what the system will do next. +- Put object details in stable camelCase `zap` fields such as `changefeedID`, `nodeID`, `dispatcherID`, `regionID`, `subscriptionID`, and `requestID`. +- Message strings should not include function names and should avoid `-` (use spaces instead). +- Avoid per-object or per-iteration logs, duplicated logs on the same error path, large objects, raw payloads, and long error dumps in default logs. +- Use metrics for counts, scale, frequency, and trends; use windowed summaries or representative samples for high-cardinality events. +- Before adding, keeping, or rewriting a log, verify that it answers a real diagnostic question, identifies the object, reason, action, and impact, and will not grow linearly with object count or retry/loop frequency. diff --git a/docs/agents/repository-map.md b/docs/agents/repository-map.md new file mode 100644 index 0000000000..9171af73d5 --- /dev/null +++ b/docs/agents/repository-map.md @@ -0,0 +1,34 @@ +# Repository Map + +Read this when choosing where to make a change or which tests to run. + +## Runtime Components + +- `cmd/`: buildable binaries such as `cmd/cdc`, `cmd/kafka-consumer`, `cmd/storage-consumer`, and helper tools. +- `server/`: server bootstrap and runtime service wiring. +- `coordinator/`: changefeed metadata, scheduling coordination, operators, drain, and GC coordination. +- `maintainer/`: table/span replication ownership, scheduling, split/range checks, and replica lifecycle. +- `logservice/`: eventstore, logpuller, schema store, transaction utilities, and protobuf definitions for log service internals. +- `downstreamadapter/`: dispatcher orchestration, routing, event collection, sinks, and syncpoint handling. + +## Shared Libraries + +- `pkg/config`: configuration types and validation. +- `pkg/errors`: predefined TiCDC errors and error helpers. +- `pkg/sink`: shared sink implementations, codecs, and sink utilities. +- `pkg/filter`, `pkg/binlog-filter`, and `pkg/integrity`: filtering and integrity-related logic. +- `pkg/etcd`, `pkg/pdutil`, `pkg/security`, and `pkg/server`: external dependency clients and shared service utilities. +- `pkg/orchestrator`, `pkg/scheduler`, and `pkg/messaging`: shared control-plane and messaging primitives. + +## Tests and Tooling + +- `tests/integration_tests/`: script-driven integration suites for MySQL, Kafka, storage, and Pulsar. +- `scripts/`: generation, lint, formatting, and integration helper scripts. +- `tools/`: pinned local tooling used by Make targets. +- `metrics/`: Grafana and next-generation dashboard assets. + +## Placement Rules + +- Put component-owned behavior close to the owning component instead of adding cross-cutting helpers prematurely. +- Put shared code under `pkg/` only when at least two components need the same abstraction. +- Prefer existing package tests before adding a new test package or broader integration test. diff --git a/docs/agents/testing.md b/docs/agents/testing.md new file mode 100644 index 0000000000..c88d60d715 --- /dev/null +++ b/docs/agents/testing.md @@ -0,0 +1,13 @@ +# Testing Guidelines + +Read this before adding or changing tests. + +- Unit tests: `*_test.go`, favor deterministic tests; use `testify/require`. +- Unit tests should cover meaningful behavior only; avoid redundant or low-value cases. +- Do not test across feature boundaries. Keep each test focused on the behavior owned by the package or component under test. +- Reuse existing tests when possible. Add a new test only when reuse would make the existing test unclear or incomplete. +- If several test functions are highly related, merge them into one concise table-driven or scenario-based test. +- When a test needs mocked components, prefer existing gomock-generated mocks over handwritten mocks. If the required mock does not exist, add it to the mock generation flow and run `make generate_mock`. +- Keep tests efficient, simple, focused, and easy to update. +- Failpoints: `make unit_test` enables/disables automatically. If you enable manually, disable before committing to avoid a dirty tree. +- For documentation-only changes, unit tests are usually unnecessary. If tests are skipped, state in the final response that only documentation was changed. diff --git a/docs/agents/validation.md b/docs/agents/validation.md new file mode 100644 index 0000000000..1cc6626573 --- /dev/null +++ b/docs/agents/validation.md @@ -0,0 +1,32 @@ +# Validation Guidelines + +Read this before choosing build, test, lint, or integration checks. + +## Principles + +- Start with the narrowest command that covers the changed behavior, then broaden only when risk or confidence requires it. +- Prefer package-scoped unit tests for code changes and reserve full-suite or integration tests for cross-component, protocol, sink, or deployment behavior. +- If a command is too expensive or requires unavailable services, state what was not run and why in the final response. +- Do not fix unrelated failures. Capture the failing command and the first relevant failure, then report it separately. + +## Common Commands + +- Build the main binary with `make cdc`. +- Format Go, shell, imports, and log style with `make fmt`. +- Run pre-submit checks with `make check`. +- Run all unit tests with `make unit_test`. +- Run focused unit tests with `make unit_test_pkg PKG=./pkg/sink/...`. +- Run targeted integration suites with `make integration_test_mysql CASE=`, `make integration_test_kafka CASE=`, `make integration_test_storage CASE=`, or `make integration_test_pulsar CASE=`. + +## Task-to-Validation Matrix + +- `pkg/` library changes: run `make unit_test_pkg PKG=./pkg//...`; broaden to `make unit_test` for shared utilities used widely. +- `downstreamadapter/sink/` changes: run the package unit tests, then the matching sink integration suite when behavior crosses process or external-system boundaries. +- `coordinator/`, `maintainer/`, or scheduling changes: run focused package tests and consider `make unit_test` when ownership, lifecycle, or concurrency invariants change. +- `logservice/` changes: run focused logservice package tests; consider broader unit tests for eventstore, logpuller, schema, or txn boundary changes. +- API, config, or CLI changes: run focused package tests and `make cdc` when command wiring or config loading changes. +- Documentation-only changes: unit tests are usually unnecessary; run markdown or whitespace checks when practical and state that only documentation changed. + +## Reporting + +In the final response, include the commands run, their result, and any checks skipped with a short reason. diff --git a/tests/integration_tests/run_heavy_it_in_ci.sh b/tests/integration_tests/run_heavy_it_in_ci.sh index 1de5414d7c..7ae9105f96 100755 --- a/tests/integration_tests/run_heavy_it_in_ci.sh +++ b/tests/integration_tests/run_heavy_it_in_ci.sh @@ -89,7 +89,7 @@ kafka_groups=( # G10 'kafka_column_selector kafka_column_selector_avro ddl_with_random_move_table' # G11 - 'fail_over region_merge multi_changefeeds' + 'fail_over region_merge multi_changefeeds split_table_check' # G12 'ddl_for_split_tables_random_schedule' # G13 diff --git a/tests/integration_tests/run_light_it_in_ci.sh b/tests/integration_tests/run_light_it_in_ci.sh index b70dd44e6b..b70548517a 100755 --- a/tests/integration_tests/run_light_it_in_ci.sh +++ b/tests/integration_tests/run_light_it_in_ci.sh @@ -91,7 +91,7 @@ kafka_groups=( # G09 'cdc_server_tips ddl_sequence log_redaction fail_over_ddl_J' # G10 - 'changefeed_error batch_add_table fail_over_ddl_K split_table_check' + 'changefeed_error batch_add_table fail_over_ddl_K' # G11 'ddl_attributes multi_tables_ddl fail_over_ddl_L' # G12