Skip to content

Wrapper the input and output stream of HadoopExtendedFileSystem#17365

Merged
highker merged 1 commit intoprestodb:masterfrom
gjhkael:fileSystemCacheBug
Mar 4, 2022
Merged

Wrapper the input and output stream of HadoopExtendedFileSystem#17365
highker merged 1 commit intoprestodb:masterfrom
gjhkael:fileSystemCacheBug

Conversation

@gjhkael
Copy link
Copy Markdown
Contributor

@gjhkael gjhkael commented Feb 28, 2022

Now the 'open' 'create' and 'append' api of HadoopExtendedFileSystem do not use Wrapper class to holds the FileSystem's reference, it will lead to FileSystem that cached in PrestoFileSystemCache closed by FinalizerService even it actually using in somewhere.

issue-17356

@linux-foundation-easycla
Copy link
Copy Markdown

linux-foundation-easycla bot commented Feb 28, 2022

CLA Signed

The committers are authorized under a signed CLA.

  • ✅ guojh (574a10a58c10830b27763bbcfcfc0bfb24524916)

@rongrong
Copy link
Copy Markdown
Contributor

rongrong commented Mar 2, 2022

The test failures seem to be related. Should we just create a wrapper class for HadoopExtendedFileSystem instead? @jainxrohit

@rongrong rongrong requested a review from jainxrohit March 2, 2022 18:57
@jainxrohit
Copy link
Copy Markdown
Contributor

Hey I am wondering if we need two commits for this change?

@gjhkael
Copy link
Copy Markdown
Contributor Author

gjhkael commented Mar 3, 2022

I will rebase it

@gjhkael gjhkael force-pushed the fileSystemCacheBug branch from 555265b to 05054b5 Compare March 3, 2022 06:46
@gjhkael
Copy link
Copy Markdown
Contributor Author

gjhkael commented Mar 3, 2022

@jainxrohit Please review the code

@jainxrohit
Copy link
Copy Markdown
Contributor

Overall looks good to me. Can you please specify what kind of testing you did?
There is a typo in the commit and PR title "oupput" => "output"

I would also prefer to have it as following
"Wrapper the input and output stream of HadoopExtendedFileSystem"

@jainxrohit jainxrohit requested a review from highker March 4, 2022 04:14
@gjhkael gjhkael changed the title Wrapper the input and oupput stream from HadoopExtendedFileSystem Wrapper the input and output stream from HadoopExtendedFileSystem Mar 4, 2022
@gjhkael gjhkael changed the title Wrapper the input and output stream from HadoopExtendedFileSystem Wrapper the input and output stream of HadoopExtendedFileSystem Mar 4, 2022
@highker highker merged commit 195ce18 into prestodb:master Mar 4, 2022
@gjhkael gjhkael deleted the fileSystemCacheBug branch March 7, 2022 02:11
@varungajjala varungajjala mentioned this pull request Mar 22, 2022
9 tasks
@asjadsyed asjadsyed mentioned this pull request Mar 23, 2022
9 tasks
@asjadsyed asjadsyed mentioned this pull request Apr 1, 2022
8 tasks
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.

5 participants