[config] Move bloom_false_positive_rate from dev_tweaks to storage - #6544
[config] Move bloom_false_positive_rate from dev_tweaks to storage#6544mihaibudiu wants to merge 1 commit into
Conversation
|
Do we know if this option is safe to modify on a pipeline without resetting state? |
|
I don't know if it's safe, but the PR doesn't change this property. |
mythical-fred
left a comment
There was a problem hiding this comment.
LGTM. Clean migration:
- New
storage.bloom_false_positive_ratefield with full doc and the f64 round-trip regression test that mirrors the pattern indev_tweaks. Good defensive testing — thatserde(deserialize_with = "crate::serde_via_value::deserialize")trick is exactly the bit that has bitten f64 fields through#[serde(flatten)]before. Runtime::bloom_false_positive_rate()prefers storage, falls back to dev_tweaks. The dev_tweaks fallback still returns 0.0001 when unset, so behaviour for pipelines on neither field is unchanged.dev_tweaksfield kept and documented as deprecated — that's the right deprecation shape (don't break configs in-flight).impl Eq for StorageOptionsis fine: serde_json cannot produce NaN for f64, and nothing in the codebase writes NaN explicitly here.
Docs (memory.md) and openapi.json regenerated. No issues.
Yes. |
|
@mihaibudiu , have you tested that this doesn't break existing pipelines? |
|
Isn't this what CI is supposed to test? |
these config changes have historically been messy and we dont test it enough in CI. |
|
Is there a test which exercises the upgrade I should be running? |
Signed-off-by: Mihai Budiu <mbudiu@feldera.com>
mythical-fred
left a comment
There was a problem hiding this comment.
Rebased over ~436 commits of main; PR content (the six touched files) is identical to the tip I approved on 2026-06-26. Re-approving so the button lights up on the current base.
Fixes #5424
The old dev_tweaks option is still present, but it is marked as deprecated. If the storage option is present it is used first, and the dev_tweaks option is used otherwise.
The option is also documented publicly.