Skip to content

Comments

*: upgrade golang to 1.25.5#63475

Merged
ti-chi-bot[bot] merged 56 commits intopingcap:masterfrom
solotzg:swisstable-go1.25
Dec 10, 2025
Merged

*: upgrade golang to 1.25.5#63475
ti-chi-bot[bot] merged 56 commits intopingcap:masterfrom
solotzg:swisstable-go1.25

Conversation

@solotzg
Copy link
Contributor

@solotzg solotzg commented Sep 11, 2025

What problem does this PR solve?

Security vulnerabilities

Issue Number: close #62981
Issue Number: ref #62487

  • From go 1.24.0 golang's switch its builtin map to swiss table: https://go.dev/blog/swisstable.
  • TiDB manually estimates a map's memory usage for memory control, since the implementation of map is changed, we need adopt the memory estimation for the new map.
  • CRDB uses another specified variables allocator to allocate and release memory used by a Map.
    • Such allocator is supposed to have the ability to record memory usage info.

What changed and how does it work?

Solution

  • Remove all bInMap and calculate approximate heap size dynamically
    • Capacity of swisstable is 1024(maxTableCapacity) which means the overhead of split(rehash) is quite low
    • It is difficult to simply determine the timing of each swisstable split(growthLeft == 0) when dirLen is large
  • Opportunities to recalculate size:
    • Increment of max len reaches 1024 or double len
    • Approximate size: groupSize * maxLen * 204 / 1000
      • 204: linear fitting of different kinds of map[K]V
  • Sampling statistics of heap size:
    • calculate approximate valid swisstable number & total heap size of map
  • Check abi each time then modify func checkMapABI before upgrading golang version

TODO

  • Upgrade client_golang and testify(some unit tests will break if using testify @ v1.11.1).

Check List

Tests

  • Unit test
  • Integration test
  • Manual test (add detailed scripts or steps below)

Benchmark

TPC-H

ENV:

  • tidb x 1: 4c 20g
  • pd x 1, tikv x 1
  • TPC-H (sf 8)
  • set global tidb_mem_quota_query=20<<30
TPC-H (sf 8) go1.23.12 go1.25.x
Q1 6.07 6.07
Q2 1.58 1.31
Q3 2.99 2.92
Q4 1.24 1.17
Q5 4.73 4.6
Q6 2.52 2.72
Q7 3.99 3.93
Q8 2.05 1.98
Q9 8.56 8.29
Q10 2.25 2.18
Q11 1.24 1.11
Q12 4.19 4.13
Q13 4.87 4.19
Q14 3.12 2.92
Q15 5.6 5.67
Q16 1.24 1.31
Q17 12.99 13.59
Q18 14.66 15.33
Q19 6.07 6.21
Q20 3.19 3.12
Q21 5.34 5.2
Q22 1.44 1.38
SUM 99.93 99.33

TPC-C

Env:

  • tidb: 4c 2g
    • no copr-cache
  • pd x 1, tikv x 1
go1.23.12
  • warehouses: 4, threads: 100, duration: 3m
    • tpmC: 22262.8, tpmTotal: 49279.3, efficiency: 43279.1%
  • warehouses: 4, threads: 200, duration: 3m
    • tpmC: 18614.0, tpmTotal: 41521.7, efficiency: 36185.8%
    • Error 8176: kill the top-1 SQL
go1.25.x
  • warehouses: 4, threads: 100, duration: 3m
    • tpmC: 22944.3, tpmTotal: 50980.4, efficiency: 44604.0%
  • warehouses: 4, threads: 200, duration: 3m
    • tpmC: 20613.8, tpmTotal: 45561.0, efficiency: 40073.5%
    • Error 8176: kill the top-1 SQL

Release note

Please refer to Release Notes Language Style Guide to write a quality release note.

Upgrade Golang version to 1.25.5 to fix security vulnerabilities.

@ti-chi-bot
Copy link

ti-chi-bot bot commented Sep 11, 2025

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@ti-chi-bot ti-chi-bot bot added release-note-none Denotes a PR that doesn't merit a release note. do-not-merge/invalid-title do-not-merge/needs-triage-completed do-not-merge/needs-tests-checked do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Sep 11, 2025
@tiprow
Copy link

tiprow bot commented Sep 11, 2025

Hi @solotzg. Thanks for your PR.

PRs from untrusted users cannot be marked as trusted with /ok-to-test in this repo meaning untrusted PR authors can never trigger tests themselves. Collaborators can still trigger tests on the PR using /test all.

I understand the commands that are listed here.

Details

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.

@solotzg solotzg changed the title [wip] executor: update golang to 1.25.0 Sep 11, 2025
@solotzg solotzg marked this pull request as ready for review September 16, 2025 06:06
@ti-chi-bot ti-chi-bot bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 16, 2025
@ti-chi-bot ti-chi-bot bot added the component/dumpling This is related to Dumpling of TiDB. label Sep 16, 2025
@solotzg solotzg force-pushed the swisstable-go1.25 branch 3 times, most recently from 0a3c716 to 89458bc Compare September 16, 2025 08:36
Signed-off-by: Zhigao <tongzhigao@pingcap.com>
Signed-off-by: Zhigao TONG <tongzhigao@pingcap.com>
@solotzg solotzg requested a review from D3Hunter December 9, 2025 08:59
@solotzg
Copy link
Contributor Author

solotzg commented Dec 9, 2025

/cc @bb7133

@ti-chi-bot ti-chi-bot bot requested a review from bb7133 December 9, 2025 08:59
@D3Hunter
Copy link
Contributor

D3Hunter commented Dec 9, 2025

/approve

@ti-chi-bot
Copy link

ti-chi-bot bot commented Dec 9, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: D3Hunter, hawkingrei, windtalker, xzhangxian1008
Once this PR has been reviewed and has the lgtm label, please assign bb7133, d3hunter for approval. For more information see the Code Review Process.
Please ensure that each of them provides their approval before proceeding.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@solotzg
Copy link
Contributor Author

solotzg commented Dec 10, 2025

/unhold

@ti-chi-bot ti-chi-bot bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 10, 2025
hawkingrei and others added 4 commits December 10, 2025 13:43
Signed-off-by: Weizhen Wang <wangweizhen@pingcap.com>
Signed-off-by: Weizhen Wang <wangweizhen@pingcap.com>
Signed-off-by: Weizhen Wang <wangweizhen@pingcap.com>
@solotzg
Copy link
Contributor Author

solotzg commented Dec 10, 2025

/retest

@solotzg solotzg changed the title *: upgrade golang to 1.25.x *: upgrade golang to 1.25.5 Dec 10, 2025
@ti-chi-bot
Copy link
Member

fast put it into merge queue, skip wait for approval of go.mod change

@ti-chi-bot ti-chi-bot bot merged commit ab31468 into pingcap:master Dec 10, 2025
34 checks passed
@solotzg solotzg mentioned this pull request Dec 10, 2025
@solotzg solotzg deleted the swisstable-go1.25 branch December 10, 2025 09:51
@ti-chi-bot ti-chi-bot bot added the needs-cherry-pick-release-8.5 Should cherry pick this PR to release-8.5 branch. label Dec 15, 2025
@ti-chi-bot
Copy link
Member

In response to a cherrypick label: new pull request created to branch release-8.5: #65043.
But this PR has conflicts, please resolve them!

ti-chi-bot pushed a commit to ti-chi-bot/tidb that referenced this pull request Dec 15, 2025
Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>
ti-chi-bot added a commit that referenced this pull request Dec 16, 2025
* 1: go1.25.

Signed-off-by: Zhigao TONG <tongzhigao@pingcap.com>

* 2: CP #65047

Signed-off-by: Zhigao TONG <tongzhigao@pingcap.com>

* update bazel config

Signed-off-by: Weizhen Wang <wangweizhen@pingcap.com>

* 3: txntest_test timeout = "long"

Signed-off-by: Zhigao TONG <tongzhigao@pingcap.com>

* 4: analyzetest_test, sessiontest_test

Signed-off-by: Zhigao TONG <tongzhigao@pingcap.com>

---------

Signed-off-by: Zhigao TONG <tongzhigao@pingcap.com>
Signed-off-by: Weizhen Wang <wangweizhen@pingcap.com>
Co-authored-by: Zhigao TONG <tongzhigao@pingcap.com>
Co-authored-by: Weizhen Wang <wangweizhen@pingcap.com>
@ti-chi-bot ti-chi-bot bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-none Denotes a PR that doesn't merit a release note. labels Dec 29, 2025
lcwangchao pushed a commit to lcwangchao/tidb that referenced this pull request Jan 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved component/dumpling This is related to Dumpling of TiDB. lgtm needs-cherry-pick-release-8.5 Should cherry pick this PR to release-8.5 branch. ok-to-test Indicates a PR is ready to be tested. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TestAggPartialResultMapperB will fail after upgrading go 1.25