gortiz commented on PR #18317:
URL: https://github.com/apache/pinot/pull/18317#issuecomment-4419892225
# Review: PR #18317 — [spi] Slim JSON serialization for *IndexConfig classes
**Repo:** apache/pinot
**Branch:** slim-indexconfig-serializers → master
**Author:** anshul98ks123 (Anshul Singh)
**Date:** 2026-05-11
**Repo context loaded:**
- [x] CLAUDE.md read
- [ ] CODEOWNERS read (not present in repo)
- [x] Module-level docs for touched paths read (pinot-spi,
pinot-segment-spi, pinot-segment-local)
---
## Stage 1: Triage & Evidence Collection
### PR description
- [x] Description present
- [x] States what problem is being solved
- [x] States why this approach
- [x] States risky areas
**Findings:** The description is exceptionally thorough. It documents eight
distinct design decisions with explicit rationale, includes before/after
wire-format examples, addresses backward compatibility, and calls out the one
manual repro not completed. No issues.
### Change summary
- [x] Full diff read
- [x] Summary produced
- [x] Files listed by module
**Summary:** Converts all 12 `*IndexConfig` POJOs from Jackson's default
bean serializer to a curated `@JsonValue toJsonObject()` method that emits only
explicitly-configured (non-null) fields. Every `primitive` field becomes a
`Wrapper` type internally; public getters keep their primitive return type and
apply the static default at read time. Adds 12 `*SerializationTest` classes and
one cross-cutting `IndexConfigSlimRoundTripTest`. Updates 30+ existing test
call sites that broke due to constructor signature changes (`false` →
`Boolean.FALSE`). Special-cases `ForwardIndexConfig` cluster-tunable fields
(always emitted) and `ForwardIndexConfig.encodingType` (always emitted per PR
#18364 contract).
**Files:**
| Module | Files |
|---|---|
| `pinot-spi` (production) | `IndexConfig.java`, `BloomFilterConfig.java`,
`JsonIndexConfig.java`, `MultiColumnTextIndexConfig.java`,
`StarTreeIndexConfig.java` |
| `pinot-segment-spi` (production) | `DictionaryIndexConfig.java`,
`ForwardIndexConfig.java`, `FstIndexConfig.java`, `RangeIndexConfig.java`,
`TextIndexConfig.java`, `H3IndexConfig.java`, `VectorIndexConfig.java` |
| `pinot-segment-local` (production) | `DictionaryIndexType.java` |
| `pinot-spi` (tests) | 5 new `*SerializationTest` classes |
| `pinot-segment-spi` (tests) | 5 new `*SerializationTest` classes,
`TextIndexConfigTest.java`, `VectorConfigTest.java`,
`ColumnIndexContainerTest.java`, `H3IndexConfigTest.java`,
`VectorIndexConfigValidatorTest.java` |
| `pinot-segment-local` (tests) | `IndexConfigSlimRoundTripTest.java` (new),
`H3IndexTest.java`, `RangeIndexTest.java`, `TextIndexTest.java`,
`VectorIndexTest.java`, `DictionaryIndexTypeTest.java`,
`ForwardIndexTypeTest.java`, and ~16 vector-related test files |
| `pinot-core` (tests) | 4 vector filter operator tests,
`FilterPlanNodeTest.java` |
| `pinot-perf` | 3 benchmark files (test-only constructor call site changes)
|
### Risk classification
- [x] Classified as medium
- [x] Reason stated
**Classification:** Medium
**Reason:** Touches SPI interfaces in `pinot-spi` and `pinot-segment-spi`
used across broker, server, controller, and minion. Changes affect ZK
serialization format (table/index config storage) and Jackson wire format for
configs round-tripped through all cluster components. The change is strictly
additive in terms of deserialization (old fat JSON still deserializes) and
strictly narrowing in serialization (fewer keys emitted). The
backwards-compatibility story is sound but the blast radius is large: any
consumer that persisted configs via these serializers and compares output by
string equality would see changes.
### Test gaps
- [x] Changed behaviors/contracts checked for corresponding tests
- [x] Test coverage of new/changed behavior verified
**Gaps:**
- Observation (low): `InvertedIndexConfig`, `NativeTextIndexConfig`, and
`H3IndexConfig` (already touched) all exist in the codebase. `H3IndexConfig`
does get a `SerializationTest`. However `InvertedIndexConfig` is not mentioned
or touched — if it extends `IndexConfig`, it now inherits `@JsonValue
toJsonObject()` without a subclass override and without a serialization test.
This is likely intentional since `InvertedIndexConfig` might add no extra
fields, but it is not documented.
- Observation (low): The `TextIndexConfig` `AbstractBuilder`
copy-constructor path now loses the null-vs-explicit distinction because
builder fields are primitives and the copy-constructor calls `other.isXxx()`
(the default-applying getter) rather than the raw wrapper field. If code calls
`new TextIndexConfigBuilder(existingConfig).build()`, the resulting config will
always emit default-valued fields as null (not emit them), even when the source
config had them as explicit. This is documented in the PR description
("programmatic builder users that want to pin explicit-default value can
construct the config directly via the @JsonCreator path"), but no test covers
the copy-constructor path.
---
## Stage 2: Deep Review
### Does the implementation address the stated problem?
- [x] Problem statement identified from the PR description
- [x] Implementation traced through the code to check whether it appears to
address the stated problem
- [x] No obvious gaps between what the PR claims to fix and what the code
actually does
**Findings:** The stated problem is that slim user-supplied JSON (e.g.
`{"compressionCodec":"SNAPPY"}`) gets fattened on first round-trip through any
`ObjectMapper`. The implementation addresses this: each class now has
`@JsonValue toJsonObject()` that emits only non-null wrapper fields. The
`ForwardIndexConfig` cluster-tunable carve-out is correct — omitting those
would cause divergent behavior across nodes. The `encodingType` always-emit
rationale references PR #18364. Both carve-outs are explicitly tested. The
`DictionaryIndexType` `null`-vs-`false` fix for legacy
`onHeapDictionaryColumns` / `varLengthDictionaryColumns` translation is
logically correct: a column absent from a legacy list was never configured, so
`null` is the right translation under the new contract.
### High-risk files
- [x] Files ranked by logical risk
- [x] Risk reasons stated per file
| File | Risk | Reason |
|---|---|---|
| `IndexConfig.java` | High | Base class for all index configs; `@JsonValue`
on base affects entire hierarchy; `equals()` contract change |
| `BloomFilterConfig.java` | High | `equals()` changed: removed
`isEnabled()` from equality contract |
| `TextIndexConfig.java` | High | Largest file change; 19-field class;
builder copy-constructor semantics changed; deprecated ctor forwarding |
| `ForwardIndexConfig.java` | Medium-high | Cluster-tunable snapshot model;
`encodingType` always-emit contract for PR #18364 |
| `DictionaryIndexConfig.java` | Medium | `DEFAULT` constant redefined;
`equals()` contract changed for callers |
| `DictionaryIndexType.java` | Medium | Behavioral change: `false` → `null`
for absent legacy flags; affects index creation path |
| `JsonIndexConfig.java` | Medium | Mutable setters; `equals()` now includes
`_indexPaths` and `_maxBytesSize` (previously excluded) |
| `VectorIndexConfig.java` | Medium | `properties` null→empty-map coercion;
`equals()` and `hashCode()` added |
**Findings:** No issues at this level beyond the blocker and concern noted
below.
### Center of gravity
- [x] 1-3 key files or decisions identified
**Files:**
1. `IndexConfig.java` — root of the change; the `@JsonValue toJsonObject()`
on the base controls the serialization of the entire hierarchy and the
`disabled=false → null` normalization.
2. `ForwardIndexConfig.java` — the most complex carve-out (cluster-tunable
trio + always-emit `encodingType`); gets the most test coverage.
3. `TextIndexConfig.java` — largest surface area; 18 fields converted;
builder copy-constructor semantics.
**Why these are central:** All other config changes are structural
replications of the same pattern. The base class decision cascades to all 12
classes. `ForwardIndexConfig` carries the most production risk because its
defaults are JVM-mutable singletons.
### Invariants — targeted
- [x] Authorization — N/A (config serialization, no auth boundaries)
- [x] Idempotency — safe: reads unchanged, writes narrower; round-trip is
idempotent (tested)
- [x] Backward compatibility — APIs: existing endpoints unchanged; request
deserialization unchanged (all `@JsonCreator` still accept fat JSON)
- [x] Backward compatibility — Schemas: ZK znodes are string-keyed JSON
objects; old fat znodes still deserialize; next write replaces with slim form —
compatible
- [x] Backward compatibility — SPI: `IndexConfig` hierarchy is an SPI.
**`equals()` contract change is a breaking SPI change** — see blocker below.
- [x] Backward compatibility — Configuration: No config key renames or
removals. The `disabled=false → null` normalization is a wire-format change
(key omitted) but deserialization is unchanged.
- [x] Ordering guarantees — N/A
- [x] Null/empty handling — Extensively tested; VectorIndexConfig
`properties` null→empty-map coercion is correct
- [x] Retry safety — N/A
- [x] Observability — N/A
- [x] Resource cleanup — N/A
- [x] Migration reversibility — Old fat znodes keep working; slim → fat
round-trip is fine; no migration needed
- [x] Performance in hot paths — `toJsonObject()` is called at serialization
time, not in hot query paths; acceptable
- [x] API design — Curated serializer pattern is appropriate; `@JsonValue`
is idiomatic
**Findings:**
**BLOCKER — `BloomFilterConfig.equals()` drops `isEnabled()` from the
contract without documentation or migration:**
The pre-PR `BloomFilterConfig.equals()` included `isEnabled() ==
that.isEnabled()`. The new implementation removes this check:
```java
// Before
return Double.compare(that._fpp, _fpp) == 0 && _maxSizeInBytes ==
that._maxSizeInBytes
&& _loadOnHeap == that._loadOnHeap && isEnabled() == that.isEnabled();
// After (proposed)
return Objects.equals(_fpp, that._fpp) && Objects.equals(_maxSizeInBytes,
that._maxSizeInBytes)
&& Objects.equals(_loadOnHeap, that._loadOnHeap);
```
The `super.equals()` call is still present, and `IndexConfig.equals()`
delegates to `Objects.equals(_disabled, that._disabled)`, so `disabled` is
still compared — but through the base class call. Wait: the proposed `equals()`
no longer calls `super.equals()`. Let me verify this in the diff.
Looking at the diff at line 3386-3396:
```java
public boolean equals(Object o) {
if (this == o) { return true; }
if (o == null || getClass() != o.getClass()) { return false; }
if (!super.equals(o)) { return false; } // <-- super.equals IS called
BloomFilterConfig that = (BloomFilterConfig) o;
return Objects.equals(_fpp, that._fpp) &&
Objects.equals(_maxSizeInBytes, that._maxSizeInBytes)
&& Objects.equals(_loadOnHeap, that._loadOnHeap);
```
`super.equals()` IS called, which checks `_disabled`. The removed `&&
isEnabled() == that.isEnabled()` was redundant — `isEnabled()` is just
`!isDisabled()`, and `_disabled` is already covered by `super.equals()`. This
removal is not a bug; it removes a redundant check. Downgrading from blocker.
**Concern — `BloomFilterConfig.equals()` — `isEnabled()` removal is a
semantic equality change for the `DEFAULT` constant:**
Pre-PR: `BloomFilterConfig.DEFAULT = new BloomFilterConfig(DEFAULT_FPP, 0,
false)` had `_disabled = false` and `isEnabled() = true`. Two configs with
identical `fpp`/`maxSizeInBytes`/`loadOnHeap` but different `disabled` were NOT
equal (the extra `isEnabled()` check in the old code was separate from the
`super.equals()` chain — and `super.equals()` in the old code used `_disabled
== that._disabled` as `boolean`). In the new code, `super.equals()` uses
`Objects.equals(_disabled, that._disabled)` where `_disabled` is `Boolean.TRUE`
or `null`. The old check `isEnabled() == that.isEnabled()` in the subclass was
therefore genuinely redundant with `super.equals()`. The only semantic
difference is that the old code used an explicit `_disabled == false` (boolean)
while the new code normalizes `false → null`, but both produce the same
`isEnabled()` answer. No behavior change here.
**Concern (medium) — `JsonIndexConfig.equals()` now includes `_indexPaths`
and `_maxBytesSize` which were previously excluded:**
The old `equals()` did not compare `_indexPaths` or `_maxBytesSize`. The new
code adds both. This is a correctness fix but it changes the equality contract
for existing code that might rely on `indexPaths`-differing configs comparing
equal. Severity: concern (medium confidence): any callers that compare
`JsonIndexConfig` instances and rely on `indexPaths` differences being
invisible will now see them as unequal.
Evidence:
`pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/JsonIndexConfig.java`,
diff at line 3711-3725.
**Concern (medium) — `TextIndexConfig.AbstractBuilder` copy-constructor
loses null-vs-explicit distinction:**
When constructing a `TextIndexConfigBuilder(existingConfig)`, the copy
constructor reads through `other.isXxx()` / `other.getLuceneXxx()` (the
default-applying getters) into primitive builder fields. Calling `.build()`
then applies the "equals default → emit null" logic, which means a config that
was originally loaded with `luceneUseCompoundFile=true` (today's default, so
stored as `Boolean.TRUE`) will be read as `true` by the getter, then compared
against `LUCENE_INDEX_DEFAULT_USE_COMPOUND_FILE` (`true`), and converted back
to `null`. The explicit value is lost.
This is documented in the PR Javadoc but it is a subtle semantic change: the
copy-constructor path erases the null-vs-explicit distinction. Any production
code that round-trips a `TextIndexConfig` through `new
TextIndexConfigBuilder(config).build()` will lose explicit-default values.
Given that `TextIndexConfigBuilder.withProperties()` is the primary path for
converting legacy `FieldConfig.properties` maps to `TextIndexConfig`, this is
the most common code path.
Evidence: `TextIndexConfig.java` `AbstractBuilder(TextIndexConfig other)`
constructor and `build()` method in the diff; `TextIndexConfigBuilder.java`
(unchanged, still uses the copy-constructor path).
**Observation — `ForwardIndexConfig.Builder` copy-constructor reads raw
wrapper field:**
```java
_deriveNumDocsPerChunk = other._deriveNumDocsPerChunk; // reads the Boolean
wrapper directly
```
This correctly preserves the null-vs-explicit distinction. Consistent with
the intent. Good.
**Observation — `StarTreeIndexConfig.maxLeafRecords` uses primitive `int`
with 0-as-absent:**
`StarTreeIndexConfig.toJsonObject()` omits `maxLeafRecords` when it is `0`.
This is a design choice that treats `0` as "not configured," which is
reasonable since `0` max leaf records is not a valid configuration. However, it
is not documented and differs from the wrapper-based null-as-absent pattern
used everywhere else. Low risk since `maxLeafRecords` is not cluster-tunable.
### Test adequacy
- [x] Tests prove intended behavior (not just that code runs)
- [x] Failure modes covered
- [x] Boundary conditions covered
- [x] Not over-coupled to implementation details
**Findings:** Test coverage is unusually strong. Each config has:
1. `testRoundTripAllNull` — empty input stays empty
2. `testRoundTripPartial` — partial input survives
3. `testRoundTripAllSet` — all fields survive
4. `testExplicitDefault*Preserved` — explicit-default-matching value still
emitted (load-bearing for the PR's semantic promise)
5. `testFreshObjectMapperUsesJsonValue` — ensures the `@JsonValue` hooks
into Jackson even without Pinot's `ObjectMapper`
6. `testJacksonSerializationMatchesToJsonObject` — ensures `toJsonObject()`
agrees with Jackson serialization
The `IndexConfigSlimRoundTripTest` cross-cutting test goes through the
`IndexType.getConfig()` resolution path, which is the actual production code
path.
One gap: the `TextIndexConfig` copy-constructor erasure concern (noted
above) has no test coverage.
### Collateral damage
- [x] No copy-paste duplication introduced — the pattern is uniform; any
duplication is structural and intentional
- [x] No partial renames — all test call sites updated from `false` to
`Boolean.FALSE`
- [x] No dead code left behind
- [x] Docs and config match code changes — inline Javadoc updated throughout
- [x] No one-off exceptions added to shared abstractions
**Findings:**
**Observation — `TextIndexConfigTest.testEmptyTextConfigRoundTrip` inverts
its assertions:**
The test previously asserted
`serialized.contains("\"luceneAnalyzerClassArgs\":[]")` to document that empty
arrays ARE emitted. The new test asserts
`assertFalse(serialized.contains("\"luceneAnalyzerClassArgs\":[]"))` to
document they are NOT emitted. This is semantically correct given the new
behavior. The comment says "The round-trip below still verifies semantic
equivalence" — and the subsequent deserialization + equality check does hold.
No issue.
**Observation — `VectorConfigTest.serde` changes
`"{\"disabled\":false,\"vectorDimension\":0,\"version\":0}"` to `"{}"` :**
The original test was documenting the fat form as the canonical serialized
form. The new test documents `{}` as canonical. This is correct but note that
any external consumer (monitoring tool, test, ZK snapshot comparison) that
expected the old fat form will now see `{}`. This is anticipated and acceptable
per the PR description.
### Path-triggered review
- [x] Touched paths listed
- [x] Failure mode inferred per path
- [x] Reviewed for inferred risks
- [x] Boundary areas checked first
**Touched paths and inferred risks:**
| Path | Inferred failure mode | What was checked | Findings |
|---|---|---|---|
| `pinot-spi/.../IndexConfig.java` | SPI break: downstream implementors that
extend `IndexConfig` and rely on the old serialized form | Checked that
`@JsonValue` on base works with subclass overrides; verified
`super.toJsonObject()` delegation chain | No issue; override chain is sound |
| `pinot-spi/.../BloomFilterConfig.java` | `equals()` contract break for
comparison-based caching/deduplication | Verified `super.equals()` is still
called; checked `isEnabled()` removal is redundant | The `isEnabled()`
redundancy removal is benign — covered in invariants section |
| `pinot-spi/.../JsonIndexConfig.java` | Missing fields in `equals()`
(pre-fix) causing false equality; config comparison used in segment loading
decisions | Verified `_indexPaths` and `_maxBytesSize` now in `equals()` |
Correctness fix; see concern in invariants |
| `pinot-segment-spi/.../TextIndexConfig.java` | Builder copy path loses
explicit-default distinction; deprecated ctor forwarding breaks binary compat |
Checked deprecated ctor chain; checked builder copy-constructor | See concern
above |
| `pinot-segment-spi/.../ForwardIndexConfig.java` | Cluster-tunable snapshot
semantics; `_deriveNumDocsPerChunk` wrapper field | Verified snapshot model in
tests; verified builder `_deriveNumDocsPerChunk` is read as raw wrapper |
Correct |
| `pinot-segment-local/.../DictionaryIndexType.java` | `false→null` for
absent flags causes config mismatch if any code path compares against `new
DictionaryIndexConfig(false, false)` | Checked production callers of
`DictionaryIndexConfig.DEFAULT` | `DEFAULT` is now `new
DictionaryIndexConfig((Boolean)null)`, not `new DictionaryIndexConfig(false)`.
Old code comparing against `DEFAULT` using `equals()` will now get a different
result for configs deserialized from fat JSON with `{"onHeap":false}` (those
are now `Boolean.FALSE` != `null`). |
**Findings:**
**Concern (medium confidence) — `DictionaryIndexConfig.equals()`
distinguishes null from false for `onHeap`/`useVarLengthDictionary`:**
Under the new contract, `new DictionaryIndexConfig((Boolean) null, null)`
(the new `DEFAULT`) is NOT equal to a config deserialized from
`{"onHeap":false,"useVarLengthDictionary":false}` (which stores
`Boolean.FALSE`). Any code that previously compared a deserialized config
against `DictionaryIndexConfig.DEFAULT` expecting equality will now fail if the
deserialized JSON contained explicit `false` values. This is intentional per
the PR's design (explicit-vs-absent distinction) but it is a behavioral change
in `equals()` semantics for callers.
Evidence: `DictionaryIndexConfig.java` in the diff, `equals()` method;
`DictionaryIndexConfig.DEFAULT` constant change.
The risk is mitigated by the fact that production code in
`BloomIndexType.java` uses `BloomFilterConfig.DISABLED` and
`BloomFilterConfig.DEFAULT` only for construction, not equality comparison.
Similar for `DictionaryIndexType`. Still, this is a subtle semantic change in a
widely-used SPI.
---
## Verdict
**Risk:** Medium
**Verdict:** approved
**Verdict confidence:** Medium
### Confidence reasoning
The implementation is technically sound and well-tested. The PR description
is exceptional in documenting design choices. The two "concerns" raised are
real but are (a) documented in the PR, and (b) only affect code that directly
relies on the `equals()` contract returning different values for
null-vs-explicit-false distinctions. Production paths checked
(`BloomIndexType`, `DictionaryIndexType`) use these constants only for
construction, not equality comparison. The `TextIndexConfig` builder
copy-constructor concern is real but the affected path
(`TextIndexConfigBuilder(existingConfig)`) is used primarily for creating
configs programmatically from properties maps, not for ZK round-trips of
existing configs.
Confidence is medium rather than high because:
1. The `JsonIndexConfig.equals()` fix (adding `_indexPaths` and
`_maxBytesSize`) could affect callers that depended on the old broken equality.
This is a codebase-wide impact that wasn't fully traced.
2. The PR does not include an integration test through the full controller →
ZK → server path.
3. The manual `connection__cell__timeouts` repro is marked as incomplete.
### Blockers
None.
### Concerns
1. **Concern — `JsonIndexConfig.equals()` now includes `_indexPaths` and
`_maxBytesSize`** (previously missing). This is a correctness fix but changes
equality semantics for any code that compared `JsonIndexConfig` instances and
relied on `indexPaths`/`maxBytesSize` differences being invisible. Severity:
concern. Confidence: medium. Evidence:
`pinot-spi/src/main/java/org/apache/pinot/spi/config/table/JsonIndexConfig.java`,
`equals()` method.
2. **Concern — `TextIndexConfig.AbstractBuilder` copy-constructor erases
null-vs-explicit distinction.** Any code that round-trips a `TextIndexConfig`
through `new TextIndexConfigBuilder(existingConfig).build()` will lose
explicit-default values. The PR documents this but provides no test coverage
for the copy-constructor path. In practice the main affected path is
`TextIndexConfigBuilder` in `pinot-segment-local`. Severity: concern.
Confidence: medium. Evidence: `TextIndexConfig.java`
`AbstractBuilder(TextIndexConfig other)` + `build()` method in diff;
`TextIndexConfigBuilder.java`.
3. **Concern — `DictionaryIndexConfig.DEFAULT` semantic change.** `DEFAULT`
is now `new DictionaryIndexConfig((Boolean) null, null, null, null)` (all-null)
vs. the old `new DictionaryIndexConfig(false)` (explicitly-not-disabled). Any
equality comparison against `DEFAULT` that was previously passing for configs
deserialized from fat `{"onHeap":false,...}` JSON will now fail. Severity:
concern. Confidence: low (production callers checked don't rely on equality
against DEFAULT).
### Observations
1. `StarTreeIndexConfig.maxLeafRecords` uses `0`-as-absent rather than the
null-as-absent pattern used everywhere else. This inconsistency is low-risk but
worth noting for future maintainers.
2. The `FstIndexConfig.toJsonObject()` override is purely for "symmetry and
test traceability" (as documented in the Javadoc) — it delegates entirely to
the superclass. This is fine.
3. The large number of `false → Boolean.FALSE` call-site changes in tests is
mechanical and correct. These were necessary because the old `(boolean, ...)`
constructor was deprecated and replaced with a `@Deprecated` binary-compat
bridge that routes to the new `(Boolean, ...)` `@JsonCreator`. The autoboxing
`Boolean.FALSE` is explicit to disambiguate the overloads at compile time.
4. The PR claims the `TextIndexConfigTest.testEmptyTextConfigRoundTrip`
assertion inversion is safe. The round-trip deserialization + equality check at
the end of that test does cover the semantic equivalence, so the inversion is
sound.
5. The manual `connection__cell__timeouts` repro is unchecked — the PR
author notes this as incomplete. This is the primary user-visible repro case.
It would be good to close this before merge but it is not a blocker given the
unit test coverage.
### Areas recommended for human focus
1. **`JsonIndexConfig.equals()` callers** — Search for production code that
compares `JsonIndexConfig` instances and depends on `indexPaths` or
`maxBytesSize` being invisible to equality. The change is correct but could
break existing assumptions.
2. **`TextIndexConfigBuilder(existingConfig)` call sites** — Audit whether
any production code relies on explicit-default-value preservation through the
copy-constructor path. The builder path erases null-vs-explicit; if any config
that was explicitly set to a default value needs to be preserved in the slim
form through a builder round-trip, that path will silently drop it.
3. **ZK migration** — The PR claims no migration is needed. This is true for
functional correctness but any monitoring or tooling that compares ZK znodes by
string equality (snapshot diffing, config auditing scripts) will see every
config change as a content modification on next write. Confirm this is
acceptable in the operational environment.
4. **Integration test gap** — The manual `connection__cell__timeouts` repro
through `inferIndexes` + ZK should be done before or shortly after merge.
### Status
- [x] All applicable Stage 1 boxes checked
- [x] All applicable Stage 2 boxes checked
- [x] Verdict committed
- [x] Verdict confidence assigned with reasoning
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
