Skip to content

[common] Reuse the file system built by the FileIO access check - #9471

Closed
wombatu-kun wants to merge 1 commit into
apache:masterfrom
wombatu-kun:issue/8548-fileio-reuse-access-check-instance
Closed

[common] Reuse the file system built by the FileIO access check#9471
wombatu-kun wants to merge 1 commit into
apache:masterfrom
wombatu-kun:issue/8548-fileio-reuse-access-check-instance

Conversation

@wombatu-kun

Copy link
Copy Markdown
Contributor

Purpose

Split out of #8962 at @JingsongLi's request. Answers this review comment.

FileIO.get probes a loader by calling load(path), configuring the result and calling exists, then discards it and calls load again for the instance it returns. Every FileIO.get therefore builds the file system twice for the same path, and with fs.<scheme>.impl.disable.cache=true the first one is unreachable afterwards.

checkAccess now returns the instance it checked and get hands that one back, releasing it if the candidate is rejected or the selection throws. That also covers the reviewer's point that FileIOLoader.load is @Public and may return a cached instance, so closing the probe and reloading could hand back a closed one.

Its return type changes from FileIOLoader to FileIO, a @Public signature change with no other caller in the repo.

Tests

FileIOTest: the checked instance is handed back rather than reloaded and configured twice, and it is released when its loader is dropped for missing options or when requiredOptions() throws.

@JingsongLi

Copy link
Copy Markdown
Contributor

FileIOLoader.load does not guarantee the return of a fresh or exclusive instance; the new tests also explicitly treat the return of a singleton as valid.

The solution is simple, never let fs..impl.disable.cache=true, I feel the changes in this PR are pointless, and I don't want to waste any more time on it.

@JingsongLi JingsongLi closed this Aug 30, 2026
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.

2 participants