homatthew commented on code in PR #3704:
URL: https://github.com/apache/gobblin/pull/3704#discussion_r1244382872
##########
gobblin-cluster/src/main/java/org/apache/gobblin/cluster/GobblinHelixJobScheduler.java:
##########
@@ -360,7 +400,7 @@ private void waitForJobCompletion(String jobName) {
}
@Subscribe
- public void handleDeleteJobConfigArrival(DeleteJobConfigArrivalEvent
deleteJobArrival) throws InterruptedException {
+ public synchronized void
handleDeleteJobConfigArrival(DeleteJobConfigArrivalEvent deleteJobArrival)
throws InterruptedException {
Review Comment:
1. Since handleDeleteJobConfigArrival is a completely synchronous method,
the synchronized method handleUpdateJobConfigArrival would just hold the lock
while deleting and then proceed to handleNewJobConfigArrival. There would be no
need to distinguish between the two since @Peiyingy needs to address the race
condition described in
https://github.com/apache/gobblin/pull/3704/files#r1243111712 by updating the
map immediately in the newJobConfigArrival method.
2. Yeah we don't call explicitly call delete so it's just semantics about
which is more intuitive behavior if we ever use this in the future. Since this
is purely hypothetical I don't want to waste effort changing the behavior to
(1). I think we should just add a comment describing that deleting a workflow
with throttling enabled means that the next schedulable time for the workflow
will remain unchanged and you have to wait out the throttle timeout before
being able to reschedule
--
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]
