Skip to content

use reflection to call hadoop fs.rename to workaround different hadoop jar version in main and hdfs-storage extension class loader#5296

Merged
gianm merged 2 commits intoapache:masterfrom
himanshug:update_3787
Jan 29, 2018
Merged

Conversation

@himanshug
Copy link
Copy Markdown
Contributor

@himanshug himanshug commented Jan 24, 2018

Fixes #3786 with updated version of #3787

Verified with hadoop-2.7.1 on a dev environment.

…p jar version in main and hdfs-storage extension class loader
@b-slim
Copy link
Copy Markdown
Contributor

b-slim commented Jan 24, 2018

👍 after tests.

@himanshug himanshug changed the title use reflection to call hadoop fs.rename to workaround different hadoop jar version in main and hdfs-storage extension class loader [WIP]use reflection to call hadoop fs.rename to workaround different hadoop jar version in main and hdfs-storage extension class loader Jan 25, 2018
@himanshug
Copy link
Copy Markdown
Contributor Author

marking it WIP as it needs more testing and looks very hacky.

@himanshug
Copy link
Copy Markdown
Contributor Author

himanshug commented Jan 25, 2018

@hamlet-lee in my testing, straight forward fs.getClass().getMethod(..) does not appear to work because rename method is not PUBLIC. This is noted in the javadocs too
. So, I had to add a recursive impl to go over all superclasses to find the rename method. See findRenameMethodRecursively(..) added in HadoopFsWrapper.java .

I wonder how it is working for you without this recursive search. Have you done anything else special in your setup?

@himanshug
Copy link
Copy Markdown
Contributor Author

I tested current patch with our hadoop cluster, seems to be working fine.

@himanshug himanshug changed the title [WIP]use reflection to call hadoop fs.rename to workaround different hadoop jar version in main and hdfs-storage extension class loader use reflection to call hadoop fs.rename to workaround different hadoop jar version in main and hdfs-storage extension class loader Jan 26, 2018
@hamlet-lee
Copy link
Copy Markdown
Contributor

@himanshug rename is PUBLIC in DistributedFileSystem.

public void rename(Path src, Path dst, final Rename... options) throws IOException {

@himanshug
Copy link
Copy Markdown
Contributor Author

himanshug commented Jan 28, 2018

@hamlet-lee i see, it is increasing visibility in the override .. however, one of of my tests tested it for LocalFileSystem where it is not overridden to be made public. and in FileSystem it is declared to be protected.
So unfortunately, that assumption about it being public can't be made in general and recursion is still necessary.

thanks for pointing that out, now I understand why it worked for you without recursion.

Copy link
Copy Markdown
Contributor

@gianm gianm left a comment

Choose a reason for hiding this comment

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

@himanshug - I'm sad it's needed but thanks for writing it.

@gianm gianm merged commit 632e44c into apache:master Jan 29, 2018
jon-wei pushed a commit to implydata/druid-public that referenced this pull request Oct 9, 2018
…p jar version in main and hdfs-storage extension class loader (apache#5296)

* use reflection to call hadoop fs.rename to workaround different hadoop jar version in main and hdfs-storage extension class loader

* find rename method recursively
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.

4 participants