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

[FLINK-4871] [mini cluster] Add memory calculation for TaskManagers to MiniCluster #2669

Closed
wants to merge 4 commits into from

Conversation

tillrohrmann
Copy link
Contributor

@tillrohrmann tillrohrmann commented Oct 20, 2016

This PR is based on #2651, #2655 and #2657. Only 4918b63 is relevant.

If the managed memory size for the task manager has not been set in the Configuration, then
it is automatically calculated by dividing the available memory by the number of distributed
components. Additionally this PR allows to provide a MetricRegistry to the TaskManagerRunner.
That way it is possible to use the MiniCluster's MetricRegistry.

Add memory calculation for task managers.

Copy link
Contributor

@mxm mxm left a comment

Choose a reason for hiding this comment

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

+1 LGTM with just some minor comments.

*
* @return
*/
private long calculateManagedMemoryPerTaskManager() {
Copy link
Contributor

Choose a reason for hiding this comment

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

getOrCalculateManagedMemoryPerTaskManager? 😃

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, will rename the method.

// ------------------------------------------------------------------------
// getters
// ------------------------------------------------------------------------

public Configuration getConfiguration() {
// update the memory in case that we've changed the number of components (TM, RM, JM)
long memory = calculateManagedMemoryPerTaskManager();
Copy link
Contributor

Choose a reason for hiding this comment

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

Getters should usually not perform any calculation. How about changing the method name to updateConfiguration()?

Copy link
Contributor

Choose a reason for hiding this comment

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

After this method has been called, you can't change the memory configuration anymore because the config value will prevent new calculation in calculateManagedMemoryPerTaskManager. Is that desired?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

True. I guess we should create a new configuration every time this method is called. Will rename the method to then togenerateConfiguration.

@tillrohrmann
Copy link
Contributor Author

Thanks for the review @mxm. I will address your comments and if Travis gives green light, I will merge the PR.

Allowing the RpcEndpoint.start/shutDown to throw exceptions will help to let rpc endpoints
to quickly fail without having to use a callback like the FatalErrorHandler.

This closes apache#2651.
This PR introduces a FatalErrorHandler and the MetricRegistry to the RM. The FatalErrorHandler is used to handle fatal errors. Additionally, the PR adds the MetricRegistry to the RM which can be used
to register metrics.

Apart from these changes the PR restructures the code of the RM a little bit and fixes some
blocking operations.

The PR also moves the TestingFatalErrorHandler into the util package of flink-runtime test. That
it is usable across multiple tests.

Introduce ResourceManagerRunner to handle errors in the ResourceManager

This closes apache#2655.
…anager

Introduce the JobLeaderIdService which automatically retrieves the current job leader id.
This job leader id is used to validate job manager registartion attempts. Additionally, it
is used to disconnect old job leaders from the resource manager.

Add comments

This closes apache#2657.
…o MiniCluster

If the managed memory size for the task manager has not been set in the Configuration, then
it is automatically calculated by dividing the available memory by the number of distributed
components. Additionally this PR allows to provide a MetricRegistry to the TaskManagerRunner.
That way it is possible to use the MiniCluster's MetricRegistry.

Add memory calculation for task managers

This closes apache#2669.
asfgit pushed a commit that referenced this pull request Oct 23, 2016
…o MiniCluster

If the managed memory size for the task manager has not been set in the Configuration, then
it is automatically calculated by dividing the available memory by the number of distributed
components. Additionally this PR allows to provide a MetricRegistry to the TaskManagerRunner.
That way it is possible to use the MiniCluster's MetricRegistry.

Add memory calculation for task managers

This closes #2669.
@tillrohrmann
Copy link
Contributor Author

Merged

tillrohrmann added a commit to tillrohrmann/flink that referenced this pull request Oct 31, 2016
…o MiniCluster

If the managed memory size for the task manager has not been set in the Configuration, then
it is automatically calculated by dividing the available memory by the number of distributed
components. Additionally this PR allows to provide a MetricRegistry to the TaskManagerRunner.
That way it is possible to use the MiniCluster's MetricRegistry.

Add memory calculation for task managers

This closes apache#2669.
tillrohrmann added a commit to tillrohrmann/flink that referenced this pull request Oct 31, 2016
…o MiniCluster

If the managed memory size for the task manager has not been set in the Configuration, then
it is automatically calculated by dividing the available memory by the number of distributed
components. Additionally this PR allows to provide a MetricRegistry to the TaskManagerRunner.
That way it is possible to use the MiniCluster's MetricRegistry.

Add memory calculation for task managers

This closes apache#2669.
asfgit pushed a commit that referenced this pull request Nov 1, 2016
…o MiniCluster

If the managed memory size for the task manager has not been set in the Configuration, then
it is automatically calculated by dividing the available memory by the number of distributed
components. Additionally this PR allows to provide a MetricRegistry to the TaskManagerRunner.
That way it is possible to use the MiniCluster's MetricRegistry.

Add memory calculation for task managers

This closes #2669.
StephanEwen pushed a commit to StephanEwen/flink that referenced this pull request Dec 23, 2016
…o MiniCluster

If the managed memory size for the task manager has not been set in the Configuration, then
it is automatically calculated by dividing the available memory by the number of distributed
components. Additionally this PR allows to provide a MetricRegistry to the TaskManagerRunner.
That way it is possible to use the MiniCluster's MetricRegistry.

Add memory calculation for task managers

This closes apache#2669.
StephanEwen pushed a commit to StephanEwen/flink that referenced this pull request Dec 23, 2016
…o MiniCluster

If the managed memory size for the task manager has not been set in the Configuration, then
it is automatically calculated by dividing the available memory by the number of distributed
components. Additionally this PR allows to provide a MetricRegistry to the TaskManagerRunner.
That way it is possible to use the MiniCluster's MetricRegistry.

Add memory calculation for task managers

This closes apache#2669.
StephanEwen pushed a commit to StephanEwen/flink that referenced this pull request Dec 23, 2016
…o MiniCluster

If the managed memory size for the task manager has not been set in the Configuration, then
it is automatically calculated by dividing the available memory by the number of distributed
components. Additionally this PR allows to provide a MetricRegistry to the TaskManagerRunner.
That way it is possible to use the MiniCluster's MetricRegistry.

Add memory calculation for task managers

This closes apache#2669.
liuyuzhong7 pushed a commit to liuyuzhong7/flink that referenced this pull request Jan 4, 2017
…o MiniCluster

If the managed memory size for the task manager has not been set in the Configuration, then
it is automatically calculated by dividing the available memory by the number of distributed
components. Additionally this PR allows to provide a MetricRegistry to the TaskManagerRunner.
That way it is possible to use the MiniCluster's MetricRegistry.

Add memory calculation for task managers

This closes apache#2669.
liuyuzhong7 pushed a commit to liuyuzhong7/flink that referenced this pull request Jan 17, 2017
…o MiniCluster

If the managed memory size for the task manager has not been set in the Configuration, then
it is automatically calculated by dividing the available memory by the number of distributed
components. Additionally this PR allows to provide a MetricRegistry to the TaskManagerRunner.
That way it is possible to use the MiniCluster's MetricRegistry.

Add memory calculation for task managers

This closes apache#2669.
joseprupi pushed a commit to joseprupi/flink that referenced this pull request Feb 12, 2017
…o MiniCluster

If the managed memory size for the task manager has not been set in the Configuration, then
it is automatically calculated by dividing the available memory by the number of distributed
components. Additionally this PR allows to provide a MetricRegistry to the TaskManagerRunner.
That way it is possible to use the MiniCluster's MetricRegistry.

Add memory calculation for task managers

This closes apache#2669.
@tillrohrmann tillrohrmann deleted the miniClusterUpdate branch March 6, 2017 15:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants