PutStore address uniqueness check is racy under concurrent registration
- Dominant language
- Go
- Stars
- 1.2k
- Forks
- 783
- Avg merge
- 5d 21h
- Merged PRs (30d)
- 36
Description
## Bug Report
### What did you do?
While reviewing #11007 (fix for #11002), @lhy1024 pointed out that PD's store address / peer-address uniqueness check in `putStoreImpl` is a classic check-then-write path and is not atomic with the store write.
Reproduction sketch (two concurrent gRPC `PutStore` calls with different store IDs but the same non-empty `peer_address`, against the same PD leader):
1. Request A and request B both call `GetStores()` and observe no conflicting live store.
2. Both pass the uniqueness validation.
3. Both proceed to `setStore()` / `SaveStoreMeta()` under different store-ID keys.
4. Both succeed, so two live stores with the same `peer_address` (or cross-colliding `address` / `peer_address`) are persisted.
Notes:
- This race already exists for the historical main-`address` uniqueness check; #11007 does not introduce the window, but the new peer-address validation inherits the same limitation.
- Existing `storeStateLock` is keyed by store ID, so it does not serialize conflicting registrations that use different store IDs.
- Related discussion: https://github.com/tikv/pd/pull/11007#discussion_r3702918476
### What did you expect to see?
Address / peer-address uniqueness validation and the subsequent store write should be atomic from the caller's perspective: at most one of two concurrent conflicting `PutStore` requests should succeed; the other should fail with a duplicated address / peer-address error.
### What did you see instead?
Serialized conflicting registrations are rejected correctly, but concurrent conflicting registrations can both pass the in-memory snapshot check and both persist. The resulting cluster can still end up with two live stores publishing the same Raft-facing address, which can misroute Raft traffic (same class of impact as #11002).
### What version of PD are you using (`pd-server -V`)?
e3d71823d855f31e15e01c966a0a0629171c18fc
Contributor guide
Research direction
Start at PD's putStoreImpl path and trace GetStores(), setStore(), and SaveStoreMeta(), then inspect how storeStateLock is keyed by store ID. Review the related discussion in PR #11007 before choosing an atomicity approach. Done means conflicting concurrent PutStore requests cannot both persist, with one returning the duplicated address or peer-address error.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- backend-api-design, distributed-systems
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100