Skip to content

Add runtime setters for MULTI_POLL reactor count, dispatch, and request ceiling - #1049

Merged
rapids-bot[bot] merged 7 commits into
rapidsai:release/26.10from
Matt711:fea/add-set-get-apis
Sep 10, 2026
Merged

rapids-bot[bot] merged 7 commits into
rapidsai:release/26.10from
Matt711:fea/add-set-get-apis

Conversation

@Matt711

@Matt711 Matt711 commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Adds setters for remote_io_num_reactors, remote_io_reactor_dispatch, and remote_io_max_concurrent_requests

@Matt711
Matt711 requested review from a team as code owners August 27, 2026 21:50
@Matt711 Matt711 added feature request New feature or request non-breaking Introduces a non-breaking change labels Aug 27, 2026

@madsbk madsbk left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Overall looks good.

  • update the stale sentence at runtime_settings.rst:82
  • add missing docs for three newly user-facing settings

Comment thread cpp/src/defaults.cpp
Comment thread cpp/include/kvikio/defaults.hpp Outdated
Comment on lines +494 to +495
* Takes effect only if set before the `MULTI_POLL` reactor pool is first used (the pool, like
* the default thread pool, is created lazily on first use and is never rebuilt).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: Like Mads, I think we should throw if this set call would not take effect.

Comment thread cpp/include/kvikio/defaults.hpp Outdated
Comment on lines +518 to +519
* Takes effect only if set before the `MULTI_POLL` reactor pool is first used (the pool, like
* the default thread pool, is created lazily on first use and is never rebuilt).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same here.

Comment thread cpp/include/kvikio/defaults.hpp Outdated
Comment on lines +550 to +551
* Takes effect only if set before the `MULTI_POLL` reactor pool is first used (the pool, like
* the default thread pool, is created lazily on first use and is never rebuilt).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

And here.

Comment thread cpp/src/defaults.cpp
Comment thread cpp/include/kvikio/defaults.hpp Outdated
* runtime, overriding `KVIKIO_REMOTE_IO_NUM_REACTORS`.
*
* Takes effect only if set before the `MULTI_POLL` reactor pool is first used (the pool, like
* the default thread pool, is created lazily on first use and is never rebuilt).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I agree with the suggestion. Just want to point out that for the BS thread pool used for local I/O and easy backend, we do support changing the number of threads midway at runtime, which would block the calling thread, drain the task queue, destroy the worker threads, and then recreate a new set of worker threads. So I think the phrasing "like the default thread pool" should be removed from the doc string here.

Comment thread cpp/src/defaults.cpp
madsbk
madsbk previously requested changes Aug 28, 2026

@madsbk madsbk left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Since we cannot reset the pool size, the defaults context manager is now broken:

with kvikio.defaults.set("remote_io_num_reactors", 8):
    remote_file.pread(...)

It fails when trying to reset remote_io_num_reactors on exit.

@Matt711

Matt711 commented Sep 2, 2026

Copy link
Copy Markdown
Contributor Author
kvikio.defaults.set("remote_io_num_reactors", 8)

Can we just not support context managers for these settings? I did that in 7373370

@Matt711
Matt711 requested review from madsbk and wence- September 2, 2026 22:10
Comment thread python/kvikio/kvikio/defaults.py Outdated
Comment thread python/kvikio/tests/test_defaults.py
Comment thread python/kvikio/kvikio/defaults.py
@wence-
wence- dismissed madsbk’s stale review September 3, 2026 08:39

Subsumed by other review.

@Matt711
Matt711 marked this pull request as draft September 10, 2026 14:55
@copy-pr-bot

copy-pr-bot Bot commented Sep 10, 2026

Copy link
Copy Markdown

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

@Matt711
Matt711 changed the base branch from main to release/26.10 September 10, 2026 14:55
@Matt711
Matt711 requested a review from wence- September 10, 2026 14:55
@Matt711
Matt711 marked this pull request as ready for review September 10, 2026 14:55
@Matt711

Matt711 commented Sep 10, 2026

Copy link
Copy Markdown
Contributor Author

/merge

@rapids-bot
rapids-bot Bot merged commit 3ea0db0 into rapidsai:release/26.10 Sep 10, 2026
68 checks passed
rapids-bot Bot pushed a commit to NVIDIA/cudf that referenced this pull request Sep 10, 2026
Defaults cudf-polars to the kvikio MULTI_POLL backend with new defaults for `kvikio_task_size`, `kvikio_bounce_buffer_bytes`, `kvikio_reactor_count`, `kvikio_reactor_dispatch`, and `kvikio_request_ceiling`. These are the best defaults we've found based on benchmarking on a g7e.8xlarge instance.

| Setting | MULTI_POLL default | EASY_THREADPOOL default |
|---|---|---|
| `kvikio_remote_io_backend` | `MULTI_POLL` | `EASY_THREADPOOL` |
| `kvikio_task_size` | 16 MiB | 64 MiB |
| `kvikio_bounce_buffer_bytes` | 16 MiB | 16 MiB |
| `kvikio_nthreads` | unset (defers to kvikio's own default) | 256 |
| `kvikio_reactor_count` | 24 | not used |
| `kvikio_reactor_dispatch` | `PER_CHUNK` | not used |
| `kvikio_request_ceiling` | 256 | not used |

Notes:
- MULTI_POLL doesn't use `kvikio_nthreads` for remote I/O, it uses the reactor threads instead, so we don't force that pool to 256 threads under MULTI_POLL. 
- EASY_THREADPOOL doesn't use the reactor settings at all. 
- `kvikio_bounce_buffer_bytes` applies to both backends, it's not MULTI_POLL-specific.

- Depends on rapidsai/kvikio#1049
- Depends on #23683

Authors:
  - Matthew Murray (https://github.com/Matt711)
  - Peter Andreas Entschev (https://github.com/pentschev)

Approvers:
  - Peter Andreas Entschev (https://github.com/pentschev)

URL: #23839
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature request New feature or request non-breaking Introduces a non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants