Skip to content
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.

Sign up for GitHub

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

Jump to bottom

kvclient: make kv.dist_sender.concurrency_limit public #136299

Closed
wants to merge 1 commit into from
Closed

kvclient: make kv.dist_sender.concurrency_limit public #136299

wants to merge 1 commit into from

Conversation

arulajmani
Copy link
Collaborator

@arulajmani arulajmani commented Nov 27, 2024 •
edited
Loading

While here, also do the same for kv.streamer.concurrency_limit.

Closes #135615
Release note: None

@arulajmani arulajmani requested review from nicktrav and kvoli November 27, 2024 15:38
@arulajmani arulajmani requested a review from a team as a code owner November 27, 2024 15:38
@blathers-crl blathers[crl]
Copy link

blathers-crl bot commented Nov 27, 2024

It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR?

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

@cockroach-teamcity
Copy link
Member

This change is Reviewable

@arulajmani arulajmani added backport-24.3.x Flags PRs that need to be backported to 24.3 backport-24.1.x Flags PRs that need to be backported to 24.1. backport-24.2.x labels Nov 27, 2024
@arulajmani arulajmani requested a review from a team as a code owner November 27, 2024 15:47
@arulajmani arulajmani requested review from mw5h and removed request for a team November 27, 2024 15:47
kvoli
kvoli approved these changes Nov 27, 2024
View reviewed changes
Copy link
Collaborator

@kvoli kvoli left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 4 of 4 files at r1, all commit messages.
Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @mw5h and @nicktrav)

@arulajmani
Copy link
Collaborator Author

Turns out, kv.streamer.concurrency_limit draws its default value from GOMAXPROCS, which makes automatically generating docs for it impossible. I'll just leave that out of this commit, I don't think adding docs for this setting is worth whatever change that'll be required to address this snowflake.

TFTR!

bors r=kvoli

craig bot pushed a commit that referenced this pull request Nov 28, 2024
@arulajmani
Merge #136299
011c330 
136299: kvclient: make kv.dist_sender.concurrency_limit public r=kvoli a=arulajmani

While here, also do the same for kv.streamer.concurrency_limit.

Closes #135615
Release note: None

Co-authored-by: Arul Ajmani <[email protected]>
@craig
Copy link
Contributor

craig bot commented Nov 28, 2024

Build failed:

@arulajmani
Copy link
Collaborator Author

Turns out, kv.streamer.concurrency_limit draws its default value from GOMAXPROCS, which makes automatically generating docs for it impossible.

Heh, the same applies to kv.dist_sender.concurrency_limit as well.

@arulajmani arulajmani closed this Nov 28, 2024
@rmloveland rmloveland mentioned this pull request Dec 2, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kvoli kvoli kvoli approved these changes

@nicktrav nicktrav Awaiting requested review from nicktrav

@mw5h mw5h Awaiting requested review from mw5h mw5h was automatically assigned from cockroachdb/sql-queries-prs

Assignees
No one assigned
Labels
backport-24.1.x Flags PRs that need to be backported to 24.1. backport-24.3.x Flags PRs that need to be backported to 24.3
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Consider making kv.dist_sender.concurrency_limit cluster setting public in at least v24.3, possibly v24.1+
3 participants
@arulajmani @cockroach-teamcity @kvoli