persist: preparation for inline writes #26429
Conversation
|
MitigationsCompleting required mitigations increases Resilience Coverage.
Risk Summary:The pull request poses a high risk with a score of 83, mainly driven by the average line count and the number of executable lines within the files modified. Historically, pull requests with these characteristics are 126% more likely to introduce bugs compared to the baseline for this repository. Although the observed bug trend in the repository is increasing, the predicted trend is decreasing, indicating an expected improvement in the near future. Note: The risk score is not based on semantic analysis but on historical predictors of bug occurrence in the repository. The attributes above were deemed the strongest predictors based on that history. Predictors and the score may change as the PR evolves in code, time, and review activity. |
danhhz
force-pushed
the
persist_inline_prep
branch
from
1a1930d to
888e05b
Compare
We've gotten somewhat lucky so far to be able to more-or-less pick a default set of configuration values for persist unit tests, perhaps adding a couple new tests for the non-default cases. But inline writes probably wants to keep testing coverage of both the enabled and disabled path in perpetuity. So, introduce a new `mz_persist_proc::test` prod macro that runs an entire cargo test under an arbitrary set of configurations and hook it up to persist's tests. Intentionally don't add a lint requiring its usage because many tests don't use configuration and so running them multiple times would be pure waste. Particularly egregious would be the various randomized prop tests (e.g. for encoding).
We would have otherwise had to add a third level of Result in inline writes.
We'll need it in inline writes to implement the method that flushes a batch out to s3.
Used by the upcoming inline writes PR to disable it for the two datadriven files that don't make sense with inline writes (testing blob things).
Separating out some of the mechanical bits from the inline writes PR. It works essentially by adding a second `Inline` variant to this enum.
danhhz
force-pushed
the
persist_inline_prep
branch
from
25f03c8 to
2e3f4b1
Compare
|
TFTR! Merging once this nightlies run confirms that there indeed is no behavior change (as intended) |
There are a number of mechanical bits in #19383 that can easily be separated out. Do so in a separate PR to keep that one focused on the interesting diffs (as much as possible).
See individual commits for details. This PR is not terribly motivated on its own, definitely look at the other one for the context of why these things are happening.
Motivation
Tips for reviewer
Intended to be no behavior change.
Checklist
$T ⇔ Proto$Tmapping (possibly in a backwards-incompatible way), then it is tagged with aT-protolabel.