----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65810/#review198337 -----------------------------------------------------------
Ship it! Ship It! - David McLaughlin On Feb. 27, 2018, 5:58 a.m., Jordan Ly wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/65810/ > ----------------------------------------------------------- > > (Updated Feb. 27, 2018, 5:58 a.m.) > > > Review request for Aurora, David McLaughlin and Santhosh Kumar Shanmugham. > > > Repository: aurora > > > Description > ------- > > This reverts commit e2ea191473397691605602c6e40c6aad8a56d81a. > > A bug was found where jobs that were killed via the KILL_EXISTING flag would > set `path` as `null` in `JobDataMap` that would block concurrent runs, but > that value would never be set to `key` after the the delayed run finished > because it would run outside of the `Job` execution. > > The issue in https://reviews.apache.org/r/65680/ will occur again, but it is > rare and has been around for a few years. > > This bug was not caught in the unit test `testKillExisting` because > `executeWithReplay` is mocked and runs synchronously within the `Job` > execution, allowing the `key` to be persisted. > > > Diffs > ----- > > src/main/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJob.java > b77e032a245396143f103da1c0e5c9d508fe8098 > > src/test/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJobTest.java > 8ae9bb56708f484bd32580efd74a425973b47093 > > > Diff: https://reviews.apache.org/r/65810/diff/1/ > > > Testing > ------- > > `./gradlew test` > > Manually verified in Vagrant environment that the bug no longer occurs. > > > Thanks, > > Jordan Ly > >