-
Notifications
You must be signed in to change notification settings - Fork 3.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
server: DB Console causing cockroach process to crash #71728
Labels
C-bug
Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.
Comments
Azhng
added
C-bug
Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.
T-server-and-security
DB Server & Security
labels
Oct 19, 2021
craig bot
pushed a commit
that referenced
this issue
Oct 20, 2021
68370: kvserver: avoid exceeding `TargetBytes` r=nvanbenschoten,yuzefovich a=erikgrinaker **kvserver: avoid exceeding `TargetBytes`** Previously, `Header.TargetBytes` would always overshoot the target, i.e. a scan would terminate only after the result set hit or exceeded `TargetBytes`. This patch changes the behavior to never overshoot, except for when the first result exceeds `TargetBytes` (to make sure clients can always make progress). However, this is not backwards-compatible with the 21.2 `DistSender` logic, which relies on responses overshooting `TargetBytes` when enforcing limits for split batches. It therefore adds a corresponding `TargetBytesAvoidExcess` version gate that only enables this for 22.1, and `MVCCScanOptions.TargetBytesAvoidExcess` to vary the scan behavior based on it. Release note: None **kvserver: add `TargetBytesAllowEmpty` option** The `Header.TargetBytes` request limit will be overshot if the first result exceeds it, to ensure the client can make progress. However, an upcoming [parallel get/scan streaming library][1] needs to be able to disable this behavior to avoid exceeding memory budgets when dispatching parallel requests. This patch adds a field `TargetBytesAllowEmpty` which will allow an empty response if the first result exceeds `TargetBytes`. This only has an effect for `Get` and `Scan` requests, and only on 22.1 clusters since 21.2 `DistSender` logic relies on the limit always being overshot. It is not supported for `Export` requests where it will return an error. [1]: https://github.com/cockroachdb/cockroach/blob/master/docs/RFCS/20210617_index_lookups_memory_limits.md Resolves #68050. Release note: None **kvserver: add `ResponseHeader.ResumeNextBytes`** This patch adds the field `ResponseHeader.ResumeNextBytes` which may be populated with the size of the next result in the `ResumeSpan` if `ResumeReason` is `RESUME_BYTE_LIMIT` (i.e. if `TargetBytes` was exceeded). This will be needed by an upcoming [parallel get/scan streaming library][1] to better manage its memory budget. The field is best-effort, and may be omitted in some cases where the result size exactly equals `TargetBytes` and we would have to do additional work to obtain `ResumeNextBytes` (e.g. at the end of a range, where we would have to send an additional RPC to the next range). [1]: https://github.com/cockroachdb/cockroach/blob/master/docs/RFCS/20210617_index_lookups_memory_limits.md Release note: None **kvclient: rename replyResults to replyKeys** Release note: None 71739: server: fix panic on invalid session cookie. r=azhng a=dhartunian A recent change (#70792) added some code to deal with duplicate cookies in the HTTP header on DB Console requests to account for situations where someone may have multiple `session` cookies in addition to the CRDB one. That change failed to properly check for a dangling `err` value outside of a loop before proceeding. The tests have been modified to exercise the code that resulted in a panic. The bad cookie header is now also appended to unauthenticated requests in the test to manufacture a situation where only invalid cookies are in the header. Resolves #71728 Release note: None Co-authored-by: Erik Grinaker <[email protected]> Co-authored-by: David Hartunian <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
C-bug
Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.
Describe the problem
Opening DB Console crashes Cockroach process on master
To Reproduce
Open DB Console at
localhost:8080
Expected behavior
DB Console loads
Additional data / screenshots
Stack trace
The text was updated successfully, but these errors were encountered: