fix(rdma): v2.6.3 review fixes — real-us idle threshold, eviction UAF, mode typo guard - #272
Merged
Merged
Conversation
…, mode typo guard Review of v2.6.2 (dingodb#269/dingodb#270) found: 1. SteadyUs() returned steady_clock::count() raw ticks, which are NANOSECONDS on Linux. kEvictIdleMinUs=2,000,000 was therefore 2 ms, not 2 s: under normal traffic nearly every pooled connection counts as idle, so a full segment could evict recently-active connections and trigger reconnection storms. Now duration_cast<microseconds>. 2. Eviction woke a live_eps_ pointer whose owning Serve thread then exited and destroyed the stack endpoint — a concurrent evictor could Wake a freed endpoint (UAF). The evictor now erases the victim from live_eps_ under conn_mu_ before Wake (Serve's own erase becomes a no-op), so exactly one evictor ever touches an endpoint. 3. Total eviction wait is bounded to 5 s (was up to 32 s, exceeding the client's 10 s bootstrap). 4. last_active_us_ == 0 (inserted, not yet stamped by Serve) is never treated as idle. 5. DFKV_RAM_WRITE_MODE accepts only writeback|writearound; anything else warns and falls back to writeback (was silent writeback on typos). Verified on 0064 (64MB segment = 15 conns, t16/b1 rounds): A all-active refusal 26 fails (expected), B-F 1-2 fails each (eviction boundary), 75 evictions, zero crashes/segfaults.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Review fixes for v2.6.2 (#269/#270)
SteadyUs()returnedsteady_clock::count()raw ticks, which are nanoseconds on Linux. Under normal traffic nearly every pooled connection counts as idle → a full segment could evict recently-active connections. Nowduration_cast<microseconds>.live_eps_pointer whose Serve thread then exited and destroyed the stack endpoint; a concurrent evictor couldWakea freed endpoint. The evictor now erases the victim fromlive_eps_underconn_mu_beforeWake(Serve's own erase becomes a no-op) — exactly one evictor ever touches an endpoint.last_active_us_ == 0(inserted, not yet stamped by Serve) is never treated as idle.DFKV_RAM_WRITE_MODEtypo guard: onlywriteback|writearoundaccepted; anything else warns and falls back to writeback.Verified (0064, 64MB segment = 15 conns, t16/b1 rounds)
v2.6.2 advisory
v2.6.2 shipped #270 with the 2ms-threshold bug; do not deploy it. This fix is the corrected v2.6.3.