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-8613] [flip6] [yarn] Return excess containers #5436
[FLINK-8613] [flip6] [yarn] Return excess containers #5436
Conversation
numPendingContainerRequests); | ||
|
||
if (numPendingContainerRequests > 0) { | ||
numPendingContainerRequests = Math.max(0, numPendingContainerRequests - 1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't it enough to write numPendingContainerRequests--
?
If numPendingContainerRequests
is 0
, it won't go into this branch in the next iteration.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
true
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's ok.
} | ||
} | ||
|
||
// if we are waiting for no further containers, we can go to the | ||
// regular heartbeat interval | ||
if (numPendingContainerRequests <= 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think going negative should not be possible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes indeed. However, the logic should not be affected by using <=
as the comparison operator.
Let all Yarn entry points use the YarnConfigOptions.APPLICATION_MASTER_PORT option to specify the valid port range for the common RpcService. This closes apache#5388.
…n result retrieval Split RestClusterClient#submitJob into submitJob and requestJobResult which can be called individually. This closes apache#5428.
Register the JobTerminationHandler at the WebMonitorEndpoint to make it accessible to all REST endpoints. This closes apache#5429.
In order to support the job cancellation from the web UI, including when using Yarn, we have to register the JobTerminationHandler under /jobs/:jobid/yarn-cancel and /jobs/:jobid/yarn-stop. This is just a temporary fix until we can send arbitrary REST verbs through the Yarn proxy. This closes apache#5430.
The MiniDispatcher is responsible for submitting the single job with which a job mode cluster is started. Once the job has completed and if the cluster has been started in detached mode, the MiniDispatcher will terminate. In order to reduce code duplication, the MiniDispatcher is a sub class of the Dispatcher which is started with a single job submitted job graph store. This closes apache#5431.
This commit allows to deploy detached job mode clusters via the CliFrontend. In order to do that, it first extracts the JobGraph from the PackagedProgram and then uses the ClusterDescriptor to deploy the job mode cluster. This closes apache#5432.
Upon notification of newly allocated containers, the YarnResourceManager will only accept as many containers as there are pending container requests. All excess containers will be returned. This closes apache#5436.
Thanks for the review @GJL. I addressed your comments. Merging this PR. |
56ed240
to
d112084
Compare
What is the purpose of the change
Upon notification of newly allocated containers, the YarnResourceManager
will only accept as many containers as there are pending container requests.
All excess containers will be returned.
Does this pull request potentially affect one of the following parts:
@Public(Evolving)
: (no)Documentation