Skip to content

Adapt EloqStore IO QoS configuration - #537

Draft
thweetkomputer wants to merge 2 commits into
mainfrom
agent/adapt-eloqstore-io-qos-config
Draft

Adapt EloqStore IO QoS configuration#537
thweetkomputer wants to merge 2 commits into
mainfrom
agent/adapt-eloqstore-io-qos-config

Conversation

@thweetkomputer

Copy link
Copy Markdown
Collaborator

Context

EloqStore's io_qos branch replaces the obsolete per-write-task batch knob with per-device rate limiting and an optional in-flight IO window. Data Substrate needs to expose those options through its gflags/INI integration before EloqKV and other consumers can configure them.

Behavior before and after

Before this change, EloqStoreConfig could not pass the new IO QoS settings to eloqstore::KvOptions, and still exposed eloq_store_max_write_batch_pages, which EloqStore now treats as deprecated and ineffective.

After this change, command-line flags and [store] INI values can configure:

  • eloq_store_disk_rate_limit_iops
  • eloq_store_disk_rate_limit_mbps
  • eloq_store_rate_limit_burst_ms
  • eloq_store_rate_limit_io_unit
  • eloq_store_rate_bg_ratio
  • eloq_store_max_inflight_io

The obsolete eloq_store_max_write_batch_pages integration is removed.

Implementation

  • Add gflags matching the new KvOptions defaults.
  • Preserve the existing precedence rule: explicit command-line flag, then INI value, then gflag default.
  • Parse rate_limit_io_unit through the existing size parser so values such as 2KB map to bytes.
  • Pass per-device limits through unchanged; EloqStore divides them across store paths and shards internally.
  • Advance the EloqStore submodule to the io_qos implementation commit.

Design decisions and alternatives

The adapter does not divide the new rate limits by eloq_store_worker_num. These are defined by EloqStore as per-disk values and EloqStore owns the path/shard allocation. Dividing them again in Data Substrate would under-limit IO.

Test plan

  • Unit/TCL tests
  • Integration or manual validation
  • Formatting/build checks
  • Compatibility or performance validation

Commands and results:

cmake --build bld --target data_substrate --parallel 4
# Built target data_substrate

cmake --build bld --target eloqkv --parallel 4
# Built target eloqkv

git -C data_substrate diff --check
# passed

git diff --check
# passed

clang-format-18 and clang-format were unavailable in the current environment. Runtime and performance tests were not run because this PR only wires configuration into the already-tested EloqStore implementation.

Risk assessment

The main operational risk is misconfigured device limits. The defaults mirror EloqStore, 0 retains the documented disable behavior where applicable, and command-line precedence remains compatible. Removing eloq_store_max_write_batch_pages means deployments still setting that obsolete flag must remove it rather than silently relying on a no-op.

This draft currently points at an EloqStore io_qos branch commit. The EloqStore change must land first, then this PR's submodule pointer must be updated to the final commit reachable from EloqStore's target branch.

Rollback plan

Revert this PR and restore the previous EloqStore submodule pointer.

Reviewer guide

Start with store_handler/eloq_data_store_service/eloq_store_config.cpp. Verify the flag defaults against eloqstore::KvOptions, the command-line-over-INI precedence, and that per-disk rates are not divided in the adapter. Then verify the EloqStore submodule commit and dependency order.

Follow-up work

Expose the same settings in each parent project's sample INI and update the submodule pointer after the EloqStore PR lands.

@coderabbitai

coderabbitai Bot commented Jul 30, 2026

Copy link
Copy Markdown

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: c827886b-6e75-4e12-a5ea-1084565278b5

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@CLAassistant

CLAassistant commented Jul 30, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@thweetkomputer
thweetkomputer force-pushed the agent/adapt-eloqstore-io-qos-config branch from 609ddb3 to ceffdc8 Compare July 30, 2026 00:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants