Yingyi Bu has posted comments on this change. Change subject: Implements concurrent query management support. ......................................................................
Patch Set 23: (56 comments) >>1) Using "capacity" instead of "resource" in some class (and variable) names. Done. >> 2) It seems a bit unintuitive to have the controller of whether a a cluster >> can accept a job with the job and not with the resource manager. It would be >> nice to change this if it does make important functionality more >> complicated. Done >> 3) The result of JobManager.get seems to return the same value as before in >> some cases, but it others the behavior is changed. Need to validate if that >> is correct. This is not an issue as job id is unique, which is guaranteed by JobIdFactory. To have a single get(...) method makes the interface easier. https://asterix-gerrit.ics.uci.edu/#/c/1424/23/asterixdb/asterix-app/src/main/java/org/apache/asterix/api/common/APIFramework.java File asterixdb/asterix-app/src/main/java/org/apache/asterix/api/common/APIFramework.java: Line 317: spec.setJobResourceController( > Would it be feasible to just give the requires resources (or a resource pro Done https://asterix-gerrit.ics.uci.edu/#/c/1424/23/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/resource/ComputationResourceVisitor.java File asterixdb/asterix-app/src/main/java/org/apache/asterix/app/resource/ComputationResourceVisitor.java: Line 71: public class ComputationResourceVisitor implements ILogicalOperatorVisitor<Void, Void> { > RequiredCapacityVisitor? Done https://asterix-gerrit.ics.uci.edu/#/c/1424/23/asterixdb/asterix-app/src/main/java/org/apache/asterix/hyracks/bootstrap/NCApplicationEntryPoint.java File asterixdb/asterix-app/src/main/java/org/apache/asterix/hyracks/bootstrap/NCApplicationEntryPoint.java: Line 267: public NodeResource getAvailableResource() { > getCapacity? Done Line 272: int allCores = Runtime.getRuntime().availableProcessors(); > create variables for the 2 numbers and pass those to the constructor? Shoul Done https://asterix-gerrit.ics.uci.edu/#/c/1424/23/asterixdb/asterix-app/src/test/java/org/apache/asterix/app/resource/ComputationResourceVisitorTest.java File asterixdb/asterix-app/src/test/java/org/apache/asterix/app/resource/ComputationResourceVisitorTest.java: Line 43: public class ComputationResourceVisitorTest { > RequiredCapacityVisitorTest? Done https://asterix-gerrit.ics.uci.edu/#/c/1424/23/asterixdb/asterix-installer/src/test/resources/integrationts/asterix-configuration.xml File asterixdb/asterix-installer/src/test/resources/integrationts/asterix-configuration.xml: Line 221: <value>8MB</value> > Wasn't this pushed up just recently? Done https://asterix-gerrit.ics.uci.edu/#/c/1424/23/hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/application/INCApplicationEntryPoint.java File hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/application/INCApplicationEntryPoint.java: Line 30: NodeResource getAvailableResource(); > getCapacity()? Done https://asterix-gerrit.ics.uci.edu/#/c/1424/23/hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/exceptions/ErrorCode.java File hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/exceptions/ErrorCode.java: Line 46: public static final int JOB_RESOURCE_BEYOND_MAXIMUM = 9; > JOB_REQUIREMENTS_EXCEED_CAPACITY Done Line 47: public static final int NODE_NOT_EXIST = 10; > NO_SUCH_NODE Done https://asterix-gerrit.ics.uci.edu/#/c/1424/23/hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/job/JobSpecification.java File hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/job/JobSpecification.java: Line 82: private IJobResourceController jobResourceConstroller; > As mentioned elsewhere: could this be just the require capacity without the Done https://asterix-gerrit.ics.uci.edu/#/c/1424/23/hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/job/resource/ComputationResource.java File hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/job/resource/ComputationResource.java: Line 29: public class ComputationResource implements IComputationResource { > ClusterCapacity? Done Line 84: if (nodeResource == null) { > Is there ever a valid reason why this should be null? Done https://asterix-gerrit.ics.uci.edu/#/c/1424/23/hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/job/resource/IComputationResource.java File hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/job/resource/IComputationResource.java: Line 27: public interface IComputationResource extends IReadOnlyComputationResource { > I think that it would be good to call these IClusterCapacity and IReadOnlyC Done Line 43: void setAggregatedAvailableCores(int aggregatedAvailableCores); > If we move to the "capacity" naming scheme, I think that we don't need "ava Done https://asterix-gerrit.ics.uci.edu/#/c/1424/23/hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/job/resource/IJobResourceController.java File hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/job/resource/IJobResourceController.java: Line 31: */ > I'm not sure I understand the interfaces here. It seems that a resource con Done https://asterix-gerrit.ics.uci.edu/#/c/1424/23/hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/job/resource/IReadOnlyComputationResource.java File hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/job/resource/IReadOnlyComputationResource.java: Line 27: public interface IReadOnlyComputationResource { > As mentioned before, I think that IReadOnlyClusterCapacity would be better. Done Line 37: int getAggregatedAvailableCores(); > If we move to the "capacity" naming scheme, I think that we don't need "ava Done https://asterix-gerrit.ics.uci.edu/#/c/1424/23/hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/job/resource/NodeResource.java File hyracks-fullstack/hyracks/hyracks-api/src/main/java/org/apache/hyracks/api/job/resource/NodeResource.java: Line 27: public class NodeResource implements Serializable { > NodeCapacity? Done Line 30: private final long availableMemoryByteSize; > memoryByteSize? Done Line 34: private final int availableCores; > cores? Done Line 36: public NodeResource(long memorySize, int availableCores) { > What does it mean, if these parameters are negative? Done https://asterix-gerrit.ics.uci.edu/#/c/1424/23/hyracks-fullstack/hyracks/hyracks-api/src/main/resources/errormsg/en.properties File hyracks-fullstack/hyracks/hyracks-api/src/main/resources/errormsg/en.properties: Line 30: 9=Job requires %1$s, which is more than the maximum available resource %2$s > Job requirement %1$s exceeds capacity %2$s Done https://asterix-gerrit.ics.uci.edu/#/c/1424/23/hyracks-fullstack/hyracks/hyracks-control/hyracks-control-cc/src/main/java/org/apache/hyracks/control/cc/NodeControllerState.java File hyracks-fullstack/hyracks/hyracks-control/hyracks-control-cc/src/main/java/org/apache/hyracks/control/cc/NodeControllerState.java: Line 145: private NodeResource resource; > s/resource/capacity/ Done Line 208: resource = reg.getResource(); > s/resource/capacity/ Done Line 277: public NodeResource getResource() { > s/resource/capacity/ Done https://asterix-gerrit.ics.uci.edu/#/c/1424/23/hyracks-fullstack/hyracks/hyracks-control/hyracks-control-cc/src/main/java/org/apache/hyracks/control/cc/cluster/INodeManager.java File hyracks-fullstack/hyracks/hyracks-control/hyracks-control-cc/src/main/java/org/apache/hyracks/control/cc/cluster/INodeManager.java: Line 43: > remove empty line? Done Line 94: void addNode(String nodeId, NodeControllerState ncState) throws HyracksException; > cluster the methods by "applied to one node" and "applies to all nodes"? Done https://asterix-gerrit.ics.uci.edu/#/c/1424/23/hyracks-fullstack/hyracks/hyracks-control/hyracks-control-cc/src/main/java/org/apache/hyracks/control/cc/cluster/NodeManager.java File hyracks-fullstack/hyracks/hyracks-control/hyracks-control-cc/src/main/java/org/apache/hyracks/control/cc/cluster/NodeManager.java: Line 62: return new HashMap<>(ipAddressNodeNameMap); > Should we use Collections.unmodifiableMap() here to document the intent? Done Line 67: return new TreeSet<>(nodeRegistry.keySet()); > Should we use Collections.unmodifiableMap() here to document the intent? Done Line 72: return new HashSet<>(nodeRegistry.values()); > Should we use Collections.unmodifiableMap() here to document the intent? Done https://asterix-gerrit.ics.uci.edu/#/c/1424/23/hyracks-fullstack/hyracks/hyracks-control/hyracks-control-cc/src/main/java/org/apache/hyracks/control/cc/executor/ActivityClusterPlanner.java File hyracks-fullstack/hyracks/hyracks-control/hyracks-control-cc/src/main/java/org/apache/hyracks/control/cc/executor/ActivityClusterPlanner.java: Line 58: private final JobExecutor scheduler; > rename variable? Done Line 62: ActivityClusterPlanner(JobExecutor newJobScheduler) { > rename variable? Done https://asterix-gerrit.ics.uci.edu/#/c/1424/23/hyracks-fullstack/hyracks/hyracks-control/hyracks-control-cc/src/main/java/org/apache/hyracks/control/cc/job/IJobManager.java File hyracks-fullstack/hyracks/hyracks-control/hyracks-control-cc/src/main/java/org/apache/hyracks/control/cc/job/IJobManager.java: Line 48: * This method gets called when all slave processes have notified the master that their individual parallel > Can we call them "worker" instead of "slave"? Done Line 63: * each individual parallel partition. > s/method should dictated all involved slave processes to cleanup the states Done Line 91: Collection<JobRun> getAllRunningJobs(); > remove "All" from the 3 method names? Done Line 110: List<Exception> getRunHistory(JobId jobId); > Move this up to the "single job" methods? Done https://asterix-gerrit.ics.uci.edu/#/c/1424/23/hyracks-fullstack/hyracks/hyracks-control/hyracks-control-cc/src/main/java/org/apache/hyracks/control/cc/job/JobManager.java File hyracks-fullstack/hyracks/hyracks-control/hyracks-control-cc/src/main/java/org/apache/hyracks/control/cc/job/JobManager.java: Line 77: throw new HyracksException(e); > Error code? Done https://asterix-gerrit.ics.uci.edu/#/c/1424/23/hyracks-fullstack/hyracks/hyracks-control/hyracks-control-cc/src/main/java/org/apache/hyracks/control/cc/scheduler/IJobQueue.java File hyracks-fullstack/hyracks/hyracks-control/hyracks-control-cc/src/main/java/org/apache/hyracks/control/cc/scheduler/IJobQueue.java: Line 54: List<JobRun> remove(IReadOnlyComputationResource maxResource, IComputationResource availableResource); > This interface is also a little confusing. We add a JobRun to the queue, bu Done https://asterix-gerrit.ics.uci.edu/#/c/1424/23/hyracks-fullstack/hyracks/hyracks-control/hyracks-control-cc/src/main/java/org/apache/hyracks/control/cc/scheduler/IResourceManager.java File hyracks-fullstack/hyracks/hyracks-control/hyracks-control-cc/src/main/java/org/apache/hyracks/control/cc/scheduler/IResourceManager.java: Line 36: IReadOnlyComputationResource getMaximumResource(); > s/getMaximumResource/getMaximumCapacity/ Done Line 41: IComputationResource getCurrentResource(); > s/getCurrentResource/getCurrentCapacity/ Done https://asterix-gerrit.ics.uci.edu/#/c/1424/23/hyracks-fullstack/hyracks/hyracks-control/hyracks-control-cc/src/main/java/org/apache/hyracks/control/cc/scheduler/ResourceManager.java File hyracks-fullstack/hyracks/hyracks-control/hyracks-control-cc/src/main/java/org/apache/hyracks/control/cc/scheduler/ResourceManager.java: Line 32: private IComputationResource maxResource = new ComputationResource(); > s/Resource/Capacity/ Done Line 35: private IComputationResource currentResource = new ComputationResource(); > s/Resource/Capacity/ Done Line 38: public IReadOnlyComputationResource getMaximumResource() { > s/Resource/Capacity/ Done Line 43: public IComputationResource getCurrentResource() { > s/Resource/Capacity/ Done https://asterix-gerrit.ics.uci.edu/#/c/1424/23/hyracks-fullstack/hyracks/hyracks-control/hyracks-control-cc/src/main/java/org/apache/hyracks/control/cc/work/AbstractTaskLifecycleWork.java File hyracks-fullstack/hyracks/hyracks-control/hyracks-control-cc/src/main/java/org/apache/hyracks/control/cc/work/AbstractTaskLifecycleWork.java: Line 55: JobRun run = jobManager.get(jobId); > this might return an archived job Not an issue? https://asterix-gerrit.ics.uci.edu/#/c/1424/23/hyracks-fullstack/hyracks/hyracks-control/hyracks-control-cc/src/main/java/org/apache/hyracks/control/cc/work/GetActivityClusterGraphJSONWork.java File hyracks-fullstack/hyracks/hyracks-control/hyracks-control-cc/src/main/java/org/apache/hyracks/control/cc/work/GetActivityClusterGraphJSONWork.java: Line 44: JobRun run = jobManager.get(jobId); > this might return an archived job The original code returns either a running or archived job. https://asterix-gerrit.ics.uci.edu/#/c/1424/23/hyracks-fullstack/hyracks/hyracks-control/hyracks-control-cc/src/main/java/org/apache/hyracks/control/cc/work/GetJobRunJSONWork.java File hyracks-fullstack/hyracks/hyracks-control/hyracks-control-cc/src/main/java/org/apache/hyracks/control/cc/work/GetJobRunJSONWork.java: Line 42: JobRun run = jobManager.get(jobId); > this might return an archived job The original code returns either a running job or an archived job. https://asterix-gerrit.ics.uci.edu/#/c/1424/23/hyracks-fullstack/hyracks/hyracks-control/hyracks-control-cc/src/main/java/org/apache/hyracks/control/cc/work/JobCleanupWork.java File hyracks-fullstack/hyracks/hyracks-control/hyracks-control-cc/src/main/java/org/apache/hyracks/control/cc/work/JobCleanupWork.java: Line 57: JobRun run = jobManager.get(jobId); > this might return an archived job Not an issue? https://asterix-gerrit.ics.uci.edu/#/c/1424/23/hyracks-fullstack/hyracks/hyracks-control/hyracks-control-cc/src/main/java/org/apache/hyracks/control/cc/work/JobletCleanupNotificationWork.java File hyracks-fullstack/hyracks/hyracks-control/hyracks-control-cc/src/main/java/org/apache/hyracks/control/cc/work/JobletCleanupNotificationWork.java: Line 51: final JobRun run = jobManager.get(jobId); > this might return an archived job Not an issue? https://asterix-gerrit.ics.uci.edu/#/c/1424/23/hyracks-fullstack/hyracks/hyracks-control/hyracks-control-cc/src/main/java/org/apache/hyracks/control/cc/work/RegisterPartitionAvailibilityWork.java File hyracks-fullstack/hyracks/hyracks-control/hyracks-control-cc/src/main/java/org/apache/hyracks/control/cc/work/RegisterPartitionAvailibilityWork.java: Line 47: JobRun run = jobManager.get(pid.getJobId()); > this might return an archived job Not an issue? https://asterix-gerrit.ics.uci.edu/#/c/1424/23/hyracks-fullstack/hyracks/hyracks-control/hyracks-control-cc/src/main/java/org/apache/hyracks/control/cc/work/RegisterPartitionRequestWork.java File hyracks-fullstack/hyracks/hyracks-control/hyracks-control-cc/src/main/java/org/apache/hyracks/control/cc/work/RegisterPartitionRequestWork.java: Line 45: JobRun run = jobManager.get(pid.getJobId()); > this might return an archived job Not an issue? https://asterix-gerrit.ics.uci.edu/#/c/1424/23/hyracks-fullstack/hyracks/hyracks-control/hyracks-control-cc/src/main/java/org/apache/hyracks/control/cc/work/ReportProfilesWork.java File hyracks-fullstack/hyracks/hyracks-control/hyracks-control-cc/src/main/java/org/apache/hyracks/control/cc/work/ReportProfilesWork.java: Line 42: JobRun run = jobManager.get(profile.getJobId()); > this might also return an archived job Not an issue? https://asterix-gerrit.ics.uci.edu/#/c/1424/23/hyracks-fullstack/hyracks/hyracks-control/hyracks-control-cc/src/main/java/org/apache/hyracks/control/cc/work/TaskCompleteWork.java File hyracks-fullstack/hyracks/hyracks-control/hyracks-control-cc/src/main/java/org/apache/hyracks/control/cc/work/TaskCompleteWork.java: Line 47: JobRun run = jobManager.get(jobId); > this might also return an archived job Not an issue? https://asterix-gerrit.ics.uci.edu/#/c/1424/23/hyracks-fullstack/hyracks/hyracks-control/hyracks-control-cc/src/main/java/org/apache/hyracks/control/cc/work/TaskFailureWork.java File hyracks-fullstack/hyracks/hyracks-control/hyracks-control-cc/src/main/java/org/apache/hyracks/control/cc/work/TaskFailureWork.java: Line 42: JobRun run = jobManager.get(jobId); > This might also return an archived job. Not an issue? https://asterix-gerrit.ics.uci.edu/#/c/1424/23/hyracks-fullstack/hyracks/hyracks-control/hyracks-control-cc/src/main/java/org/apache/hyracks/control/cc/work/WaitForJobCompletionWork.java File hyracks-fullstack/hyracks/hyracks-control/hyracks-control-cc/src/main/java/org/apache/hyracks/control/cc/work/WaitForJobCompletionWork.java: Line 58: final IJobStatusConditionVariable cArchivedVar = jobManager.get(jobId); > This seems wrong. If the first call to jobManager.get(jobId) returned null, Done https://asterix-gerrit.ics.uci.edu/#/c/1424/23/hyracks-fullstack/hyracks/hyracks-examples/btree-example/btreehelper/src/main/java/org/apache/hyracks/examples/btree/helper/NCApplicationEntryPoint.java File hyracks-fullstack/hyracks/hyracks-examples/btree-example/btreehelper/src/main/java/org/apache/hyracks/examples/btree/helper/NCApplicationEntryPoint.java: Line 44: public NodeResource getAvailableResource() { > getCapacity? Done -- To view, visit https://asterix-gerrit.ics.uci.edu/1424 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8fb6fda57efa139114dd234e08cc7de7129468c8 Gerrit-PatchSet: 23 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: Yingyi Bu <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Michael Blow <[email protected]> Gerrit-Reviewer: Till Westmann <[email protected]> Gerrit-Reviewer: Yingyi Bu <[email protected]> Gerrit-Reviewer: abdullah alamoudi <[email protected]> Gerrit-HasComments: Yes
