Skip to content

[Ambari 23549] Blueprint configuration processor updates for HDFS NameNode Federation#985

Merged
rnettleton merged 11 commits into
apache:trunkfrom
rnettleton:AMBARI-23549-trunk
Apr 16, 2018
Merged

[Ambari 23549] Blueprint configuration processor updates for HDFS NameNode Federation#985
rnettleton merged 11 commits into
apache:trunkfrom
rnettleton:AMBARI-23549-trunk

Conversation

@rnettleton
Copy link
Copy Markdown

What changes were proposed in this pull request?

The Blueprint configuration processor was updated to account for HDFS NameNode Federation deployments. The following changes were introduced:

  1. The "hadoop-env/dfs_ha_initial_cluster_id" property will now be set automatically by the Blueprint configuration processor to be the cluster's name, unless this setting was already customized by the user.

  2. In addition to setting the cluster ID, this change should also implement some minor configuration conveniences that will make Blueprint creation for Federated clusters simpler:

    • Add property updater support for "dfs.namenode.shared.edits.dir.NAMESERVICE" properties.
    • Add property update support for "dfs.namenode.servicerpc-address.NAMESERVICE.NAMENODE" properties
  3. Unit tests were updated to validate this change.

How was this patch tested?

  1. Ran the ambari-server unit test suite (passed).

  2. Deployed a 4-node HDFS NameNode Federation cluster using a Blueprint, and verified that the HDFS services are all started as expected, and that the hdfs-site configuration includes the expected configuration values.

Please review Ambari Contributing Guide before opening a pull request.

@rnettleton rnettleton self-assigned this Apr 12, 2018
clusterConfig.setProperty(HADOOP_ENV_CONFIG_TYPE_NAME, HDFS_HA_INITIAL_CLUSTER_ID_PROPERTY_NAME, clusterName);
} else {
LOG.warn("Cluster name could not obtained, this may indicate a deployment or configuration error.");
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Shouldn't missing cluster name mean an exception or are there valid cases when cluster name could be missing?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

That's a good question. I've checked the usages of the getClusterName() method throughout the server code, and some cases treat this as an exception case, and some just proceed assuming that the name is always non-null.

I don't think there are any valid cases when the cluster name would be missing, so I'll update this code to throw an exception in the event that a null cluster name is found.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Updated the patch to include more error-checking in this area, and added a unit test to verify that the expected exception is thrown in the event that getClusterName() returns null.

@asfgit
Copy link
Copy Markdown

asfgit commented Apr 12, 2018

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/Ambari-Github-PullRequest-Builder/1818/
Test FAILed.
Test FAILured.

@adoroszlai
Copy link
Copy Markdown
Contributor

@rnettleton When merging this, please make sure to select "Squash and merge" from the dropdown.

https://help.github.com/articles/about-pull-request-merges/#squash-and-merge-your-pull-request-commits

Copy link
Copy Markdown
Contributor

@benyoka benyoka left a comment

Choose a reason for hiding this comment

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

LGTM, one minor question

@asfgit
Copy link
Copy Markdown

asfgit commented Apr 13, 2018

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/Ambari-Github-PullRequest-Builder/1850/
Test PASSed.

@rnettleton rnettleton merged commit 4f0fbd2 into apache:trunk Apr 16, 2018
@rnettleton rnettleton deleted the AMBARI-23549-trunk branch April 16, 2018 14:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants