Skip to content

Conversation

@rmetzger
Copy link
Contributor

No description provided.

Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder about the naming here. Maybe this should be YARN_RESOURCE_MANAGER_ENV_PREFIX? Or change YARN_APPLICATION_MASTER_ENV_PREFIX to YARN_JOB_MANAGER_ENV_PREFIX?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think YARN_RESOURCE_MANAGER_ENV_PREFIX is wrong bc its a service of YARN, not of Flink (http://hortonworks.com/blog/apache-hadoop-yarn-resourcemanager/).

Whether to use AM or JM is a good question. In the YARN context, AM and JM are basically the same. I think AM is a bit more consistent.

@mxm
Copy link
Contributor

mxm commented Nov 26, 2015

Looks good except for some minor issues.

@rmetzger
Copy link
Contributor Author

Thank you for the good review. I addressed some of your concerns.

@rmetzger
Copy link
Contributor Author

rmetzger commented Dec 5, 2015

Thanks for the review. I'll merge it soon

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.

3 participants