-
Notifications
You must be signed in to change notification settings - Fork 293
chroe(*): Upgrade TiDB dependencies and adapt to API changes to ensure compatibility with the latest TiDB APIs and internal refactorings. #12306
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
- Updated Go version from 1.23.8 to 1.23.12 and toolchain to go1.24.4. - Migrated `session.SetSchemaLease` to `vardef.SetSchemaLease` due to TiDB refactoring. - Migrated `ddl.JobTableID` to `metadef.TiDBDDLJobTableID` due to TiDB refactoring. - Migrated TiDB topology information paths from `infosync` to `serverinfo` due to TiDB refactoring. - Updated various other Go module dependencies to newer versions. Signed-off-by: tenfyzhong <[email protected]>
- Remove checksums for unused or outdated module versions. Signed-off-by: tenfyzhong <[email protected]>
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary of Changes
Hello @tenfyzhong, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request focuses on maintaining the health and compatibility of TiFlow by updating its core dependencies, particularly those related to TiDB and Etcd. The primary goal is to ensure that TiFlow can seamlessly interact with newer versions of these upstream projects, benefiting from their latest features, performance enhancements, and bug fixes, while also reducing accumulated technical debt. The changes involve both module version bumps and necessary code adjustments to align with updated APIs.
Highlights
- Dependency and Toolchain Upgrades: The pull request updates the Go version from 1.23.8 to 1.23.12 and introduces
toolchain go1.24.4
. It also updates numerous direct and indirect Go module dependencies, including core components likegithub1.rensumo.top/pingcap/tidb
,github.com/tikv/client-go/v2
,github.com/tikv/pd/client
, andgo.etcd.io/etcd
related modules, ensuring compatibility and leveraging the latest improvements. - TiDB API Adaptations: The codebase has been adapted to recent API changes in the TiDB repository. Specifically,
session.SetSchemaLease
calls were migrated tovardef.SetSchemaLease
,ddl.JobTableID
was updated tometadef.TiDBDDLJobTableID
, andinfosync.TopologyInformationPath
/infosync.TopologySessionTTL
were changed to useserverinfo
equivalents.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command>
or @gemini-code-assist <command>
. Below is a summary of the supported commands.
Feature | Command | Description |
---|---|---|
Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/
folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request effectively upgrades TiDB dependencies and adapts the codebase to the corresponding API changes. The modifications are clean and directly address the compatibility issues arising from the dependency updates. I have one suggestion to improve the maintainability of a code comment by using a permanent link to a specific commit instead of a branch. Overall, this is a solid and necessary update.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
/retest |
/retest-required |
- Align DDL `ADD COLUMN IF NOT EXISTS` parsing with TiDB's behavior. - The `/*T! ... */` syntax is used by TiDB to handle `IF NOT EXISTS` clauses. Signed-off-by: tenfyzhong <[email protected]>
- The parser now includes an extra space within the `/*T! ... */` comment for `IF NOT EXISTS` in `ALTER TABLE ADD COLUMN`. - This change updates the test case to match the new expected output format. Signed-off-by: tenfyzhong <[email protected]>
… EXISTS - The parser now represents `IF NOT EXISTS` for `ADD INDEX` using the `/*T! ... */` syntax. - Update the test case to reflect the correct re-serialized form of the statement. - This ensures the test accurately validates the parser's handling of TiDB-specific syntax. Signed-off-by: tenfyzhong <[email protected]>
/retest-required |
1 similar comment
/retest-required |
- Add an extra space in the `IF NOT EXISTS` comment within the expected SQL string. - This ensures the test case accurately reflects the parser's output for consistency. Signed-off-by: tenfyzhong <[email protected]>
- The parser now represents `IF NOT EXISTS` for foreign key constraints using a TiDB-specific comment `/*T! ... */`. - This update ensures the test case reflects the correct and current output format. Signed-off-by: tenfyzhong <[email protected]>
- The parser now canonicalizes `IF EXISTS` and `IF NOT EXISTS` clauses by wrapping them in `/*T! ... */` comments. - This update ensures the test cases reflect the new canonical form of these statements. Signed-off-by: tenfyzhong <[email protected]>
- The `SetSchemaLease` function has been relocated from the `session` package to the `vardef` package in the upstream TiDB project. - Update test files in `cdc/entry` to use the new `vardef.SetSchemaLease` function to align with the upstream change. Signed-off-by: tenfyzhong <[email protected]>
- Replace `tifilter` import with `metadef` - Use `metadef` constants for system schema names to align with updated definitions Signed-off-by: tenfyzhong <[email protected]>
I have found all callers and converted their |
/retest-required |
5 similar comments
/retest-required |
/retest-required |
/retest-required |
/retest-required |
/retest-required |
- Enforce static analysis checks as part of the standard `make check` command. - Ensures code quality and consistency are verified automatically. Signed-off-by: tenfyzhong <[email protected]>
/retest-required |
3 similar comments
/retest-required |
/retest-required |
/retest-required |
…ency - Removed the explicit `concurrency 2` flag to allow `golangci-lint` to utilize available CPU resources more effectively by default. - Increased the `golangci-lint` timeout from 60 minutes to 120 minutes to prevent timeouts on larger codebases or slower CI environments. Signed-off-by: tenfyzhong <[email protected]>
/retest-required |
- Upgrade `golangci-lint` to `v1.64.8` to leverage the latest features, bug fixes, and performance improvements. - Enable `--new-from-merge-base master` for `golangci-lint` runs, which significantly speeds up static checks in CI by only linting new code. - Reduce the `golangci-lint` timeout from `120m0s` to `10m0s` due to the faster linting process on new code. - Update various Go module dependencies in `tools/check/go.mod` and `go.sum`. Signed-off-by: tenfyzhong <[email protected]>
- The `go.sum` file was updated to include a new entry for `golang.org/x/term v0.30.0`. This ensures module integrity after a dependency update or addition. Signed-off-by: tenfyzhong <[email protected]>
/retest-required |
- Added `time` command to `golangci-lint run` calls in the `check-static` target. - This helps in monitoring the duration of static analysis checks, which can be useful for performance optimization and identifying slow linting stages. Signed-off-by: tenfyzhong <[email protected]>
- Add `golangci-lint version` command to the `check-static` target. - This helps in debugging CI failures by providing visibility into the exact linter version being used. Signed-off-by: tenfyzhong <[email protected]>
- Ensures that the entire Git history is available for subsequent build steps. - This is often necessary for tools that rely on git tags or commit history for versioning or other operations. Signed-off-by: tenfyzhong <[email protected]>
ffa2aa8
to
b0e9f52
Compare
- The previous 10-minute timeout for golangci-lint was insufficient, leading to frequent timeouts in CI. - This change increases the timeout to 120 minutes to allow the linter to complete its analysis. Signed-off-by: tenfyzhong <[email protected]>
- The `-v` flag provides more detailed output from `golangci-lint`. - This can be useful for debugging linting issues or understanding the linter's execution during static checks. Signed-off-by: tenfyzhong <[email protected]>
@tenfyzhong: The following test failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
What problem does this PR solve?
The primary problem addressed by this PR is keeping TiFlow's dependencies up-to-date with upstream projects, particularly TiDB and Etcd. Outdated dependencies can lead to:
This PR updates the Go modules and adapts the TiFlow codebase to recent API changes in the TiDB repository, ensuring continued compatibility and leveraging the latest improvements from our dependencies.
Issue Number: close #12305
What is changed and how it works?
This PR includes a significant update to Go module dependencies and corresponding code adjustments to accommodate API changes in upstream libraries, primarily TiDB.
Changes include:
1.23.8
to1.23.12
.toolchain go1.24.4
directive to enforce a specific Go toolchain version.go.mod
andgo.sum
. Key updates include:github.com/pingcap/errors
: updated tov0.11.5-0.20250523034308-74f78ae071ee
github.com/pingcap/kvproto
: updated tov0.0.0-20250728031536-f08901d17bf4
github.com/pingcap/log
: updated tov1.1.1-0.20250514022801-14f3b4ca066e
github.com/pingcap/tidb
and its sub-packages (pkg/parser
): updated tov1.1.0-beta.0.20250901173104-9695b09e847c
github.com/spf13/pflag
: updated tov1.0.7
github.com/tikv/client-go/v2
: updated tov2.0.8-0.20250828075934-b794d681774f
github.com/tikv/pd/client
: updated tov0.0.0-20250703091733-dfd345b89500
go.etcd.io/etcd
related modules (api/v3
,client/pkg/v3
,client/v3
,pkg/v3
,raft/v3
,server/v3
,tests/v3
): updated tov3.5.15
go.uber.org/mock
: updated tov0.5.2
gorm.io/gorm
: updated tov1.25.12
golang.org/x/crypto
,golang.org/x/mod
,golang.org/x/net
,golang.org/x/oauth2
,golang.org/x/sync
,golang.org/x/sys
,golang.org/x/term
,golang.org/x/text
,golang.org/x/time
, andgolang.org/x/tools
.session.SetSchemaLease(time.Second)
withvardef.SetSchemaLease(time.Second)
incdc/entry/schema_test_helper.go
,dm/pkg/conn/mockdb.go
, andpkg/filter/filter_test_helper.go
. This change reflects the migration ofSetSchemaLease
to thesessionctx/vardef
package in TiDB.ddl.JobTableID
tometadef.TiDBDDLJobTableID
inpkg/spanz/span.go
. This adapts to the relocation ofJobTableID
within the TiDB metadata definitions.infosync.TopologyInformationPath
toserverinfo.TopologyInformationPath
andinfosync.TopologySessionTTL
toserverinfo.TopologySessionTTL
inpkg/upstream/topo.go
. This reflects the move of topology information constants to theserverinfo
package in TiDB.Check List
Tests
Questions
Will it cause performance regression or break compatibility?
This PR primarily updates dependencies and adapts to API changes in upstream TiDB. It should not cause performance regression. Compatibility with external systems should be maintained as the changes are internal to TiFlow's interaction with TiDB libraries. However, thorough testing is crucial to ensure no unexpected side effects.
Do you need to update user documentation, design documentation or monitoring documentation?
No, these are internal code changes and dependency updates, so no user-facing documentation updates are required.
Release note