Skip to content

kvserver: allow exceeding MaxSpanRequestKeys limit in some cases#92883

Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom
yuzefovich:kvserver
Dec 2, 2022
Merged

kvserver: allow exceeding MaxSpanRequestKeys limit in some cases#92883
craig[bot] merged 1 commit intocockroachdb:masterfrom
yuzefovich:kvserver

Conversation

@yuzefovich
Copy link
Copy Markdown
Member

This commit removes an assertion that MaxSpanRequestKeys is not exceeded. In particular, this limit can be exceeded when SQL asks for full SQL rows (by specifying non-zero WholeRowsOfSize) and doesn't allow for empty responses (by specifying AllowEmpty as false). Note that this code path hasn't been hit so far in production because the only user of WholeRowsOfSize (the streamer) currently doesn't set MaxSpanRequestKeys.

Informs: #82323.

Release note: None

This commit removes an assertion that `MaxSpanRequestKeys` is not
exceeded. In particular, this limit can be exceeded when SQL asks for
full SQL rows (by specifying non-zero `WholeRowsOfSize`) and doesn't
allow for empty responses (by specifying `AllowEmpty` as `false`). Note
that this code path hasn't been hit so far in production because the
only user of `WholeRowsOfSize` (the streamer) currently doesn't set
`MaxSpanRequestKeys`.

Release note: None
@yuzefovich yuzefovich requested review from a team and erikgrinaker December 2, 2022 04:01
@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

Copy link
Copy Markdown
Contributor

@erikgrinaker erikgrinaker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I would be ok with just removing the assertion outright as well, but this works.

@yuzefovich
Copy link
Copy Markdown
Member Author

The assertion seems somewhat useful to me, so let's keep it for now.

TFTR!

bors r+

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Dec 2, 2022

Build succeeded:

@craig craig bot merged commit a475947 into cockroachdb:master Dec 2, 2022
@yuzefovich yuzefovich deleted the kvserver branch December 2, 2022 18:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants