Create pull request#26
Merged
Merged
Conversation
When pod5_files < cpu_count, POD5 records are redistributed across all workers. Multiple workers then access the same Pod5FileReader_t* concurrently, causing a data race crash (the POD5 C API is not thread-safe for same-reader access). Fix: each worker lazily opens its own Pod5FileReader_t* per POD5 file on first access, stored in a local map. Readers are closed when the worker completes. - Add file_path field to Pod5RecordMeta - Lazy-open per-worker readers in MergedDataWorker (streaming) - Lazy-open per-worker readers in process_merged_data_meta_worker (consistency mode) - Close per-worker readers on completion - Skip records when pod5_open_file fails (avoid shared reader fallback which would re-introduce the data race) - Add error logging for pod5_open_file failures in both paths Tested on node05 with pod5_8 dataset (8 files, 256 workers): - Before fix: crash during batch 1 processing (same as AWS) - After fix: completed in ~30 min, 33,891 NPZ files output Signed-off-by: donghyuk <donghyuk@genome4me.com>
- Back-pressure with configurable max_queue_size (default: process_once * 4 * workers, -Q CLI option) - Half-watermark notify: wake read_loop only when queue drops below 50% - Convert-later: defer bam1_t to BamRecord conversion until batch processing, skip convert for unmatched reads in order to avoid memory consumption - NpzWriter: offset/count based save_chunk, flat array reuse, rvalue add_records - RecordMerger: reuse single instance across batches - Replace queue with deque in BAM read/dispatch for efficient range insert/erase - Elapsed time logging (HH:MM:SS.ss format) - Memory monitoring via get_rss_mb() pod5_81 benchmark (node07 SSD, 256 workers, n=1000): Before: 266 min, 1675 GB peak RSS After: 91 min, 1119 GB peak RSS (2.9x faster, 33% less memory) Signed-off-by: donghyuk <donghyuk@genome4me.com>
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
2 participants
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.
[preprocess] Fix concurrent Pod5FileReader access crash
[preprocess] Optimize dispatch and convert-later pattern