You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@tajo.apache.org by jinossy <gi...@git.apache.org> on 2015/06/21 17:12:32 UTC
[GitHub] tajo pull request: TAJO-1397
GitHub user jinossy opened a pull request:
https://github.com/apache/tajo/pull/608
TAJO-1397
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/jinossy/tajo TAJO-1397
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/tajo/pull/608.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #608
----
commit 6dd0c0a90b1c2aaa56e3f144ecc57c996715c481
Author: Jinho Kim <jh...@apache.org>
Date: 2015-06-01T08:58:08Z
merge from TAJO-1615
commit 91319ee3eed1c950551071eca10a6587f635a48e
Author: Jinho Kim <jh...@apache.org>
Date: 2015-06-08T07:57:23Z
TAJO-1397
commit aba1188da4452c3c641e48411d6dd4a24998d93f
Author: Jinho Kim <jh...@apache.org>
Date: 2015-06-08T07:57:39Z
Merge branch 'master' of https://git-wip-us.apache.org/repos/asf/tajo into TAJO-1397
commit 1bfa430fd65006b79bfcdc382b0b7aeb17bd9686
Author: Jinho Kim <jh...@apache.org>
Date: 2015-06-16T11:13:30Z
TAJO-1397
commit 13e71b99ea0136fe5ad7f2489af0883664ca6547
Author: Jinho Kim <jh...@apache.org>
Date: 2015-06-16T11:13:46Z
Merge branch 'master' of https://git-wip-us.apache.org/repos/asf/tajo into TAJO-1397
commit 133e3f2c04a435a6da9ae823be0fa2dd803db377
Author: Jinho Kim <jh...@apache.org>
Date: 2015-06-17T15:50:18Z
TAJO-1397
commit b347e04aa5372bfb34f0e283aead42f9ff904112
Author: Jinho Kim <jh...@apache.org>
Date: 2015-06-18T02:16:12Z
TAJO-1397
commit b397ff15eedf982de704fb38ec1abe9a0f69df69
Author: Jinho Kim <jh...@apache.org>
Date: 2015-06-18T07:16:28Z
TAJO-1397
commit 4fab8787591302dace885417f4487976d586642d
Author: Jinho Kim <jh...@apache.org>
Date: 2015-06-18T08:01:07Z
remove unused codes
commit b82916043dccb26dd4dcb20f5ae70c1c202d5327
Author: Jinho Kim <jh...@apache.org>
Date: 2015-06-18T08:38:46Z
add debug message
commit e5318b8030bbf1821aacbe0ddae45d94bc403afa
Author: Jinho Kim <jh...@apache.org>
Date: 2015-06-18T09:23:07Z
add changable heartBeatInterval
commit 7ec50d034f9cd1bbc2fc7cce3eda35324681fee8
Author: Jinho Kim <jh...@apache.org>
Date: 2015-06-19T03:40:06Z
TAJO-1397
commit a8424871c2fb9a6f7af700f198ebe2f332852a80
Author: Jinho Kim <jh...@apache.org>
Date: 2015-06-19T04:15:14Z
fix wrong QM address
commit 50b78e6d4e4f6f9464f149cf3452d51115d9c22e
Author: Jinho Kim <jh...@apache.org>
Date: 2015-06-19T04:28:30Z
TAJO-1397
commit a520494bd141da189074e1ea170efaadd3bbd3a0
Author: Jinho Kim <jh...@apache.org>
Date: 2015-06-19T07:33:35Z
TAJO-1397
commit 3983008808e3351ba723941095643feae458eb49
Author: Jinho Kim <jh...@apache.org>
Date: 2015-06-21T14:42:53Z
remove StartExecutionBlock event
commit 749cfa2bf742bd913d4faf57b5ba7a673ebabb5b
Author: Jinho Kim <jh...@apache.org>
Date: 2015-06-21T14:44:03Z
Merge branch 'master' of https://git-wip-us.apache.org/repos/asf/tajo into TAJO-1397
commit 07b7b91f8c3cb8614dcf4ecc6e19d6e790a220ec
Author: Jinho Kim <jh...@apache.org>
Date: 2015-06-21T14:47:44Z
remove unused class
commit ee0c34e59df250e5b88f91b2d8c2d6b231318bde
Author: Jinho Kim <jh...@apache.org>
Date: 2015-06-21T15:07:40Z
add internal error handling in DefaultTaskScheduler
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1397: Resource allocation should be fine g...
Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on a diff in the pull request:
https://github.com/apache/tajo/pull/608#discussion_r34864506
--- Diff: tajo-core/src/main/java/org/apache/tajo/querymaster/DefaultTaskScheduler.java ---
@@ -89,20 +99,29 @@ public void init(Configuration conf) {
@Override
public void start() {
LOG.info("Start TaskScheduler");
+ isLeaf = stage.getMasterPlan().isLeaf(stage.getBlock());
+
+ if (!isLeaf) {
--- End diff --
Please fix this condition.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1397: Resource allocation should be fine g...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/tajo/pull/608
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1397: Resource allocation should be fine g...
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on a diff in the pull request:
https://github.com/apache/tajo/pull/608#discussion_r34887031
--- Diff: tajo-core/src/main/java/org/apache/tajo/engine/query/TaskRequest.java ---
@@ -21,32 +21,31 @@
*/
package org.apache.tajo.engine.query;
+import org.apache.tajo.ResourceProtos.TaskRequestProto;
import org.apache.tajo.TaskAttemptId;
import org.apache.tajo.catalog.proto.CatalogProtos;
import org.apache.tajo.common.ProtoObject;
import org.apache.tajo.conf.TajoConf;
import org.apache.tajo.engine.planner.enforce.Enforcer;
import org.apache.tajo.engine.planner.global.DataChannel;
-import org.apache.tajo.ipc.TajoWorkerProtocol;
import org.apache.tajo.plan.serder.PlanProto;
import org.apache.tajo.worker.FetchImpl;
import java.util.List;
-public interface TaskRequest extends ProtoObject<TajoWorkerProtocol.TaskRequestProto> {
+public interface TaskRequest extends ProtoObject<TaskRequestProto> {
- public TaskAttemptId getId();
- public List<CatalogProtos.FragmentProto> getFragments();
- public String getOutputTableId();
- public boolean isClusteredOutput();
- public PlanProto.LogicalNodeTree getPlan();
- public boolean isInterQuery();
- public void setInterQuery();
- public void addFetch(String name, FetchImpl fetch);
- public List<FetchImpl> getFetches();
- public boolean shouldDie();
- public void setShouldDie();
- public QueryContext getQueryContext(TajoConf conf);
- public DataChannel getDataChannel();
- public Enforcer getEnforcer();
+ String getQueryMasterHostAndPort();
+ TaskAttemptId getId();
+ List<CatalogProtos.FragmentProto> getFragments();
+ String getOutputTableId();
+ boolean isClusteredOutput();
--- End diff --
unused method
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1397: Resource allocation should be fine g...
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on a diff in the pull request:
https://github.com/apache/tajo/pull/608#discussion_r34889423
--- Diff: .travis.yml ---
@@ -1,4 +1,5 @@
# Licensed to the Apache Software Foundation (ASF) under one or more
+
--- End diff --
It seems to be a mistake enter.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1397: Resource allocation should be fine g...
Posted by jinossy <gi...@git.apache.org>.
Github user jinossy commented on a diff in the pull request:
https://github.com/apache/tajo/pull/608#discussion_r34868312
--- Diff: tajo-core/src/main/java/org/apache/tajo/querymaster/DefaultTaskScheduler.java ---
@@ -89,20 +99,29 @@ public void init(Configuration conf) {
@Override
public void start() {
LOG.info("Start TaskScheduler");
+ isLeaf = stage.getMasterPlan().isLeaf(stage.getBlock());
+
+ if (!isLeaf) {
--- End diff --
It is my big mistake. Thank you!!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1397: Resource allocation should be fine g...
Posted by jinossy <gi...@git.apache.org>.
Github user jinossy commented on the pull request:
https://github.com/apache/tajo/pull/608#issuecomment-121178442
This patch is ready, please review this PR
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1397: Resource allocation should be fine g...
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on a diff in the pull request:
https://github.com/apache/tajo/pull/608#discussion_r34887052
--- Diff: tajo-core/src/main/java/org/apache/tajo/engine/query/TaskRequest.java ---
@@ -21,32 +21,31 @@
*/
package org.apache.tajo.engine.query;
+import org.apache.tajo.ResourceProtos.TaskRequestProto;
import org.apache.tajo.TaskAttemptId;
import org.apache.tajo.catalog.proto.CatalogProtos;
import org.apache.tajo.common.ProtoObject;
import org.apache.tajo.conf.TajoConf;
import org.apache.tajo.engine.planner.enforce.Enforcer;
import org.apache.tajo.engine.planner.global.DataChannel;
-import org.apache.tajo.ipc.TajoWorkerProtocol;
import org.apache.tajo.plan.serder.PlanProto;
import org.apache.tajo.worker.FetchImpl;
import java.util.List;
-public interface TaskRequest extends ProtoObject<TajoWorkerProtocol.TaskRequestProto> {
+public interface TaskRequest extends ProtoObject<TaskRequestProto> {
- public TaskAttemptId getId();
- public List<CatalogProtos.FragmentProto> getFragments();
- public String getOutputTableId();
- public boolean isClusteredOutput();
- public PlanProto.LogicalNodeTree getPlan();
- public boolean isInterQuery();
- public void setInterQuery();
- public void addFetch(String name, FetchImpl fetch);
- public List<FetchImpl> getFetches();
- public boolean shouldDie();
- public void setShouldDie();
- public QueryContext getQueryContext(TajoConf conf);
- public DataChannel getDataChannel();
- public Enforcer getEnforcer();
+ String getQueryMasterHostAndPort();
+ TaskAttemptId getId();
+ List<CatalogProtos.FragmentProto> getFragments();
+ String getOutputTableId();
+ boolean isClusteredOutput();
+ PlanProto.LogicalNodeTree getPlan();
+ boolean isInterQuery();
--- End diff --
unused method
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1397: Resource allocation should be fine g...
Posted by jinossy <gi...@git.apache.org>.
Github user jinossy commented on a diff in the pull request:
https://github.com/apache/tajo/pull/608#discussion_r34868642
--- Diff: tajo-core/src/main/java/org/apache/tajo/master/rm/Worker.java ---
@@ -34,17 +39,37 @@
/** class logger */
private static final Log LOG = LogFactory.getLog(Worker.class);
--- End diff --
The worker class has a status internally. How about "NodeStatus" ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1397: Resource allocation should be fine g...
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on a diff in the pull request:
https://github.com/apache/tajo/pull/608#discussion_r34886998
--- Diff: tajo-core/src/main/java/org/apache/tajo/engine/query/TaskRequest.java ---
@@ -21,32 +21,31 @@
*/
package org.apache.tajo.engine.query;
+import org.apache.tajo.ResourceProtos.TaskRequestProto;
import org.apache.tajo.TaskAttemptId;
import org.apache.tajo.catalog.proto.CatalogProtos;
import org.apache.tajo.common.ProtoObject;
import org.apache.tajo.conf.TajoConf;
import org.apache.tajo.engine.planner.enforce.Enforcer;
import org.apache.tajo.engine.planner.global.DataChannel;
-import org.apache.tajo.ipc.TajoWorkerProtocol;
import org.apache.tajo.plan.serder.PlanProto;
import org.apache.tajo.worker.FetchImpl;
import java.util.List;
-public interface TaskRequest extends ProtoObject<TajoWorkerProtocol.TaskRequestProto> {
+public interface TaskRequest extends ProtoObject<TaskRequestProto> {
- public TaskAttemptId getId();
- public List<CatalogProtos.FragmentProto> getFragments();
- public String getOutputTableId();
- public boolean isClusteredOutput();
- public PlanProto.LogicalNodeTree getPlan();
- public boolean isInterQuery();
- public void setInterQuery();
- public void addFetch(String name, FetchImpl fetch);
- public List<FetchImpl> getFetches();
- public boolean shouldDie();
- public void setShouldDie();
- public QueryContext getQueryContext(TajoConf conf);
- public DataChannel getDataChannel();
- public Enforcer getEnforcer();
+ String getQueryMasterHostAndPort();
--- End diff --
unused method
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1397: Resource allocation should be fine g...
Posted by jinossy <gi...@git.apache.org>.
Github user jinossy commented on the pull request:
https://github.com/apache/tajo/pull/608#issuecomment-121798206
@jihoonson
Thank you for your review.
PARALLEL_QUERY_LIMIT is maximum running QM. I will change to MAXIMUM_RUNNING_QM_RATE
I'm going to update the patch that reflect your comments
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1397: Resource allocation should be fine g...
Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on a diff in the pull request:
https://github.com/apache/tajo/pull/608#discussion_r34864411
--- Diff: tajo-core/src/main/java/org/apache/tajo/master/QueryManager.java ---
@@ -178,35 +174,29 @@ public QueryInfo scheduleQuery(Session session, QueryContext queryContext, Strin
QueryInProgress queryInProgress = new QueryInProgress(masterContext, session, queryContext, queryId, sql,
jsonExpr, plan);
- synchronized (submittedQueries) {
- queryInProgress.getQueryInfo().setQueryMaster("");
- submittedQueries.put(queryInProgress.getQueryId(), queryInProgress);
- }
+ queryInProgress.getQueryInfo().setQueryMaster("");
--- End diff --
How about defining a constant to represent an empty query master host?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1397: Resource allocation should be fine g...
Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on the pull request:
https://github.com/apache/tajo/pull/608#issuecomment-122619034
The latest patch looks good to me.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1397: Resource allocation should be fine g...
Posted by jinossy <gi...@git.apache.org>.
Github user jinossy commented on a diff in the pull request:
https://github.com/apache/tajo/pull/608#discussion_r34868269
--- Diff: tajo-core/src/main/java/org/apache/tajo/querymaster/DefaultTaskScheduler.java ---
@@ -251,63 +250,70 @@ public void handle(TaskSchedulerEvent event) {
}
}
- @Override
- public void handleTaskRequestEvent(TaskRequestEvent event) {
-
- taskRequests.handle(event);
- int hosts = scheduledRequests.leafTaskHostMapping.size();
+ private Set<Integer> getWorkerIds(Collection<String> hosts){
+ Set<Integer> workerIds = Sets.newHashSet();
+ if(hosts.isEmpty()) return workerIds;
- // if available cluster resource are large then tasks, the scheduler thread are working immediately.
- if(remainingScheduledObjectNum() > 0 &&
- (remainingScheduledObjectNum() <= hosts || hosts <= taskRequests.size())){
- synchronized (schedulingThread){
- schedulingThread.notifyAll();
+ for (WorkerConnectionInfo worker : stage.getContext().getWorkerMap().values()) {
+ if(hosts.contains(worker.getHost())){
+ workerIds.add(worker.getId());
}
}
+ return workerIds;
}
- @Override
- public int remainingScheduledObjectNum() {
- return scheduledObjectNum;
- }
- private class TaskRequests implements EventHandler<TaskRequestEvent> {
- private final LinkedBlockingQueue<TaskRequestEvent> taskRequestQueue =
- new LinkedBlockingQueue<TaskRequestEvent>();
+ protected LinkedList<TaskRequestEvent> createTaskRequest() throws Exception {
+ LinkedList<TaskRequestEvent> taskRequestEvents = new LinkedList<TaskRequestEvent>();
- @Override
- public void handle(TaskRequestEvent event) {
- if(LOG.isDebugEnabled()){
- LOG.debug("TaskRequest: " + event.getContainerId() + "," + event.getExecutionBlockId());
- }
+ int requestContainerNum = Math.max(remainingScheduledObjectNum(), 1);
--- End diff --
Nice finding! It is my mistake. I will remove the Math.max
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1397: Resource allocation should be fine g...
Posted by jinossy <gi...@git.apache.org>.
Github user jinossy commented on the pull request:
https://github.com/apache/tajo/pull/608#issuecomment-122219316
I've rename **Worker** to **NodeStatus**
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1397: Resource allocation should be fine g...
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on a diff in the pull request:
https://github.com/apache/tajo/pull/608#discussion_r34887805
--- Diff: tajo-core/src/main/java/org/apache/tajo/master/rm/NodeStatus.java ---
@@ -0,0 +1,293 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.tajo.master.rm;
+
+import io.netty.util.internal.PlatformDependent;
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
+import org.apache.hadoop.yarn.event.EventHandler;
+import org.apache.hadoop.yarn.state.*;
+import org.apache.tajo.master.cluster.WorkerConnectionInfo;
+import org.apache.tajo.resource.NodeResource;
+import org.apache.tajo.resource.NodeResources;
+import org.apache.tajo.util.TUtil;
+
+import java.util.EnumSet;
+import java.util.concurrent.atomic.AtomicIntegerFieldUpdater;
+import java.util.concurrent.atomic.AtomicLongFieldUpdater;
+
+/**
+ * It contains resource and various information for a node.
+ */
+public class NodeStatus implements EventHandler<NodeEvent>, Comparable<NodeStatus> {
+ /** class logger */
+ private static final Log LOG = LogFactory.getLog(NodeStatus.class);
+
+ /** context of {@link TajoResourceManager} */
+ private final TajoRMContext rmContext;
+
+ /** last heartbeat time */
+ private volatile long lastHeartbeatTime;
+
+ private volatile int numRunningTasks;
+
+ private volatile int numRunningQueryMaster;
+
+ private static AtomicLongFieldUpdater HEARTBEAT_TIME_UPDATER;
+ private static AtomicIntegerFieldUpdater RUNNING_TASK_UPDATER;
+ private static AtomicIntegerFieldUpdater RUNNING_QM_UPDATER;
+
+ static {
+ HEARTBEAT_TIME_UPDATER = PlatformDependent.newAtomicLongFieldUpdater(NodeStatus.class, "lastHeartbeatTime");
+ if (HEARTBEAT_TIME_UPDATER == null) {
+ HEARTBEAT_TIME_UPDATER = AtomicLongFieldUpdater.newUpdater(NodeStatus.class, "lastHeartbeatTime");
+ RUNNING_TASK_UPDATER = AtomicIntegerFieldUpdater.newUpdater(NodeStatus.class, "numRunningTasks");
+ RUNNING_QM_UPDATER = AtomicIntegerFieldUpdater.newUpdater(NodeStatus.class, "numRunningQueryMaster");
+ } else {
+ RUNNING_TASK_UPDATER = PlatformDependent.newAtomicIntegerFieldUpdater(NodeStatus.class, "numRunningTasks");
+ RUNNING_QM_UPDATER = PlatformDependent.newAtomicIntegerFieldUpdater(NodeStatus.class, "numRunningQueryMaster");
+ }
+ }
+
+ /** Available resources on the node. */
+ private final NodeResource availableResource;
+
+ /** Total resources on the node. */
+ private final NodeResource totalResourceCapability;
+
+ /** Node connection information */
+ private WorkerConnectionInfo connectionInfo;
+
+ private static final ReconnectNodeTransition RECONNECT_NODE_TRANSITION = new ReconnectNodeTransition();
+ private static final StatusUpdateTransition STATUS_UPDATE_TRANSITION = new StatusUpdateTransition();
+
+ private static final StateMachineFactory<NodeStatus,
+ NodeState,
+ NodeEventType,
+ NodeEvent> stateMachineFactory
+ = new StateMachineFactory<NodeStatus,
+ NodeState,
+ NodeEventType,
+ NodeEvent>(NodeState.NEW)
+
+ // Transition from NEW
+ .addTransition(NodeState.NEW, NodeState.RUNNING,
+ NodeEventType.STARTED,
+ new AddNodeTransition())
+
+ // Transition from RUNNING
+ .addTransition(NodeState.RUNNING, EnumSet.of(NodeState.RUNNING, NodeState.UNHEALTHY),
+ NodeEventType.STATE_UPDATE,
+ STATUS_UPDATE_TRANSITION)
+ .addTransition(NodeState.RUNNING, NodeState.LOST,
+ NodeEventType.EXPIRE,
+ new DeactivateNodeTransition(NodeState.LOST))
+ .addTransition(NodeState.RUNNING, NodeState.RUNNING,
+ NodeEventType.RECONNECTED,
+ RECONNECT_NODE_TRANSITION)
+
+ // Transitions from UNHEALTHY state
+ .addTransition(NodeState.UNHEALTHY, EnumSet.of(NodeState.RUNNING, NodeState.UNHEALTHY),
+ NodeEventType.STATE_UPDATE,
+ STATUS_UPDATE_TRANSITION)
+ .addTransition(NodeState.UNHEALTHY, NodeState.LOST,
+ NodeEventType.EXPIRE,
+ new DeactivateNodeTransition(NodeState.LOST))
+ .addTransition(NodeState.UNHEALTHY, NodeState.UNHEALTHY,
+ NodeEventType.RECONNECTED,
+ RECONNECT_NODE_TRANSITION);
+
+ private final StateMachine<NodeState, NodeEventType, NodeEvent> stateMachine =
+ stateMachineFactory.make(this, NodeState.NEW);
+
+ public NodeStatus(TajoRMContext rmContext, NodeResource totalResourceCapability, WorkerConnectionInfo connectionInfo) {
+ this.rmContext = rmContext;
+
+ this.connectionInfo = connectionInfo;
+ this.lastHeartbeatTime = System.currentTimeMillis();
+ this.totalResourceCapability = totalResourceCapability;
+ this.availableResource = NodeResources.clone(totalResourceCapability);
+ }
+
+ public int getWorkerId() {
+ return connectionInfo.getId();
+ }
+
+ public WorkerConnectionInfo getConnectionInfo() {
+ return connectionInfo;
+ }
+
+ public void setLastHeartbeatTime(long lastheartbeatReportTime) {
+ HEARTBEAT_TIME_UPDATER.lazySet(this, lastheartbeatReportTime);
+ }
+
+ public void setNumRunningQueryMaster(int numRunningQueryMaster) {
+ RUNNING_QM_UPDATER.lazySet(this, numRunningQueryMaster);
+ }
+
+ public int getNumRunningQueryMaster() {
+ return numRunningQueryMaster;
+ }
+
+ public void setNumRunningTasks(int numRunningTasks) {
+ RUNNING_TASK_UPDATER.lazySet(this, numRunningTasks);
--- End diff --
``RUNNING_TASK_UPDATE`` contains the number of running tasks. But, the getter method ``getNumRunningTasks()`` uses just the member variable ``numrunningTasks``. It seems to be invalid.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1397: Resource allocation should be fine g...
Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on a diff in the pull request:
https://github.com/apache/tajo/pull/608#discussion_r34864385
--- Diff: tajo-core/src/main/java/org/apache/tajo/master/QueryInProgress.java ---
@@ -53,9 +56,9 @@
private LogicalRootNode plan;
- private AtomicBoolean querySubmitted = new AtomicBoolean(false);
+ private volatile boolean querySubmitted = false;
- private AtomicBoolean stopped = new AtomicBoolean(false);
+ private AtomicBoolean isStopped = new AtomicBoolean(false);
--- End diff --
This variable also seems to be able to be defined as a volatile boolean instead of AtomicBoolean.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1397: Resource allocation should be fine g...
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on a diff in the pull request:
https://github.com/apache/tajo/pull/608#discussion_r34887013
--- Diff: tajo-core/src/main/java/org/apache/tajo/engine/query/TaskRequest.java ---
@@ -21,32 +21,31 @@
*/
package org.apache.tajo.engine.query;
+import org.apache.tajo.ResourceProtos.TaskRequestProto;
import org.apache.tajo.TaskAttemptId;
import org.apache.tajo.catalog.proto.CatalogProtos;
import org.apache.tajo.common.ProtoObject;
import org.apache.tajo.conf.TajoConf;
import org.apache.tajo.engine.planner.enforce.Enforcer;
import org.apache.tajo.engine.planner.global.DataChannel;
-import org.apache.tajo.ipc.TajoWorkerProtocol;
import org.apache.tajo.plan.serder.PlanProto;
import org.apache.tajo.worker.FetchImpl;
import java.util.List;
-public interface TaskRequest extends ProtoObject<TajoWorkerProtocol.TaskRequestProto> {
+public interface TaskRequest extends ProtoObject<TaskRequestProto> {
- public TaskAttemptId getId();
- public List<CatalogProtos.FragmentProto> getFragments();
- public String getOutputTableId();
- public boolean isClusteredOutput();
- public PlanProto.LogicalNodeTree getPlan();
- public boolean isInterQuery();
- public void setInterQuery();
- public void addFetch(String name, FetchImpl fetch);
- public List<FetchImpl> getFetches();
- public boolean shouldDie();
- public void setShouldDie();
- public QueryContext getQueryContext(TajoConf conf);
- public DataChannel getDataChannel();
- public Enforcer getEnforcer();
+ String getQueryMasterHostAndPort();
+ TaskAttemptId getId();
+ List<CatalogProtos.FragmentProto> getFragments();
+ String getOutputTableId();
--- End diff --
unused method
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1397: Resource allocation should be fine g...
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on a diff in the pull request:
https://github.com/apache/tajo/pull/608#discussion_r34886637
--- Diff: tajo-common/src/main/java/org/apache/tajo/util/TUtil.java ---
@@ -19,6 +19,7 @@
package org.apache.tajo.util;
import com.google.common.base.Objects;
+import org.apache.tajo.conf.TajoConf;
--- End diff --
This is unused.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1397: Resource allocation should be fine g...
Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on the pull request:
https://github.com/apache/tajo/pull/608#issuecomment-122786979
+1 ship it!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1397: Resource allocation should be fine g...
Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on a diff in the pull request:
https://github.com/apache/tajo/pull/608#discussion_r34864885
--- Diff: tajo-core/src/main/java/org/apache/tajo/master/rm/Worker.java ---
@@ -34,17 +39,37 @@
/** class logger */
private static final Log LOG = LogFactory.getLog(Worker.class);
--- End diff --
The name of this class looks less intuitive. How about changing it to a more intuitive name such as LiveWorkerInfo? If you have a good idea, please talk to me.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1397: Resource allocation should be fine g...
Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on the pull request:
https://github.com/apache/tajo/pull/608#issuecomment-121633024
@jinossy, really nice work!
I'm still reviewing and not finished yet. Here, I left some comments first, and will finish soon.
* StringUtils.java
* The comment of isPureAscii() is incomplete.
* QueryCoordinatorProtocol.proto
* As you know, @hyunsik is working on refactoring our error propagation system. According to the guide at https://cwiki.apache.org/confluence/display/TAJO/RPC+Protocol+Design+Guide, please separate QueryCoordinatorProtocolService from this file.
* Also, please add some comments for the methods of QueryCoordinatorProtocolService.
* QueryInProgress.java
* Please change the comments of allocateToQueryMaster() into the formal java doc format.
* Please add some comments for submitToQueryMaster().
* QueryManager.java
* The name of startQueryJob() looks not intuitive. How about isQueryExecutable()?
* TajoResourceManager.java
* Please remove the unused parameter, ```systemConf```, from the constructor.
* QuerySchedulingInfo.java
* Please add some comments for the class and its variables.
* SimpleScheduler.java
* Please add some comments for the class.
* Would you please explain what PARALLEL_QUERY_LIMIT means?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1397: Resource allocation should be fine g...
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on the pull request:
https://github.com/apache/tajo/pull/608#issuecomment-122786068
LGTM. Here is my +1.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1397: Resource allocation should be fine g...
Posted by jinossy <gi...@git.apache.org>.
Github user jinossy commented on the pull request:
https://github.com/apache/tajo/pull/608#issuecomment-121877409
@jihoonson
I've update the patch but startQueryJob is not condition method
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1397
Posted by jinossy <gi...@git.apache.org>.
Github user jinossy commented on the pull request:
https://github.com/apache/tajo/pull/608#issuecomment-113912553
This PR is not ready to review. please don't start review.
I just test the Travis-CI
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1397: Resource allocation should be fine g...
Posted by hyunsik <gi...@git.apache.org>.
Github user hyunsik commented on a diff in the pull request:
https://github.com/apache/tajo/pull/608#discussion_r34887204
--- Diff: tajo-core/src/main/java/org/apache/tajo/master/event/TaskAttemptToSchedulerEvent.java ---
@@ -44,30 +43,19 @@ public TaskAttemptScheduleContext getContext() {
}
public static class TaskAttemptScheduleContext {
- private TajoContainerId containerId;
private String host;
- private RpcCallback<TajoWorkerProtocol.TaskRequestProto> callback;
+ private RpcCallback<TaskRequestProto> callback;
public TaskAttemptScheduleContext() {
}
- public TaskAttemptScheduleContext(TajoContainerId containerId,
- String host,
- RpcCallback<TajoWorkerProtocol.TaskRequestProto> callback) {
- this.containerId = containerId;
+ public TaskAttemptScheduleContext(String host,
--- End diff --
Unused constructor
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1397: Resource allocation should be fine g...
Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on a diff in the pull request:
https://github.com/apache/tajo/pull/608#discussion_r34864494
--- Diff: tajo-core/src/main/java/org/apache/tajo/master/scheduler/AbstractQueryScheduler.java ---
@@ -0,0 +1,70 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.tajo.master.scheduler;
+
+import org.apache.hadoop.service.AbstractService;
+import org.apache.tajo.QueryId;
+import org.apache.tajo.resource.NodeResource;
+import org.apache.tajo.resource.NodeResources;
+import org.apache.tajo.resource.ResourceCalculator;
+
+public abstract class AbstractQueryScheduler extends AbstractService implements TajoResourceScheduler {
--- End diff --
I know that there are detailed comments at TajoResourceScheduler interface.
So, please add a simple comment such as ```please refer to TajoResourceScheduler for detailed information```.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1397: Resource allocation should be fine g...
Posted by jinossy <gi...@git.apache.org>.
Github user jinossy commented on the pull request:
https://github.com/apache/tajo/pull/608#issuecomment-122522460
@hyunsik
Thank you for your review
I've update the patch that reflects your comments
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tajo pull request: TAJO-1397: Resource allocation should be fine g...
Posted by jihoonson <gi...@git.apache.org>.
Github user jihoonson commented on a diff in the pull request:
https://github.com/apache/tajo/pull/608#discussion_r34864573
--- Diff: tajo-core/src/main/java/org/apache/tajo/querymaster/DefaultTaskScheduler.java ---
@@ -251,63 +250,70 @@ public void handle(TaskSchedulerEvent event) {
}
}
- @Override
- public void handleTaskRequestEvent(TaskRequestEvent event) {
-
- taskRequests.handle(event);
- int hosts = scheduledRequests.leafTaskHostMapping.size();
+ private Set<Integer> getWorkerIds(Collection<String> hosts){
+ Set<Integer> workerIds = Sets.newHashSet();
+ if(hosts.isEmpty()) return workerIds;
- // if available cluster resource are large then tasks, the scheduler thread are working immediately.
- if(remainingScheduledObjectNum() > 0 &&
- (remainingScheduledObjectNum() <= hosts || hosts <= taskRequests.size())){
- synchronized (schedulingThread){
- schedulingThread.notifyAll();
+ for (WorkerConnectionInfo worker : stage.getContext().getWorkerMap().values()) {
+ if(hosts.contains(worker.getHost())){
+ workerIds.add(worker.getId());
}
}
+ return workerIds;
}
- @Override
- public int remainingScheduledObjectNum() {
- return scheduledObjectNum;
- }
- private class TaskRequests implements EventHandler<TaskRequestEvent> {
- private final LinkedBlockingQueue<TaskRequestEvent> taskRequestQueue =
- new LinkedBlockingQueue<TaskRequestEvent>();
+ protected LinkedList<TaskRequestEvent> createTaskRequest() throws Exception {
+ LinkedList<TaskRequestEvent> taskRequestEvents = new LinkedList<TaskRequestEvent>();
- @Override
- public void handle(TaskRequestEvent event) {
- if(LOG.isDebugEnabled()){
- LOG.debug("TaskRequest: " + event.getContainerId() + "," + event.getExecutionBlockId());
- }
+ int requestContainerNum = Math.max(remainingScheduledObjectNum(), 1);
--- End diff --
Would you give a explanation on the above line?
Even though ```remainingScheduledObjectNum()``` is 0, requestContainerNum is 1. I wonder why at least one container is requested even though there is no remaining scheduled objects.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---