-
Notifications
You must be signed in to change notification settings - Fork 751
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[GOBBLIN-1907] Handle Lease Completion From Other Multi-active Participants #3771
Conversation
gobblin-runtime/src/main/java/org/apache/gobblin/runtime/api/MysqlMultiActiveLeaseArbiter.java
Outdated
Show resolved
Hide resolved
@@ -17,8 +17,10 @@ | |||
|
|||
package org.apache.gobblin.runtime.api; | |||
|
|||
import com.google.common.base.Optional; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's use java Optional
(seems the original class under test mistakenly used the deprecated guava one... I presume it's not too difficult at this early stage to correct)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I replaced the guava optional with the Java one in the MysqlMultiActiveLeaseArbiter[Test]
but am leaving the other related classes as is for now. Will fix going forward.
...e/src/main/java/org/apache/gobblin/service/modules/scheduler/GobblinServiceJobScheduler.java
Outdated
Show resolved
Hide resolved
Codecov Report
@@ Coverage Diff @@
## master #3771 +/- ##
============================================
- Coverage 49.74% 47.31% -2.43%
- Complexity 9438 10955 +1517
============================================
Files 1767 2152 +385
Lines 69294 85105 +15811
Branches 7888 9451 +1563
============================================
+ Hits 34471 40271 +5800
- Misses 31636 41186 +9550
- Partials 3187 3648 +461
... and 391 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
Dear Gobblin maintainers,
Please accept this PR. I understand that it will not be reviewed until I have checked off all the steps below!
JIRA
Description
When other participants complete a lease (not just acquire it) in the time that the current participant has attempted the lease, we will see a NULL
lease_acquisition_timestamp
and should appropriately handle the resulting value. This case was missed and we were only expecting another participant to have acquired the lease.We also add warning logs for
JobExceptions
that can be hiding the stack trace of errors encountered in runtime.Tests
Tested behavior without change to see NPE and after updates the tests pass without any exceptions
Commits