fix: replace local storage aiofile usage to avoid linux caio read failures - #12433
fix: replace local storage aiofile usage to avoid linux caio read failures#12433manav2000 wants to merge 6 commits into
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThis pull request migrates async file I/O from Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 inconclusive)
✅ Passed checks (6 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Hey @ogabrielluiz @Cristhianzl could either of you please review this PR? |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lfx/src/lfx/_assets/component_index.json (1)
73434-118491:⚠️ Potential issue | 🟠 MajorInconsistency: PR description doesn't match the file changes shown.
The PR objectives state this PR "replaces usage of aiofile/caio with aiofiles" in the storage service to fix Linux read failures. However, the only file provided for review is
component_index.jsoncontaining dependency metadata updates.Key concerns:
- Missing files: Where are the actual storage service code changes (replacing
aiofile.async_openwithaiofiles.open)?- Missing tests: The PR summary mentions "adds comprehensive edge-case tests for storage operations" - where are these test files?
- Unrelated changes: Why is a dependency version downgrade in component metadata included in a PR about storage I/O implementation?
This suggests either:
- The review context is incomplete (missing the main code changes)
- The wrong files were committed to this PR
- The component_index.json changes should be in a separate PR
#!/bin/bash # Description: Find the actual storage service changes mentioned in PR description echo "=== Looking for storage service files with aiofile/aiofiles changes ===" git diff origin/main...HEAD --name-only | grep -E "(storage|aiofile)" echo "" echo "=== Searching for aiofile imports in the codebase ===" rg -l "from aiofile|import aiofile" --type=py echo "" echo "=== Searching for aiofiles imports (new implementation) ===" rg -l "from aiofiles|import aiofiles" --type=py echo "" echo "=== Looking for test files added/modified ===" git diff origin/main...HEAD --name-only | grep test🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lfx/src/lfx/_assets/component_index.json` around lines 73434 - 118491, The PR description claims changes for replacing aiofile/caio imports with aiofiles and adding storage service tests, but only shows component_index.json metadata updates. To fix this discrepancy, locate the actual storage service implementation files where aiofile usage should be replaced by aiofiles, and add the corresponding edge-case test files for storage operations. Ensure these crucial code and test changes are included in the PR and separate unrelated dependency version changes into different PRs. Focus on the files responsible for storage handling (e.g., storage service Python modules) and test files that validate storage functionality.
🧹 Nitpick comments (1)
src/backend/tests/unit/services/storage/test_local_storage_service.py (1)
398-419: Clever regression guard, but relies on internalaiofileimplementation detail.This test effectively validates that the storage service no longer uses
aiofile(since migrating toaiofilesmeans no contexts should be created inDEFAULT_CONTEXT_STORE). However, it depends onaiofile.aio.DEFAULT_CONTEXT_STORE, an internal detail that could change in futureaiofileversions.Consider adding a comment explaining the intent: this test ensures the migration to
aiofilespersists and guards against accidental reversion.📝 Suggested documentation improvement
`@pytest.mark.skipif`(sys.platform != "linux", reason="Linux-specific caio leak regression test") async def test_storage_operations_do_not_leak_caio_contexts(self, local_storage_service): - """Test storage operations do not leak caio contexts on Linux.""" + """Guard against regression to aiofile usage which can leak caio contexts on Linux. + + This test imports from aiofile's internal DEFAULT_CONTEXT_STORE to verify that + storage operations (now using aiofiles) do not create caio contexts. If someone + accidentally reverts to aiofile, this test will fail. + """ try: from aiofile.aio import DEFAULT_CONTEXT_STORE except ImportError:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/backend/tests/unit/services/storage/test_local_storage_service.py` around lines 398 - 419, The test test_storage_operations_do_not_leak_caio_contexts relies on the internal aiofile symbol DEFAULT_CONTEXT_STORE to guard against accidental re-introduction of aiofile usage; add a clear explanatory comment immediately above the test (referencing DEFAULT_CONTEXT_STORE and local_storage_service) stating that this is an intentional regression guard to ensure the project remains migrated to aiofiles and that the test intentionally inspects aiofile internals only to detect reversion, and keep the existing try/except ImportError behavior so the test is skipped when aiofile is not installed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/lfx/src/lfx/_assets/component_index.json`:
- Line 73434: The component_index.json contains invalid google package versions
(2.8.0 / 2.30.0) caused by a bug in the auto-generator; inspect
scripts/build_component_index.py and fix the version-detection routine (e.g.,
the function responsible for resolving dependency versions such as
get_dependency_version / detect_dependency_version / infer_version_from_dist) to
correctly read version metadata (prefer package metadata from importlib.metadata
or parsing pyproject/PKG-INFO) and handle missing/ambiguous values, validate
resolved versions against PyPI (or a local source list) and skip or flag unknown
versions, then re-run the generator to regenerate component_index.json (do not
manually edit the generated file) and update any component templates whose
dependency specifiers are malformed.
---
Outside diff comments:
In `@src/lfx/src/lfx/_assets/component_index.json`:
- Around line 73434-118491: The PR description claims changes for replacing
aiofile/caio imports with aiofiles and adding storage service tests, but only
shows component_index.json metadata updates. To fix this discrepancy, locate the
actual storage service implementation files where aiofile usage should be
replaced by aiofiles, and add the corresponding edge-case test files for storage
operations. Ensure these crucial code and test changes are included in the PR
and separate unrelated dependency version changes into different PRs. Focus on
the files responsible for storage handling (e.g., storage service Python
modules) and test files that validate storage functionality.
---
Nitpick comments:
In `@src/backend/tests/unit/services/storage/test_local_storage_service.py`:
- Around line 398-419: The test
test_storage_operations_do_not_leak_caio_contexts relies on the internal aiofile
symbol DEFAULT_CONTEXT_STORE to guard against accidental re-introduction of
aiofile usage; add a clear explanatory comment immediately above the test
(referencing DEFAULT_CONTEXT_STORE and local_storage_service) stating that this
is an intentional regression guard to ensure the project remains migrated to
aiofiles and that the test intentionally inspects aiofile internals only to
detect reversion, and keep the existing try/except ImportError behavior so the
test is skipped when aiofile is not installed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6d9fe3a6-92ad-4584-98cd-0493443e73ea
📒 Files selected for processing (3)
src/backend/base/langflow/services/storage/local.pysrc/backend/tests/unit/services/storage/test_local_storage_service.pysrc/lfx/src/lfx/_assets/component_index.json
| { | ||
| "name": "google", | ||
| "version": "2.30.0" | ||
| "version": "2.8.0" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check if google package version 2.8.0 exists and compare with version 2.30.0
# Check PyPI for google package versions
echo "=== Checking google package on PyPI ==="
curl -s https://pypi.org/pypi/google/json | jq -r '.releases | keys | .[]' | sort -V | tail -20
echo ""
echo "=== Checking if version 2.8.0 exists ==="
curl -s https://pypi.org/pypi/google/2.8.0/json | jq -r '.info.version // "NOT FOUND"'
echo ""
echo "=== Checking if version 2.30.0 exists ==="
curl -s https://pypi.org/pypi/google/2.30.0/json | jq -r '.info.version // "NOT FOUND"'
echo ""
echo "=== Checking latest version ==="
curl -s https://pypi.org/pypi/google/json | jq -r '.info.version'Repository: langflow-ai/langflow
Length of output: 277
🏁 Script executed:
#!/bin/bash
# Description: Check component templates for google dependency versions
echo "=== Searching for google dependency declarations in component files ==="
rg -n --type=py -A2 -B2 '"google"' --glob='**/components/**/*.py' | head -50
echo ""
echo "=== Checking if build script needs to be re-run ==="
git log -1 --format="%H %s" -- src/lfx/src/lfx/_assets/component_index.jsonRepository: langflow-ai/langflow
Length of output: 4236
Both version numbers (2.8.0 and 2.30.0) are invalid and do not exist on PyPI.
The google package versions in component_index.json do not correspond to any released versions on PyPI. PyPI records show that the google package has versions: 1.9.3, 2.0.1, 2.0.2, 2.0.3, and 3.0.0. Neither 2.8.0 nor 2.30.0 exist.
This file is auto-generated by scripts/build_component_index.py, which dynamically imports components and detects dependency versions. The presence of invalid version numbers across 9 separate entries suggests a systematic error in version detection or corruption in the build process.
Action required:
- Investigate
build_component_index.pyfor bugs in version detection logic - Verify that component template dependencies contain valid version specifications
- Regenerate
component_index.jsonwith corrected source data - Do not manually edit this auto-generated file
Affects lines: 73434, 73582, 73743, 73872, 74001, 74291, 74651, 113848, 114232 (and corresponding sha256 hash at line 118490)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lfx/src/lfx/_assets/component_index.json` at line 73434, The
component_index.json contains invalid google package versions (2.8.0 / 2.30.0)
caused by a bug in the auto-generator; inspect scripts/build_component_index.py
and fix the version-detection routine (e.g., the function responsible for
resolving dependency versions such as get_dependency_version /
detect_dependency_version / infer_version_from_dist) to correctly read version
metadata (prefer package metadata from importlib.metadata or parsing
pyproject/PKG-INFO) and handle missing/ambiguous values, validate resolved
versions against PyPI (or a local source list) and skip or flag unknown
versions, then re-run the generator to regenerate component_index.json (do not
manually edit the generated file) and update any component templates whose
dependency specifiers are malformed.
…ttps://github.com/manav2000/langflow into bug-12414/fix-resource-limitation-in-reading-files
… concurrent execution aiofile uses caio (kernel AIO) which creates contexts in a global dict that are never cleaned up. Under concurrent execution these accumulate until the OS aio-max-nr limit is exhausted, causing SystemError(11, 'Resource temporarily unavailable'). aiofiles uses thread pools instead and does not have this issue. Migrates all aiofile.async_open usages across both backend and lfx packages to aiofiles.open. Based on #12433 by @manav2000, extended to cover all remaining usages. Co-Authored-By: manav2000 <manav2000@users.noreply.github.com>
|
Thank you @manav2000 ! I've opened up http://github.com/langflow-ai/langflow/pull/12525 which attributes your fix and adds some more. I'll make sure it gets into the next release. Thanks so much |
|
Thank you @erichare, really appreciate it! Glad the fix was useful, and I’m happy to see it going into the next release. |
… concurrent execution (#12525) * fix: replace aiofile with aiofiles to prevent caio context leak under concurrent execution aiofile uses caio (kernel AIO) which creates contexts in a global dict that are never cleaned up. Under concurrent execution these accumulate until the OS aio-max-nr limit is exhausted, causing SystemError(11, 'Resource temporarily unavailable'). aiofiles uses thread pools instead and does not have this issue. Migrates all aiofile.async_open usages across both backend and lfx packages to aiofiles.open. Based on #12433 by @manav2000, extended to cover all remaining usages. Co-Authored-By: manav2000 <manav2000@users.noreply.github.com> * test: add concurrent write-then-read regression test for caio EAGAIN fix Exercises the exact failure pattern from #12414: multiple concurrent save-then-immediately-read operations on the storage service. This would previously trigger SystemError(11, EAGAIN) after ~150-200 runs with the aiofile/caio backend. Co-Authored-By: manav2000 <manav2000@users.noreply.github.com> * [autofix.ci] apply automated fixes * [autofix.ci] apply automated fixes (attempt 2/3) --------- Co-authored-by: manav2000 <manav2000@users.noreply.github.com> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
… concurrent execution (#12525) * fix: replace aiofile with aiofiles to prevent caio context leak under concurrent execution aiofile uses caio (kernel AIO) which creates contexts in a global dict that are never cleaned up. Under concurrent execution these accumulate until the OS aio-max-nr limit is exhausted, causing SystemError(11, 'Resource temporarily unavailable'). aiofiles uses thread pools instead and does not have this issue. Migrates all aiofile.async_open usages across both backend and lfx packages to aiofiles.open. Based on #12433 by @manav2000, extended to cover all remaining usages. Co-Authored-By: manav2000 <manav2000@users.noreply.github.com> * test: add concurrent write-then-read regression test for caio EAGAIN fix Exercises the exact failure pattern from #12414: multiple concurrent save-then-immediately-read operations on the storage service. This would previously trigger SystemError(11, EAGAIN) after ~150-200 runs with the aiofile/caio backend. Co-Authored-By: manav2000 <manav2000@users.noreply.github.com> * [autofix.ci] apply automated fixes * [autofix.ci] apply automated fixes (attempt 2/3) --------- Co-authored-by: manav2000 <manav2000@users.noreply.github.com> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Fixes: #12414
This PR fixes intermittent Linux local-storage read failures under concurrent Langflow execution by replacing the backend storage service’s aiofile/caio-based file access with aiofiles, avoiding the caio.AsyncioContext creation path shown in the reported SystemError: (11, 'Resource temporarily unavailable') traceback; it keeps the storage API unchanged and adds regression coverage for repeated immediate read-after-write, concurrent save/read behavior, and a Linux-specific guard against caio context leakage.
Summary by CodeRabbit
Tests
Chores