mfyuce commented on PR #3517: URL: https://github.com/apache/iggy/pull/3517#issuecomment-4779704356
Sorry for the late response -- was doing a local benchmark to make sure the setup is working. @hubcio good point -- kernel version string parsing is fragile. The concern about custom kernels is also valid. One clarification on the `io-uring` crate approach: `IORING_SETUP_COOP_TASKRUN` is a *setup parameter* passed to `io_uring_setup()`, not an `IORING_FEAT_*` flag returned by `io_uring_get_probe()`. So `is_feature_*` on the `io-uring` crate wouldn't directly answer "does this kernel support COOP_TASKRUN as a setup flag?". The only runtime probe for a setup flag is to attempt ring creation with it and catch `EINVAL` -- which lives inside compio's ring initialization, not in iggy. Options as I see them: 1. **Keep the kernel check (current PR)** -- pragmatic, gives operators a clear error instead of a cryptic `EINVAL`. The >= 6.8 threshold is where io_uring is mature enough for Iggy's workload anyway (per @numinnex's earlier point). 2. **Push the fix into compio** -- have compio attempt `COOP_TASKRUN` and silently fall back on `EINVAL`. Cleaner, no version parsing, handles custom kernels. Requires an upstream compio PR. 3. **Close this PR** -- hard-document 6.8+ as the minimum kernel and leave it to the operator. No code change needed. Happy to go with whichever direction the team prefers. If option 2 is the path, I can draft a compio PR, but that would delay this fix since it depends on an external release. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
