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

Consider making kv.dist_sender.concurrency_limit cluster setting public in at least v24.3, possibly v24.1+ #135615

Open
rmloveland opened this issue Nov 18, 2024 · 2 comments
Open

Consider making kv.dist_sender.concurrency_limit cluster setting public in at least v24.3, possibly v24.1+ #135615

rmloveland opened this issue Nov 18, 2024 · 2 comments
Labels
C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-kv KV Team

Comments

@rmloveland
Copy link
Collaborator

rmloveland commented Nov 18, 2024 •
edited by exalate-issue-sync bot
Loading

We are working on adding it to docs as a performance recommendation in some scenarios (see PR cockroachdb/docs#19096 for details)

via @sean- in discussion on the docs PR:

we decided to not backport the change per discussion in: #131535 (comment)

however, perhaps we would be willing to flip the setting to public on v24.1 and v24.2 even if we don't want to change the default

Jira issue: CRDB-44656

Epic CRDB-43580
@rmloveland rmloveland added the C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) label Nov 18, 2024
@rmloveland rmloveland mentioned this issue Nov 18, 2024
Merged
@yuzefovich yuzefovich added the T-kv KV Team label Nov 18, 2024
@rmloveland
Copy link
Collaborator Author

update: we should also do kv.streamer.concurrency_limit since it's also mentioned in linked docs PR

arulajmani added a commit to arulajmani/cockroach that referenced this issue Nov 27, 2024
@arulajmani
kvclient: make kv.dist_sender.concurrency_limit public
c7a959e 
Closes cockroachdb#135615

Release note: None
@arulajmani arulajmani mentioned this issue Nov 27, 2024
Closed
arulajmani added a commit to arulajmani/cockroach that referenced this issue Nov 27, 2024
@arulajmani
kvclient: make kv.dist_sender.concurrency_limit public
029456b 
While here, also do the same for kv.streamer.concurrency_limit.
Closes cockroachdb#135615

Release note: None
arulajmani added a commit to arulajmani/cockroach that referenced this issue Nov 28, 2024
@arulajmani
kvclient: make kv.dist_sender.concurrency_limit public
faddf88 
Closes cockroachdb#135615
Release note: None
craig bot pushed a commit that referenced this issue 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]>
@arulajmani
Copy link
Collaborator

@rmloveland, it turns out that these two settings take their default value from GOMAXPROCS, which is dependent on the machine they run on. This makes making the setting public hard, as the generated docs expect a constant value for public cluster settings.

@arulajmani arulajmani removed their assignment Nov 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-kv KV Team
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

kvclient: make kv.dist_sender.concurrency_limit public arulajmani/cockroach
3 participants
@rmloveland @yuzefovich @arulajmani