You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@twill.apache.org by ch...@apache.org on 2017/02/28 19:45:22 UTC
[8/9] twill git commit: Get rid of the inner loop in the doRun method
Get rid of the inner loop in the doRun method
- The inner loop can block the heartbeat thread for too long if there are a lot of runnable instances to stop
Project: http://git-wip-us.apache.org/repos/asf/twill/repo
Commit: http://git-wip-us.apache.org/repos/asf/twill/commit/aa64afae
Tree: http://git-wip-us.apache.org/repos/asf/twill/tree/aa64afae
Diff: http://git-wip-us.apache.org/repos/asf/twill/diff/aa64afae
Branch: refs/heads/feature/twill-186
Commit: aa64afae5428c9e1ea5490e3e86f337cd9efefc0
Parents: 720ed90
Author: Terence Yim <ch...@apache.org>
Authored: Tue Feb 28 11:34:35 2017 -0800
Committer: Terence Yim <ch...@apache.org>
Committed: Tue Feb 28 11:34:35 2017 -0800
----------------------------------------------------------------------
.../appmaster/ApplicationMasterService.java | 37 +++++++++-----------
.../appmaster/RunnableContainerRequest.java | 6 ++--
2 files changed, 19 insertions(+), 24 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/twill/blob/aa64afae/twill-yarn/src/main/java/org/apache/twill/internal/appmaster/ApplicationMasterService.java
----------------------------------------------------------------------
diff --git a/twill-yarn/src/main/java/org/apache/twill/internal/appmaster/ApplicationMasterService.java b/twill-yarn/src/main/java/org/apache/twill/internal/appmaster/ApplicationMasterService.java
index 3fed157..567b57d 100644
--- a/twill-yarn/src/main/java/org/apache/twill/internal/appmaster/ApplicationMasterService.java
+++ b/twill-yarn/src/main/java/org/apache/twill/internal/appmaster/ApplicationMasterService.java
@@ -379,6 +379,8 @@ public final class ApplicationMasterService extends AbstractYarnTwillService imp
boolean isRequestRelaxed = false;
long nextTimeoutCheck = System.currentTimeMillis() + Constants.PROVISION_TIMEOUT;
while (!stopped) {
+ TimeUnit.SECONDS.sleep(1);
+
// Call allocate. It has to be made at first in order to be able to get cluster resource availability.
amClient.allocate(0.0f, allocateHandler);
@@ -389,28 +391,22 @@ public final class ApplicationMasterService extends AbstractYarnTwillService imp
}
// If nothing is in provisioning, and no pending request, move to next one
- int count = runnableContainerRequests.size();
- LOG.debug("Runnable container requests: {}", count);
- while (provisioning.isEmpty() && currentRequest == null && !runnableContainerRequests.isEmpty()) {
- RunnableContainerRequest runnableContainerRequest = runnableContainerRequests.peek();
- if (!runnableContainerRequest.isReadyToBeProvisioned()) {
- // take it out from queue and put it back at the end for second chance.
- runnableContainerRequest = runnableContainerRequests.poll();
- runnableContainerRequests.add(runnableContainerRequest);
- LOG.debug("Request not ready: {}", runnableContainerRequest);
-
- // We checked all the requests that were pending when we started this loop
- // Any remaining requests are not ready to be provisioned
- if (--count <= 0) {
- break;
- }
+ if (provisioning.isEmpty() && currentRequest == null && !runnableContainerRequests.isEmpty()) {
+ RunnableContainerRequest containerRequest = runnableContainerRequests.peek();
+ // If the request at the head of the request queue is not yet ready, move it to the end of the queue
+ // so that it won't block requests that are already ready
+ if (!containerRequest.isReadyToBeProvisioned()) {
+ LOG.debug("Request not ready: {}", containerRequest);
+ runnableContainerRequests.add(runnableContainerRequests.poll());
continue;
}
- currentRequest = runnableContainerRequest.takeRequest();
+
+ currentRequest = containerRequest.takeRequest();
if (currentRequest == null) {
// All different types of resource request from current order is done, move to next one
// TODO: Need to handle order type as well
runnableContainerRequests.poll();
+ continue;
}
}
@@ -435,10 +431,6 @@ public final class ApplicationMasterService extends AbstractYarnTwillService imp
}
nextTimeoutCheck = checkProvisionTimeout(nextTimeoutCheck);
-
- if (isRunning()) {
- TimeUnit.SECONDS.sleep(1);
- }
}
}
@@ -940,8 +932,11 @@ public final class ApplicationMasterService extends AbstractYarnTwillService imp
}
}
+ LOG.info("All instances in {} for runnable {} are stopped. Ready to provision",
+ instancesToRemove, runnableName);
+
// set the container request to be ready
- containerRequest.setReadyToBeProvisioned(true);
+ containerRequest.setReadyToBeProvisioned();
// For all runnables that needs to re-request for containers, update the expected count timestamp
// so that the EventHandler would be triggered with the right expiration timestamp.
http://git-wip-us.apache.org/repos/asf/twill/blob/aa64afae/twill-yarn/src/main/java/org/apache/twill/internal/appmaster/RunnableContainerRequest.java
----------------------------------------------------------------------
diff --git a/twill-yarn/src/main/java/org/apache/twill/internal/appmaster/RunnableContainerRequest.java b/twill-yarn/src/main/java/org/apache/twill/internal/appmaster/RunnableContainerRequest.java
index e001121..1dcffed 100644
--- a/twill-yarn/src/main/java/org/apache/twill/internal/appmaster/RunnableContainerRequest.java
+++ b/twill-yarn/src/main/java/org/apache/twill/internal/appmaster/RunnableContainerRequest.java
@@ -53,12 +53,12 @@ final class RunnableContainerRequest {
return orderType;
}
- public boolean isReadyToBeProvisioned() {
+ boolean isReadyToBeProvisioned() {
return isReadyToBeProvisioned;
}
- public void setReadyToBeProvisioned(boolean isProvisioned) {
- this.isReadyToBeProvisioned = isProvisioned;
+ void setReadyToBeProvisioned() {
+ this.isReadyToBeProvisioned = true;
}
/**