Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add HA functionality to WebHdfsClient #2230

Merged
merged 1 commit into from
Oct 1, 2017
Merged

Add HA functionality to WebHdfsClient #2230

merged 1 commit into from
Oct 1, 2017

Conversation

adaitche
Copy link
Contributor

Motivation and Description

HDFS supports High Availability (HA) through the use of multiple namenodes. This is currently not supported by WebHdfsClient as it allows to specify only one namenode. Since version 2.1.0, the hdfs package allows to specify multiple namenodes, by passing an url containing multiple namenodes separated by ';'. This functionality is forwarded through to WebHdfsClient in this commit.

Have you tested this? If so, how?

I tested the HA functionality manually on a cluster with two namenodes. I did not add unit tests, as for this one would need to extend WebHdfsMiniCluster to support multiple namenodes, which is non-trivial and probably overkill.

Since version 2.1.0, the hdfs package allows to specify multiple
namenodes (HA functionality), by passing an url containing multiple
namenodes separated by ';'. This functionality is forwarded through to
WebHdfsClient in this commit.
Copy link
Collaborator

@dlstadther dlstadther left a comment

Choose a reason for hiding this comment

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

Seems like an innocent change to me, but could you place git blame some other Webhdfs luigi contributors for review?

Thanks!

@adaitche
Copy link
Contributor Author

@Tarrasch As the main author of the webhdfs client, could you take a quick look at this patch?

@Tarrasch Tarrasch merged commit f6206ee into spotify:master Oct 1, 2017
@Tarrasch
Copy link
Contributor

Tarrasch commented Oct 1, 2017

Thanks @adaitche :)

This was referenced Jun 29, 2022
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.

3 participants