Skip to content

Conversation

@okJiang
Copy link
Member

@okJiang okJiang commented Nov 3, 2025

What problem does this PR solve?

Issue Number: Close #10034

What is changed and how does it work?

Check List

Tests

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

Code changes

Side effects

  • Possible performance regression
  • Increased code complexity
  • Breaking backward compatibility

Related changes

Release note

None.

@ti-chi-bot
Copy link
Contributor

ti-chi-bot bot commented Nov 3, 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 do-not-merge/needs-linked-issue do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. release-note-none Denotes a PR that doesn't merit a release note. dco-signoff: yes Indicates the PR's author has signed the dco. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Nov 3, 2025
@okJiang
Copy link
Member Author

okJiang commented Nov 5, 2025

/retest

@codecov
Copy link

codecov bot commented Nov 5, 2025

Codecov Report

❌ Patch coverage is 86.53846% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.41%. Comparing base (8c350af) to head (99060be).
⚠️ Report is 10 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #9900      +/-   ##
==========================================
+ Coverage   78.32%   78.41%   +0.08%     
==========================================
  Files         511      511              
  Lines       68323    68415      +92     
==========================================
+ Hits        53514    53645     +131     
+ Misses      10923    10880      -43     
- Partials     3886     3890       +4     
Flag Coverage Δ
unittests 78.41% <86.53%> (+0.08%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@okJiang
Copy link
Member Author

okJiang commented Nov 5, 2025

/retest

2 similar comments
@okJiang
Copy link
Member Author

okJiang commented Nov 11, 2025

/retest

@okJiang
Copy link
Member Author

okJiang commented Nov 11, 2025

/retest

@okJiang okJiang marked this pull request as ready for review December 10, 2025 04:09
@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 Dec 10, 2025
@purelind
Copy link
Contributor

/test ?

@ti-chi-bot
Copy link
Contributor

ti-chi-bot bot commented Dec 10, 2025

@purelind: The following commands are available to trigger required jobs:

/test pull-build
/test pull-build-next-gen
/test pull-check-deps
/test pull-error-log-review
/test pull-integration-realcluster-test
/test pull-unit-test-next-gen-1
/test pull-unit-test-next-gen-2
/test pull-unit-test-next-gen-3

The following commands are available to trigger optional jobs:

/test pull-integration-copr-test
/test pull-integration-realcluster-test-next-gen
/test pull-unit-test

Use /test all to run the following jobs that were automatically triggered:

pull-build
pull-build-next-gen
pull-check-deps
pull-error-log-review
pull-unit-test-next-gen-1
pull-unit-test-next-gen-2
pull-unit-test-next-gen-3
tikv/pd/pull_integration_realcluster_test
tikv/pd/pull_integration_realcluster_test_next_gen
Details

In response to this:

/test ?

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.

@okJiang
Copy link
Member Author

okJiang commented Dec 10, 2025

/retest

@wuhuizuo
Copy link
Contributor

/test pull-unit-test-next-gen-1 pull-unit-test-next-gen-2 pull-unit-test-next-gen-3

@okJiang
Copy link
Member Author

okJiang commented Dec 10, 2025

/retest

test-tso-function: install-tools
# testing TSO function...
@$(FAILPOINT_ENABLE)
CGO_ENABLED=1 go test -race -tags without_dashboard,deadlock $(TSO_FUNCTION_TEST_PKGS) || { $(FAILPOINT_DISABLE); exit 1; }
Copy link
Member Author

Choose a reason for hiding this comment

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

deadlock will be disable temporarily after upgrading golang1.25.

Track this in #10035

Copy link
Member

@rleungx rleungx Dec 10, 2025

Choose a reason for hiding this comment

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

Upgrading deadlock is not useful?

Copy link
Member Author

Choose a reason for hiding this comment

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

I hadn't thought of this approach.😂

Copy link
Member Author

Choose a reason for hiding this comment

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

revert in 64cd23c

Comment on lines +441 to +442
if len(ranges) <= 1 {
if len(ranges) == 1 {
Copy link
Member

Choose a reason for hiding this comment

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

Reason for this change?

Copy link
Member Author

Choose a reason for hiding this comment

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

image

😂

Copy link
Member

Choose a reason for hiding this comment

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

Then removing the line 400 would be more consistent.

Copy link
Member Author

Choose a reason for hiding this comment

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

done 1f63640

@okJiang
Copy link
Member Author

okJiang commented Dec 10, 2025

/retest

.golangci.yml Outdated
- deprecatedComment
govet:
disable:
- buildtag
Copy link
Member

Choose a reason for hiding this comment

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

Why disable it

Copy link
Member Author

@okJiang okJiang Dec 10, 2025

Choose a reason for hiding this comment

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

I can't find a way to fix the warning in this code

//go:build !without_dashboard
// +build !without_dashboard

If enable this lint

tests/dashboard/service_test.go:16:1: buildtag: +build line is no longer needed (govet)
// +build !without_dashboard
^
1 issues:

If remove // +build !without_dashboard, it will report another error. I don't find the error, let's try it in 061ae8d

Copy link
Member

@rleungx rleungx Dec 10, 2025

Choose a reason for hiding this comment

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

Seems you have already removed it?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, after removing, the ci is still normal.

@ti-chi-bot ti-chi-bot bot added the needs-1-more-lgtm Indicates a PR needs 1 more LGTM. label Dec 10, 2025
@ti-chi-bot ti-chi-bot bot added the approved label Dec 10, 2025
@okJiang
Copy link
Member Author

okJiang commented Dec 10, 2025

/retest

- uses: actions/setup-go@v5
with:
go-version: '1.23'
go-version: '1.25.5'
Copy link
Member

@rleungx rleungx Dec 10, 2025

Choose a reason for hiding this comment

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

Shall we keep using '1.25'?

Copy link
Member Author

@okJiang okJiang Dec 10, 2025

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

It could be caused by setup-go?

@okJiang
Copy link
Member Author

okJiang commented Dec 10, 2025

/retest

2 similar comments
@okJiang
Copy link
Member Author

okJiang commented Dec 10, 2025

/retest

@okJiang
Copy link
Member Author

okJiang commented Dec 10, 2025

/retest

Signed-off-by: okjiang <819421878@qq.com>
@okJiang
Copy link
Member Author

okJiang commented Dec 11, 2025

/retest

Signed-off-by: okjiang <819421878@qq.com>
Signed-off-by: okjiang <819421878@qq.com>
@ti-chi-bot
Copy link
Contributor

ti-chi-bot bot commented Dec 11, 2025

@okJiang: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
pull-unit-test-next-gen b324e2a link true /test pull-unit-test-next-gen

Full PR test history. Your PR dashboard.

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. I understand the commands that are listed here.

Signed-off-by: okjiang <819421878@qq.com>
@okJiang
Copy link
Member Author

okJiang commented Dec 11, 2025

/retest

@okJiang
Copy link
Member Author

okJiang commented Dec 11, 2025

ping @rleungx @JmPotato

@ti-chi-bot ti-chi-bot bot added the lgtm label Dec 11, 2025
@ti-chi-bot
Copy link
Contributor

ti-chi-bot bot commented Dec 11, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: JmPotato, lhy1024

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

The pull request process is described 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

@ti-chi-bot ti-chi-bot bot removed the needs-1-more-lgtm Indicates a PR needs 1 more LGTM. label Dec 11, 2025
@ti-chi-bot
Copy link
Contributor

ti-chi-bot bot commented Dec 11, 2025

[LGTM Timeline notifier]

Timeline:

  • 2025-12-10 09:12:45.722017635 +0000 UTC m=+1032310.535795207: ☑️ agreed by lhy1024.
  • 2025-12-11 05:23:44.139225998 +0000 UTC m=+1104968.953003570: ☑️ agreed by JmPotato.

@ti-chi-bot ti-chi-bot bot merged commit 27c2705 into tikv:master Dec 11, 2025
39 of 42 checks passed
Comment on lines +57 to +59
# govet:
# disable:
# - buildtag
Copy link
Member

Choose a reason for hiding this comment

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

Why not remove it

@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 12, 2025
ti-chi-bot pushed a commit to ti-chi-bot/pd that referenced this pull request Dec 12, 2025
close tikv#10034

Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>
@ti-chi-bot
Copy link
Member

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

ti-chi-bot bot pushed a commit that referenced this pull request Dec 12, 2025
close #10034

Signed-off-by: okjiang <819421878@qq.com>

Co-authored-by: okjiang <819421878@qq.com>
okJiang added a commit to okJiang/pd that referenced this pull request Jan 30, 2026
close tikv#10034

Signed-off-by: okjiang <819421878@qq.com>

Co-authored-by: okjiang <819421878@qq.com>
Signed-off-by: okjiang <819421878@qq.com>
JmPotato pushed a commit to ti-chi-bot/pd that referenced this pull request Feb 6, 2026
close tikv#10034

Signed-off-by: okjiang <819421878@qq.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved dco-signoff: yes Indicates the PR's author has signed the dco. lgtm needs-cherry-pick-release-8.5 Should cherry pick this PR to release-8.5 branch. release-note-none Denotes a PR that doesn't merit a release note. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

upgrade golang to 1.25.5

7 participants