Skip to content

[ntuple] Enable disconnecting processors from storage - #23625

Merged
enirolf merged 2 commits into
root-project:masterfrom
enirolf:ntuple-proc-chain-open-files
Oct 7, 2026
Merged

enirolf merged 2 commits into
root-project:masterfrom
enirolf:ntuple-proc-chain-open-files

Conversation

@enirolf

@enirolf enirolf commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

To avoid keeping files open, especially in chains.

Hopefully fixes the failing Windows builds in #23477, too.

@enirolf enirolf self-assigned this Oct 6, 2026

@vepadulano vepadulano left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, one remark and a clarifying question from my side.

Comment thread tree/ntuple/inc/ROOT/RNTupleProcessor.hxx Outdated
Comment thread tree/ntuple/src/RNTupleProcessor.cxx Outdated
@enirolf
enirolf force-pushed the ntuple-proc-chain-open-files branch from 77e9134 to adee311 Compare October 6, 2026 11:48

@jblomer jblomer 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.

In principle LGTM. Perhaps could come with a unit tests that create, say, 2000 1-entry files and iterates them. Without this optimization, we should hit a 1024 file descriptor limit at least on some Linux platforms.

Comment thread tree/ntuple/src/RNTupleProcessor.cxx Outdated
@enirolf
enirolf force-pushed the ntuple-proc-chain-open-files branch from adee311 to 8112e65 Compare October 6, 2026 13:02
@enirolf
enirolf requested a review from jblomer October 6, 2026 13:03

@vepadulano vepadulano left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks!

@github-actions

github-actions Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

Test Results

    24 files      24 suites   3d 21h 48m 49s ⏱️
 3 883 tests  3 880 ✅ 0 💤 3 ❌
83 112 runs  83 109 ✅ 0 💤 3 ❌

For more details on these failures, see this check.

Results for commit 8112e65.

♻️ This comment has been updated with latest results.

@enirolf
enirolf merged commit 2d9fc33 into root-project:master Oct 7, 2026
56 of 63 checks passed
@enirolf
enirolf deleted the ntuple-proc-chain-open-files branch October 7, 2026 14:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants