GH-37626: [Java][Cpp] support using existing java file system #37690
Conversation
|
|
github-actions
bot
added
Component: Java
Component: C++
awaiting review
|
I think what we'd want is to add a FileSystem parameter to FileSystemDatasetFactory::Make or FileSystemFactoryOptions, instead of a global. |
hmm, just the naming, it is a field in FileSystemFactoryOptions and then passed down not a global variable. |
|
Ah, sorry. In that case it should be named something less Java-specific and should probably be typed |
|
|
||
| /// when java context have a file system reference to be used | ||
| // then use this file system reference in the context | ||
| std::shared_ptr<void> file_system_java = nullptr; |
There was a problem hiding this comment.
I don't think this needs to be Java-specific at all.
This should be shared_ptr<FileSystem>.
| std::string internal_path; | ||
| ARROW_ASSIGN_OR_RAISE(std::shared_ptr<fs::FileSystem> filesystem, | ||
| fs::FileSystemFromUri(uri, &internal_path)) | ||
| fs::FileSystemFromUriAndFs(uri, &internal_path, options.file_system_java)) |
There was a problem hiding this comment.
I think what you would do is, if a filesystem is provided, call FileSystem::PathFromUri to set internal_path. If it succeeds, then proceed, else raise an error.
github-actions
bot
added
awaiting changes
westonpace
left a comment
westonpace
left a comment
There was a problem hiding this comment.
The change (allowing a filesystem to be specified when running discovery) seems good. I agree with @lidavidm that this shouldn't be java-specific.
|
|
|
Thank you for your contribution. Unfortunately, this pull request has been marked as stale because it has had no activity in the past 365 days. Please remove the stale label or comment below, or this PR will be closed in 14 days. Feel free to re-open this if it has been closed in error. If you do not have repository permissions to reopen the PR, please tag a maintainer. |
github-actions
bot
added
the
Status: stale-warning
|
Closing this now that the Java implementation has moved to: https://github.com/apache/arrow-java |
4 participants
Rationale for this change
when using the dataset api to repeatedly read chunks of file, FileSystem instances are created every time. causing performance degrade.
What changes are included in this PR?
to reuse existing filesystem passed from java context.
Are these changes tested?
tested on my local integration. as there seems no end to end dataset testing in the base
Are there any user-facing changes?
NO