jeongyooneo commented on a change in pull request #21: [NEMO-46] Make the 
operations on ExecutorRegistry atomic
URL: https://github.com/apache/incubator-nemo/pull/21#discussion_r189782160
 
 

 ##########
 File path: 
runtime/master/src/main/java/edu/snu/nemo/runtime/master/scheduler/SchedulerRunner.java
 ##########
 @@ -83,32 +83,76 @@ public void onATaskAvailable() {
     mustCheckSchedulingAvailabilityOrSchedulerTerminated.signal();
   }
 
+  /**
+   * Run the scheduler thread.
+   */
+  void runSchedulerThread() {
+    if (!isTerminated) {
+      if (!isSchedulerRunning.getAndSet(true)) {
+        schedulerThread.execute(new SchedulerThread());
+        schedulerThread.shutdown();
+      }
+    }
+  }
+
   /**
    * Begin scheduling a job.
    * @param jobStateManager the corresponding {@link JobStateManager}
    */
   void scheduleJob(final JobStateManager jobStateManager) {
     if (!isTerminated) {
       jobStateManagers.put(jobStateManager.getJobId(), jobStateManager);
-
-      if (!initialJobScheduled) {
-        initialJobScheduled = true;
-        schedulerThread.execute(new SchedulerThread());
-        schedulerThread.shutdown();
-      }
     } // else ignore new incoming jobs when terminated.
   }
 
   void terminate() {
-    for (final String executorId : executorRegistry.getRunningExecutorIds()) {
-      final ExecutorRepresenter representer = 
executorRegistry.getRunningExecutorRepresenter(executorId);
-      representer.shutDown();
-      executorRegistry.setRepresenterAsCompleted(executorId);
-    }
     isTerminated = true;
     mustCheckSchedulingAvailabilityOrSchedulerTerminated.signal();
   }
 
+  void doScheduleTask() {
+    final Collection<ScheduledTask> schedulableTasks = 
pendingTaskCollection.peekSchedulableTasks().orElse(null);
+    if (schedulableTasks == null) {
+      // Task queue is empty
+      LOG.debug("PendingTaskCollection is empty. Awaiting for more Tasks...");
+      return;
+    }
+
+    final AtomicInteger numScheduledTasks = new AtomicInteger(0); // to be 
incremented in lambda
+    for (final ScheduledTask schedulableTask : schedulableTasks) {
+      final JobStateManager jobStateManager = 
jobStateManagers.get(schedulableTask.getJobId());
+      LOG.debug("Trying to schedule {}...", schedulableTask.getTaskId());
+
+      executorRegistry.viewExecutors(executors -> {
+        final Set<ExecutorRepresenter> candidateExecutors =
+            schedulingPolicy.filterExecutorRepresenters(executors, 
schedulableTask);
+        final Optional<ExecutorRepresenter> firstCandidate = 
candidateExecutors.stream().findFirst();
+
+        if (firstCandidate.isPresent()) {
+          // update metadata first
+          jobStateManager.onTaskStateChanged(schedulableTask.getTaskId(), 
TaskState.State.EXECUTING);
+          pendingTaskCollection.remove(schedulableTask.getTaskId());
+          numScheduledTasks.incrementAndGet();
+
+          // send the task
+          final ExecutorRepresenter selectedExecutor = firstCandidate.get();
+          selectedExecutor.onTaskScheduled(schedulableTask);
+          LOG.debug("Successfully scheduled {}", schedulableTask.getTaskId());
+        } else {
+          LOG.debug("Failed to schedule {}", schedulableTask.getTaskId());
+        }
+      });
+    }
+
+    LOG.debug("Examined {} Tasks, scheduled {} Tasks", 
schedulableTasks.size(), numScheduledTasks);
+    if (schedulableTasks.size() == numScheduledTasks.get()) {
 
 Review comment:
   Could you elaborate more on this in comment, with the meaning 
of`mustCheckSchedulingAvailabilityOrSchedulerTerminated`? 
   
   If `schedulableTasks.size() == numScheduledTasks.get()` denotes `// 
Scheduled all Tasks in the stage`, then is it correct to understand that a 
single run of `doScheduleTask` denotes running a single stage?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[email protected]


With regards,
Apache Git Services

Reply via email to