Repository navigation
[Engine] avoid hanging on PleReader initialization failures - #3385
Draft
j-luwierski wants to merge 1 commit into
Draft
j-luwierski wants to merge 1 commit into
j-luwierski wants to merge 1 commit into
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.
Problem and result
On current upstream main,
PleReader::open()still hangs during cleanup if read-buffer allocation fails or if a reader is reopened with a table range beyond EOF. The failedopen()never returns its existing error. After this change, it closes cleanly and a later valid open/read succeeds.Related discussion
Retested the report in #3045, which was closed during the 0.2.0-beta tracker reset. The original PR #3046 was also closed in that reset.
Approach
Publish the in-flight slot count only after the buffers and free-slot list are ready, and reset the count when closing. Add Linux CPU-only lifecycle coverage for normal reads, failed reopen, and injected allocation failure in caller-thread and I/O-worker modes. The allocation failure uses link-time
posix_memalignwrapping and does not add production fault-injection behavior.Validation
Linux x86_64, GCC 14.2.0, CUDA 13.4.92, NVIDIA RTX 4070 Ti SUPER; base revision
e88ee39d1d6be6e3364183af3b39b62a7ed6f211, candidate revision9f4b65fc.Passed:
cmake -S . -B build/ple-reader-beta -DSTRATA_BUILD_TESTS=ON -DSTRATA_WERROR=ON;cmake --build build/ple-reader-beta --target ple_reader_lifecycle_test direct_file_async_test -j2; andctest --test-dir build/ple-reader-beta --output-on-failure -R '^(ple_reader_(reads|reopen|alloc)_(caller|threaded)|direct_file_async_test)$': targets built and all 7 tests passed.cmake -S . -B build/ple-reader-cuda -DSTRATA_BUILD_TESTS=ON -DSTRATA_WERROR=OFF -DSTRATA_ENABLE_CUDA=ON -DCMAKE_CUDA_COMPILER=/usr/local/cuda-13.4/bin/nvcc;cmake --build build/ple-reader-cuda --target strata ple_reader_lifecycle_test direct_file_async_test -j2: all targets built.ctest --test-dir build/ple-reader-cuda --output-on-failure -R '^(ple_reader_(reads|reopen|alloc)_(caller|threaded)|direct_file_async_test)$': all 7 tests passed.e88ee39d: both read controls passed;reopenand injectedalloccases timed out after 2 seconds in caller and threaded modes (4 hangs).git diff --check origin/main...HEAD: passed.Failed:
STRATA_WERROR=ON, the CUDA build commandcmake --build build/ple-reader-cuda --target strata ple_reader_lifecycle_test direct_file_async_test -j2stopped on-Werror=misleading-indentationin unchangedsrc/core/layer.cpp. The CUDA build passed with WERROR disabled; warnings are outside this change.Not run:
hipcc,dpcppandicpxare unavailable.Limits
The reproduction uses a synthetic file and real DirectFile I/O on Linux; it verifies initialization and recovery behavior, not a production memory-pressure incident or model inference.