tso: avoid blocking split finish state reads

[rleungx]

What problem does this PR solve?

Issue Number: ref #9842

finishSplitKeyspaceGroup held the keyspace group manager lock while sending the finish split HTTP request. If the request was blocked or slow, readers such as FindGroupByKeyspaceID could not acquire RLock, which could make TSO split election checks time out.

What is changed and how does it work?

Avoid holding the keyspace group manager lock while waiting for the finish split HTTP request.

A dedicated mutex now serializes split finish requests, preserving the previous single-request behavior without blocking state readers. The keyspace group manager lock is only held briefly when reading the split finish request state and when updating the in-memory split state after the request succeeds.

Add a regression test that blocks the finish split HTTP response and verifies state reads can still acquire the manager read lock.

Check List

Tests

• Unit test
• Integration test

Release note

Fix a potential TSO keyspace group split timeout caused by blocking state reads while finishing split state.

Summary by CodeRabbit

• Refactor

  • Optimized internal lock handling during split operation completion to reduce contention and improve concurrency, allowing concurrent state access to proceed without unnecessary blocking.

• Tests

  • Added test verifying that concurrent operations are not blocked during split operation completion.

[ti-chi-bot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign okjiang 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:

• OWNERS

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

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 73155492-30b4-4c8b-85d8-7af880bfa087

📥 Commits

Reviewing files that changed from the base of the PR and between 4ae1408 and 8d35157.

📒 Files selected for processing (2)

• pkg/tso/keyspace_group_manager.go
• pkg/tso/keyspace_group_manager_test.go

---

📝 Walkthrough

Walkthrough

KeyspaceGroupManager gains a finishSplitMu mutex to serialize split-finish operations. finishSplitKeyspaceGroup is refactored into three steps—pre-check via getFinishSplitRequest, HTTP DELETE execution without holding the main lock, and in-memory state cleanup via completeFinishSplitKeyspaceGroup—preventing the HTTP call from blocking concurrent state reads.

Changes

Split-finish lock contention refactor

Layer / File(s)	Summary
New mutex, refactored flow, and state helpers pkg/tso/keyspace_group_manager.go	Adds finishSplitMu sync.Mutex to KeyspaceGroupManager. Refactors finishSplitKeyspaceGroup to acquire finishSplitMu and release kgm.Lock before the HTTP DELETE. Extracts getFinishSplitRequest (validates split-target state and returns HTTP client + URL) and completeFinishSplitKeyspaceGroup (acquires kgm.Lock to replace the group entry and clear SplitState).
Concurrency test: state reads not blocked during HTTP call pkg/tso/keyspace_group_manager_test.go	Adds TestFinishSplitKeyspaceGroupDoesNotBlockStateReads with net/http and net/http/httptest imports. Starts a blocking httptest server, calls finishSplitKeyspaceGroup asynchronously, asserts that a concurrent RLock/read is not stalled while the HTTP call is in flight, then releases the server and verifies the split-target state is cleared.

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant finishSplitKeyspaceGroup
  participant getFinishSplitRequest
  participant completeFinishSplitKeyspaceGroup
  participant PDServer as PD HTTP Server

  Caller->>finishSplitKeyspaceGroup: invoke
  finishSplitKeyspaceGroup->>finishSplitKeyspaceGroup: lock finishSplitMu
  finishSplitKeyspaceGroup->>getFinishSplitRequest: kgm.RLock, validate split target & httpClient
  getFinishSplitRequest-->>finishSplitKeyspaceGroup: (client, url, ok) then kgm.RUnlock
  note over finishSplitKeyspaceGroup,PDServer: kgm.Lock NOT held during HTTP call
  finishSplitKeyspaceGroup->>PDServer: HTTP DELETE /split
  PDServer-->>finishSplitKeyspaceGroup: response
  finishSplitKeyspaceGroup->>completeFinishSplitKeyspaceGroup: kgm.Lock, replace kgs[id], clear SplitState
  completeFinishSplitKeyspaceGroup-->>finishSplitKeyspaceGroup: kgm.Unlock
  finishSplitKeyspaceGroup->>finishSplitKeyspaceGroup: unlock finishSplitMu
  finishSplitKeyspaceGroup-->>Caller: return nil

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested labels

size/XL

Suggested reviewers

• JmPotato
• okJiang

Poem

🐇 A lock held too long makes the server quite slow,
So I carved out a mutex to let reads still flow.
The HTTP call dances without blocking the state,
And a test with a gate confirms: concurrent reads are great!
Hop, hop—no contention, the split finishes fine! 🎉

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The PR title clearly summarizes the main change: avoiding blocking split finish state reads by refactoring lock management in the keyspace group manager.
Description check	✅ Passed	The PR description includes all required sections: the problem statement (issue #9842), detailed explanation of changes with commit message, comprehensive checklist items, and appropriate release notes.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[rleungx]

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

[codecov]

Codecov Report

❌ Patch coverage is 78.26087% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.22%. Comparing base (4ae1408) to head (8d35157).
⚠️ Report is 11 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #10931      +/-   ##
==========================================
+ Coverage   79.15%   79.22%   +0.06%     
==========================================
  Files         540      540              
  Lines       74801    74813      +12     
==========================================
+ Hits        59212    59268      +56     
+ Misses      11405    11366      -39     
+ Partials     4184     4179       -5     

Flag	Coverage Δ
unittests	79.22% <78.26%> (+0.06%)	⬆️

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.

[ti-chi-bot]

@rleungx: 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-3	8d35157	link	true	/test pull-unit-test-next-gen-3

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.