Repository navigation
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
A flush detaches disk writers before finishing them. A late batch for the same schema/minute could reopen and truncate the detached writer's
.arrows.part, destroying acknowledged data. Startup recovery could also publish a live part, convert another conversion's sources, or delete its temporary Parquet output.Use exclusive part-file creation with a ULID fallback, coordinate Stream-managed flush/recovery through a lifecycle lock, and track processing directories and temporary outputs with per-Stream RAII claims. Startup plans retain their claims while pending; conversions retain them through publication/cleanup and schema staging. Failed or dropped work releases only its own claims. Recovery publishes legacy parts under the writer lock to prevent an in-process final-name clobber. Conversion parallelism and the two-group periodic bound are preserved.
Keep
pub writer: Mutex<Writer>and the exact publicarrow_files_grouped_exclude_timesignature, including ordinary non-test builds, as restored in R5. Document that direct writer operations do not acquire Stream lifecycle coordination and the grouping method mutates staging but returns an unowned snapshot. Add 12 deterministic lifecycle regressions and an external-consumer API/auto-trait test.Scope and provenance
Based on
origin/mainat9747e2ba7a6dc18e2965b24fc84dd0632dadd87e; onlystreams.rs,staging/writer.rs, andtests/staging_api_compat.rschange. This is the lifecycle correctness fix, with final R5 API restoration, isolated from the historical 941-file experimental overlay.Main's
streams.rsis byte-identical to the frozen experimental baseline. The resulting file and external API test exactly match R5; the disk-writer implementation also matches R5. No additional private overlay dependency needs porting. The experimental writer baseline's separate memory staging/chunking changes and their tests are excluded: main's entire memory writer implementation, including publicMutableBuffer.inner, remains unchanged. Existing per-batch IPC flushing is unchanged. Consequently the full branch is not the source tree used by the historical runs below.Validation
Fresh checks on this branch:
cargo +1.96.0 fmt --all --check: passed.rustfmt +1.96.0 --check --edition 2024 src/parseable/streams.rs src/parseable/staging/writer.rs tests/staging_api_compat.rs: passed.git diff --check: passed.cargo +1.96.0 metadata --offline --locked --no-deps --format-version 1: passed; discovers the external API test target.Fresh compilation and Rust test execution are deferred to CI. No Cargo build, server, native workload, benchmark, or historical-target mutation was performed for this port. Keep this PR in draft until fresh CI compilation and tests pass. Suggested targeted execution:
RAYON_NUM_THREADS=4 cargo test --locked --features kafka --lib parseable::streams::tests:: -- --test-threads=4, thencargo test --locked --features kafka --test staging_api_compatand the full library suite. These commands have not been run on this branch.Historical evidence from the experimental overlay, not fresh current-branch results (2026-10-17, retained under
staging-lifecycle/):r3/REPORT-r3.md: five shared ownership/publication regressions fail on R2 control and pass on R3; 807 default-feature library tests pass, one ignored.r5/REPORT-r5.md: restored external APIs compile; Kafka library suite has 834 passed, one ignored; Kafka Clippy passes with warnings.r6-native/REPORT-r6.md: four prospective repaired server lifetimes, each 18 million Kafka records, pass exact readback and graceful shutdown. A baseline arm committed 18 million offsets but had 10.42 million physical rows (7.58 million deficit). That deficit invalidated A/A comparison; no complete A/B block or relative performance conclusion exists. It is not proof that every missing row had one uniquely established cause.Limitations
Claims are in-process and per Stream object. Cross-process/shared-path mutation, deletion/recreation, and direct lifecycle manipulation through the historical public writer are not coordinated. Crash/SIGKILL after final Parquet rename but before source cleanup, or failed source removal, can still duplicate rows on retry. There is no new fsync, power-loss, durability, exactly-once, or universal loss-free guarantee. Invalid-source deletion and logged-only Drop errors remain. Metric workers can continue read-only work after an early error. Regression schedules use Unix inode checks and need sufficient Rayon workers; the historical runs used four. No throughput, CPU/RAM nonregression, latency, or 20% improvement claim is made; performance qualification remains incomplete.
This PR has:
Need help on this PR? Tag
@codesmith-botwith what you need.