You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@opendal.apache.org by "cool9850311 (via GitHub)" <gi...@apache.org> on 2026/10/12 15:20:37 UTC
[PR] feat(services/etcd): add max_decoding_message_size config [opendal]
cool9850311 opened a new pull request, #8396:
URL: https://github.com/apache/opendal/pull/8396
# Which issue does this PR close?
Closes #8164.
# Rationale for this change
`etcd-client` applies a 4 MiB gRPC decoding limit to KV responses. OpenDAL never
configured it, so the limit was effectively hardcoded and no OpenDAL option could
lift it, even when the etcd server was configured to accept larger payloads.
The limit affects two operations, not one:
- `read` of a value larger than 4 MiB fails with
`decoded message length too large: found 8388678 bytes, the limit is: 4194304 bytes`,
which is the failure reported in the issue.
- `list` fails the same way on a large prefix. The lister issues a keys-only range
request, and the key names alone exceed the limit once there are enough of them:
a prefix holding 30000 long keys produced a 5907968-byte keys-only response. The
issue does not mention this; I found it while reproducing.
This is a client-side response limit, so it cannot be expressed through
`Capability`: the same build can read a 4 MiB value and fail on a 5 MiB one, and
the correct value depends on how the operator's etcd server is configured. That
makes it a config option rather than a capability or a hardcoded constant.
# What changes are included in this PR?
- `EtcdConfig::max_decoding_message_size` (`Option<usize>`) with a builder setter.
Unset keeps the `etcd-client` default of 4 MiB, so existing behavior is unchanged.
- `EtcdCore::kv_client` applies the limit to the KV client that serves each request.
`max_decoding_message_size` lives on `KvClient` rather than on `ConnectOptions`,
and `Client::kv` is private, so the limit cannot be set once per connection; it
has to be applied per call site. `Client::kv_client()` is
`#[repr(transparent)]` over the shared channel, so this is a cheap clone, not a
new connection.
- Applied at the three KV read sites: `get`, `has_prefix`, and the lister.
`has_prefix` carries `with_limit(1).with_keys_only()`, so its response can never
approach the limit — it is **not** a failure site, and the limit is applied there
only so that every KV client this backend creates is configured identically.
- Set `OPENDAL_ETCD_MAX_DECODING_MESSAGE_SIZE=10485760` in the three etcd CI
fixtures, matching the `--max-request-bytes=10485760` their servers already use,
and add the variable to `.env.example`.
- Remove the etcd skip from the two .NET large-payload read tests, which were
skipped specifically because of this limit.
- Regenerated `bindings/dotnet/OpenDAL/ServiceConfig/EtcdServiceConfig.cs`,
`bindings/java/.../ServiceConfig.java` and `website/data/services.json` via
`just generate dotnet|java|docs`.
Naming: I checked how existing options are named. Where an option is a 1:1
pass-through to a named upstream API, the repo copies the upstream name and links
its docs (the moka/mini_moka options do this); where the option is OpenDAL's own
abstraction, it coins a descriptive name. This is the first case, so the option
keeps the upstream name and the doc comment links
`etcd_client::KvClient::max_decoding_message_size`.
# Are there any user-facing changes?
Yes — a new optional config option for `services-etcd`, surfaced as
`max_decoding_message_size` in the Rust config, `MaxDecodingMessageSize` in .NET,
`maxDecodingMessageSize` in Java, and `OPENDAL_ETCD_MAX_DECODING_MESSAGE_SIZE` in
the behavior-test environment. The default is unchanged.
Note that the etcd server must also be configured to allow responses of the
requested size (`--max-request-bytes`); raising only the client limit is not
enough.
# Breaking changes
# AI Usage Statement
- **Harness:** Claude Code (CLI)
- **Model:** Claude Opus 5 (`claude-opus-5`)
- **Effort:** harness default; no reasoning-effort override set
- **Role:** AI did the convention research, the code changes, and ran the
verification described below. I reviewed every change and the evidence behind
each claim before submitting.
Verified against real etcd v3.6.4 started from this repo's fixtures — standalone,
3-node cluster, and TLS with auth — not a mock or emulator:
- Baseline on unmodified `main`: 8 MiB read fails with the limit error above.
- Option set to 10 MiB: the 8 MiB round trip succeeds and the content is
byte-identical.
- Option set to 5 MiB: the failure message reports `the limit is: 5242880`,
confirming the configured value reaches the client rather than merely appearing
to work.
- Option unset: the error is byte-identical to pre-change behavior.
- Lister: the 30000-key prefix above fails before the change and returns all
30002 entries after it.
- The string-keyed config path (`Operator::via_iter`) propagates the value, which
is the path the bindings and the behavior-test harnesses use.
On the .NET skip removal: these tests use an early `return` guard, which xunit
counts as a pass, so a green run does not by itself prove the assertions executed.
I confirmed it with a negative control — with the new environment variable
removed, both tests fail at `the limit is: 4194304`; with it set, both pass. So
the CI fixture change is load-bearing, not cosmetic.
Unknowns for review:
- The 10 MiB CI value is chosen to match the fixtures' existing server-side
`--max-request-bytes`. If you would rather the fixtures exercise the default
path, the fixture change is the one to revisit.
- Removing the etcd skip left `IsService` in `BehaviorTestBase` with no remaining
call sites. I left the helper in place rather than delete a shared test helper in
this PR; happy to remove it if you prefer.
--
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: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(services/etcd): add max_decoding_message_size config [opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo merged PR #8396:
URL: https://github.com/apache/opendal/pull/8396
--
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: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(services/etcd): add max_decoding_message_size config [opendal]
Posted by "cool9850311 (via GitHub)" <gi...@apache.org>.
cool9850311 commented on PR #8396:
URL: https://github.com/apache/opendal/pull/8396#issuecomment-6135091515
The only failing check on the previous push was `core / ubuntu-latest / redis`, which is unrelated to this change. This PR touches `core/services/etcd`, the three etcd CI fixtures, `.env.example`, the .NET read behavior test, and generated binding/website files. It touches nothing under `core/core`, `core/tests`, or redis.
That job looks flaky on the redis-cluster fixture: a key the test just wrote is not visible to the read that follows. The failing test differs from run to run, and the count is the same each time (85 passed, 1 failed):
- This PR: `behavior::test_buffer_stream_metadata_with_concurrent` — `RangeNotSatisfied (persistent) at read, context: { offset: 128, size: 128, content_length: 0, ... service: redis }`.
- An unrelated PR, https://github.com/apache/opendal/actions/runs/37609830039: same job, `behavior::test_read_suffix_larger_than_file` — `NotFound (persistent) at stat ... key not found in redis`.
I re-pushed the same tree to re-trigger the workflow. Every etcd job passed on the previous push, including `binding_dotnet / ubuntu-latest / etcd / etcd` with 112 passed / 0 failed and `OPENDAL_ETCD_MAX_DECODING_MESSAGE_SIZE=10485760` present in the job environment.
Happy to open a separate issue for the redis-cluster flake if that would help.
--
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: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(services/etcd): add max_decoding_message_size config [opendal]
Posted by "cool9850311 (via GitHub)" <gi...@apache.org>.
cool9850311 commented on PR #8396:
URL: https://github.com/apache/opendal/pull/8396#issuecomment-6264332894
@Xuanwo
Hi, could you pls review this PR?
Thx.
--
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: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org