Skip to content

fix(loader): prevent concurrent file read failures - #858

Merged
imbajin merged 3 commits into
masterfrom
codex/fix-loader-skipped-line-concurrency
Oct 27, 2026
Merged

imbajin merged 3 commits into
masterfrom
codex/fix-loader-skipped-line-concurrency

Conversation

@imbajin

@imbajin imbajin commented Oct 27, 2026 •

Copy link
Copy Markdown
Member

Purpose of the PR

Fix #706. Takes over #710 by @bajisn-666 and restores the affected four-thread tests.

HDFS directory scans schedule matching direct files twice. With parallel parser threads, the first task removes the shared file checkpoint while the second still reads, causing FileReader to dereference a missing progress entry. Split readers also share a mutable line-filter Matcher, which can let comments and blank lines through to CSV parsing. Each file is now scheduled once, and each match has independent state.

Main Changes

  • Recurse only into child directories during HDFS scans, preserving file and directory filters.
  • Cache the immutable compiled Pattern and create a Matcher for each line; invalidate the cache when the regex changes.
  • Add regressions for duplicate HDFS scans, nested paths, filtering, and concurrent row/progress integrity. Restore four parser threads in the three previously restricted cases and the two HDFS cases from fix(loader): avoid NPE in FileLineFetcher close method & update HDFS test #710.

Verifying these changes

  • Need tests and can be verified as follows:
    • Both regressions fail with the previous implementations and pass with the fixes: duplicate HDFS readers and incorrect concurrent skipped-line matching.
    • Client and Loader unit suites pass on JDK 17, including source configuration roundtrips and exact row/progress assertions.
    • With the locked candidate Server and Hadoop 3.3.6, the affected multi-file header, empty-file, suffix-filter, and HDFS source cases pass with four parser threads and their original vertex-count assertions.
    • Module validation passes on the final branch: full Loader file/HDFS/JDBC/Kafka profiles, Spark Loader, Flink CDC recovery, Hubble API/UI smoke, license checks, and patch/project coverage.

Does this PR potentially affect the following parts?

  • Nope
  • Dependencies (add/update license info)
  • Modify configurations
  • The public API
  • Other affects

Documentation Status

  • Doc - TODO
  • Doc - Done
  • Doc - No Need

Visual overview

HDFS file scheduling and independent regex Matcher state

HDFS scans schedule direct files once and recurse only into child directories. Concurrent readers share an immutable compiled Pattern but create independent Matcher state for each line. Fixed-configuration concurrency is illustrated; hot regex mutation, bounded allocation performance and full compatibility/RC acceptance are not established by this diagram.

- share compiled patterns with per-call matchers
- cover concurrent filtering and source serialization
- restore four-thread local and HDFS regressions
- restore FileSource through registered InputSource adapter
- retain custom filtering after JSON roundtrip
- validate the final Client and Loader unit suites
- recurse only into child directories during HDFS scans
- keep one reader task for each direct input file
- cover nested paths and filters with Hadoop local files
@imbajin imbajin changed the title fix(loader): isolate skipped-line matchers fix(loader): fix parallel file loading Oct 27, 2026
@imbajin imbajin changed the title fix(loader): fix parallel file loading fix(loader): prevent concurrent file read failures Oct 27, 2026
@codecov

codecov Bot commented Oct 27, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 55.10%. Comparing base (b34007e) to head (238da44).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master     #858      +/-   ##
============================================
- Coverage     55.40%   55.10%   -0.31%     
+ Complexity     5989     4704    -1285     
============================================
  Files           619      393     -226     
  Lines         32582    25575    -7007     
  Branches       4217     3750     -467     
============================================
- Hits          18052    14093    -3959     
+ Misses        12952    10115    -2837     
+ Partials       1578     1367     -211     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@bitflicker64 bitflicker64 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking: no. Score: 9/10. Summary: scanReadables now schedules each direct HDFS file once and SkippedLine creates a Matcher per call, which removes the duplicate-reader progress NPE and the shared-matcher race; the new unit tests fail on the base sources and pass at this head.

@imbajin
imbajin merged commit e8476e7 into master Oct 27, 2026
20 checks passed
@imbajin
imbajin deleted the codex/fix-loader-skipped-line-concurrency branch October 27, 2026 15:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Unit tests fail with NPE in HDFS-Tests (loader)

3 participants