yuqi1129 commented on code in PR #7968:
URL: https://github.com/apache/gravitino/pull/7968#discussion_r2262192349
##########
core/src/main/java/org/apache/gravitino/job/local/LocalJobExecutor.java:
##########
@@ -255,15 +255,18 @@ public void runJob(Pair<String, JobTemplate> jobPair) {
try {
String jobId = jobPair.getLeft();
JobTemplate jobTemplate = jobPair.getRight();
+
+ Process process;
synchronized (lock) {
jobStatus.put(jobId, Pair.of(JobHandle.Status.STARTED,
UNEXPIRED_TIME_IN_MS));
- }
- LocalProcessBuilder processBuilder =
LocalProcessBuilder.create(jobTemplate, configs);
- Process process = processBuilder.start();
- runningProcesses.put(jobId, process);
+ LocalProcessBuilder processBuilder =
LocalProcessBuilder.create(jobTemplate, configs);
+ process = processBuilder.start();
+ runningProcesses.put(jobId, process);
Review Comment:
Then there is no need to put the process start logic into the lock as you
have done it in `submitJob`. The current main logic can guarantee the status
will be changed `RUNNING` before processes is started.
##########
core/src/main/java/org/apache/gravitino/job/local/LocalJobExecutor.java:
##########
@@ -156,12 +156,12 @@ public String submitJob(JobTemplate jobTemplate) {
String newJobId = LOCAL_JOB_PREFIX + UUID.randomUUID();
Pair<String, JobTemplate> jobPair = Pair.of(newJobId, jobTemplate);
- // Add the job template to the waiting queue
- if (!waitingQueue.offer(jobPair)) {
- throw new IllegalStateException("Waiting queue is full, cannot submit
job: " + jobTemplate);
- }
-
synchronized (lock) {
Review Comment:
I have not problem about this point.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
