feat(disk): add IOPS limiters.#1087
Conversation
🎉 All green!🧪 All tests passed 🎯 Code Coverage (details) 🔗 Commit SHA: c5d64ad | Docs | Datadog PR Page | Give us feedback! |
|
@codex review |
|
Codex Review: Didn't find any major issues. 🎉 ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
I'm deploying the PR and testing it 🚧 |
cgroup rejects IOPS values of 1 at the kernel level. Separate IOPS validation from BPS: allow 0 (remove limit) or >= 2, reject only 1. Update kubebuilder Minimum=2 annotations and regenerate CRDs. Lower readIOPSPerSec example from 50 to 10 for measurable demo impact. Add disk_pressure_write_iops.yaml example with writeIOPSPerSec: 2. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Update readBytesPerSec and writeBytesPerSec kubebuilder validation minimum from 0 to 2 - Update Validate() to reject value 1 (kernel rejects cgroup BPS of exactly 1) - Allow 0 to mean "remove limit" as before - Regenerate CRD manifests to reflect new minimums Rationale: the Linux kernel rejects cgroup blkio BPS throttle values of 1; valid values are 0 (no limit) or >= 2. Enforcing this at the API level fails fast before the kernel write. This commit made by [/dd:git:commit:quick](https://github.com/DataDog/claude-marketplace/tree/main/dd/commands/git/commit/quick.md)
…elds - Changed kubebuilder minimum annotations for readBytesPerSec, writeBytesPerSec, readIOPSPerSec, writeIOPSPerSec from 2 to 0 - Regenerated CRD manifests for Disruption, DisruptionCron, and DisruptionRollout Rationale: API-level validation was too strict; cgroup IOPS minimum enforcement (>=2) belongs at the injector layer, not the CRD schema. Allowing 0 lets users express "no throttling" or disable a limit explicitly. This commit made by [/dd:git:commit:quick](https://github.com/DataDog/claude-marketplace/tree/main/dd/commands/git/commit/quick.md)
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c1210dc8f1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // clean read iops throttle | ||
| i.config.Log.Infow("cleaning disk read iops throttle", tags.DeviceKey, i.config.Informer.Source()) | ||
|
|
||
| if err := i.config.Cgroup.Write(diskPressureBlkioControllerName, i.getThrottleFilename(diskPressureThrottleModeReadIops), i.formatThrottle(0, diskPressureThrottleModeReadIops)); err != nil { |
There was a problem hiding this comment.
When a disk-pressure disruption only sets byte throttles, Inject never writes the IOPS files because the IOPS fields are nil, but Clean now always resets both IOPS limits to 0/max. On nodes where an IOPS cgroup limit already existed outside this disruption, deleting any bandwidth-only disk-pressure will clear that unrelated limit; reset these modes only when the corresponding IOPS throttle was part of the disruption, or restore the prior value.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This is true, but as a first version it's enough.
2 participants
What does this PR do?
Extends the
DiskPressuredisruption with IOPS throttling support alongside the existing bandwidth (bytes/sec) throttling.Changes
API (
api/v1beta1/disk_pressure.go)readIOPSPerSecandwriteIOPSPerSecfields toDiskPressureThrottlingSpecGenerateArgs()to emit--read-iops-per-sec/--write-iops-per-secflagsExplain()with IOPS descriptionsInjector (
injector/disk_pressure.go)diskPressureThrottleModeRead/Write→diskPressureThrottleModeReadBps/WriteBpsto disambiguate from new IOPS modesdiskPressureThrottleModeReadIops/diskPressureThrottleModeWriteIopsenum valuesblkio.throttle.read_iops_device/blkio.throttle.write_iops_deviceriops=/wiops=key-value pairs to the unifiedio.maxfileInject()andClean()handle all four throttle modesIopsKeyobservability tag for structured loggingCLI (
cli/injector/disk_pressure.go)--read-iops-per-sec/--write-iops-per-secflagsDocs & Examples
docs/disk_pressure.mdanddocs/disruption_catalogue.mdexamples/disk_pressure_read_iops.yamlexample manifestexamples/complete.yamlCRDs
chaos.datadoghq.com_disruptions.yaml,_disruptioncrons.yaml,_disruptionrollouts.yamlCode Quality Checklist
Testing
unittests orend-to-endtests.unittests orend-to-endtests.readIOPSPerSecon a cgroups v1 node → verifyblkio.throttle.read_iops_devicewrittenwriteIOPSPerSecon a cgroups v2 node → verifyio.maxcontainswiops=