kv: segfault in rangecache.(*EvictionToken).SyncTokenAndMaybeUpdateCache #123146
Assignees
Labels
backport-24.1.x
Flags PRs that need to be backported to 24.1.
branch-release-24.1
Used to mark GA and release blockers and technical advisories for 24.1
C-bug
Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.
GA-blocker
T-kv
KV Team
Projects
Comments
sumeerbhola
added
C-bug
andrewbaptist
added a commit
to andrewbaptist/cockroach
that referenced
this issue
Apr 26, 2024
Previously in `DistSender.sendPartialBatch` we could hit an error path during processing of a proxy request if the method was called with an invalid token. This could happen if there was a range split or merge which could invalidate the routing token prior to the method being called. This fix validates the token is not null before attempting to Sync. Epic: none Fixes: cockroachdb#123146 Release note: None
![blathers[crl]](https://avatars.githubusercontent.com/in/59700?s=40&u=373d9101a12edc3847d2fb6ae78e3adcfb4ca41e&v=4){kind=avatar}
|
Hi @andrewbaptist, please add branch-* labels to identify which branch(es) this GA-blocker affects. 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
andrewbaptist
added
branch-release-24.1
craig bot
pushed a commit
that referenced
this issue
May 2, 2024
123151: kvclient: check for invalid routing token r=arulajmani a=andrewbaptist Previously in `DistSender.sendPartialBatch` we could hit an error path during processing of a proxy request if the method was called with an invalid token. This could happen if there was a range split or merge which could invalidate the routing token prior to the method being called. This fix validates the token is not null before attempting to Sync. Epic: none Fixes: #123146 Release note: None Co-authored-by: Andrew Baptist <baptist@cockroachlabs.com>
blathers-crl bot
pushed a commit
that referenced
this issue
May 2, 2024
Previously in `DistSender.sendPartialBatch` we could hit an error path during processing of a proxy request if the method was called with an invalid token. This could happen if there was a range split or merge which could invalidate the routing token prior to the method being called. Hitting this error required a specific sequence of events. The scenario is: 1) Start the proxy request with “stale” routing info (v1) 2) Apply the changes from the client (v2) which invalidates the token due to a merge or split. 3) Attempt to directly load the routng info from meta2. 4) Fail to load the routing info with a retriable range lookup error. 5) Attempt to apply the client a second time with an invalid token. This fix only attempts to sync the token if the token is currently valid. Epic: none Fixes: #123146 Release note: None
andrewbaptist
added a commit
to andrewbaptist/cockroach
that referenced
this issue
May 13, 2024
Previously a proxy request was handled using the same logic within sendPartialBatch and sendToReplicas. There were short circuits to handle the different cases of retries, but this was still incorrect in some places. Instead this change intercepts proxy request at the start of Send and creates and sends to the transport directly. This greatly simplifies the code for proxy requests and additionally fixes complex cases where the routing information changes. Epic: none Fixes: cockroachdb#123965 Informs: cockroachdb#123146 Release note: None
andrewbaptist
added a commit
to andrewbaptist/cockroach
that referenced
this issue
May 13, 2024
Previously a proxy request was handled using the same logic within sendPartialBatch and sendToReplicas. There were short circuits to handle the different cases of retries, but this was still incorrect in some places. Instead this change intercepts proxy request at the start of Send and creates and sends to the transport directly. This greatly simplifies the code for proxy requests and additionally fixes complex cases where the routing information changes. Epic: none Fixes: cockroachdb#123965 Informs: cockroachdb#123146 Release note: None
andrewbaptist
added a commit
to andrewbaptist/cockroach
that referenced
this issue
May 13, 2024
Previously in DistSender, a proxy request was handled using the same logic within sendPartialBatch and sendToReplicas. There were short circuits to handle the different cases of retries, but in cases with changing range boundaries, it could return the wrong error. This change intercepts proxy request at the start of DistSender.Send and sends to the transport bypassing the rest of the DistSender retry logic. This simplifies the code for proxy requests. Epic: none Fixes: cockroachdb#123965 Informs: cockroachdb#123146 Release note: None
andrewbaptist
added a commit
to andrewbaptist/cockroach
that referenced
this issue
May 14, 2024
Previously in DistSender, a proxy request was handled using the same logic within sendPartialBatch and sendToReplicas. There were short circuits to handle the different cases of retries, but in cases with changing range boundaries, it could return the wrong error. This change intercepts proxy request at the start of DistSender.Send and sends to the transport bypassing the rest of the DistSender retry logic. This simplifies the code for proxy requests. Epic: none Fixes: cockroachdb#123965 Informs: cockroachdb#123146 Release note: None
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
This is running master with a severely overloaded cluster (from a cpu perspective) due to
admission-control/tpcc-severe-overload. Full dump in https://drive.google.com/file/d/1wB68PYml74nFWf3c3qxktZo6ERyhrYkB/view?usp=sharingcc @andrewbaptist
Jira issue: CRDB-38216
The text was updated successfully, but these errors were encountered: