Re: Review Request 65873: Upgrade RBT to 0.7.11
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65873/#review198508 --- Ship it! Ship It! - Stephan Erb On März 2, 2018, 1:14 vorm., Renan DelValle wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/65873/ > --- > > (Updated März 2, 2018, 1:14 vorm.) > > > Review request for Aurora, Jordan Ly and Stephan Erb. > > > Repository: aurora > > > Description > --- > > Upgrade RBT to 0.7.11 > > > Diffs > - > > rbt 5b94e53357e8934df040e603efc9ad2378ff6575 > > > Diff: https://reviews.apache.org/r/65873/diff/1/ > > > Testing > --- > > Ran ./rbt and submitted this review request using it. > > > Thanks, > > Renan DelValle > >
Re: Review Request 65824: Second try at fixing cron id collision bug by avoiding state in Quartz jobs
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65824/#review198503 --- Adding an end to end test would be the way to go since some subtle details (that are difficult to mock) let the previous patch pass without setting off any alarms. Other than that, if I'm understanding correctly what the patch is trying to accomplish, looks like a good solution. - Renan DelValle On Feb. 27, 2018, 6:11 p.m., Jordan Ly wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/65824/ > --- > > (Updated Feb. 27, 2018, 6:11 p.m.) > > > Review request for Aurora, David McLaughlin, Renan DelValle, and Santhosh > Kumar Shanmugham. > > > Repository: aurora > > > Description > --- > > I had to revert the previous fix due to a bug around `JobDataMap` and > manipulating it outside of a Quartz execution. See > https://reviews.apache.org/r/65810/ for context. > > I believe that we can mitigate this by creating an `AtomicBoolean` and > manipulating that within the `batchWorker` execution as opposed to directly > changing the `JobDataMap`. This mirrors the previous behavior of > `killFollowups` but while keeping the state within a given job context. > > > Diffs > - > > src/main/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJob.java > 3604dd4c84043def3fe098ca0f87bf8bab99b3db > > src/test/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJobTest.java > 5083fcd407a069617634b82db1ad799b5d4d4551 > > > Diff: https://reviews.apache.org/r/65824/diff/1/ > > > Testing > --- > > `./gradlew test` > > Manually verified bug in previous patch no longer occurs. Verified the bug > the previous patch was fixing no longer occurs as well. > Will deploy to a test cluster. > > Question for reviewers: Is it worthwhile to add end-to-end tests for > scheduling crons and ensuring KILL_EXISTING works? The reason I am hesitant > is that the coverage of this feature seem comprehensive enough but the bug > that slipped through the previous patch was due to a subtle interaction with > the Quartz scheduler which we mock in unit tests. > > > Thanks, > > Jordan Ly > >
Re: Review Request 65873: Upgrade RBT to 0.7.11
> On March 1, 2018, 5:02 p.m., Jordan Ly wrote: > > Any notable changes? Nothing that immediately stands out other than 2 and half years worth of (seemingly minor) bug fixes. - Renan --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65873/#review198498 --- On March 1, 2018, 4:14 p.m., Renan DelValle wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/65873/ > --- > > (Updated March 1, 2018, 4:14 p.m.) > > > Review request for Aurora, Jordan Ly and Stephan Erb. > > > Repository: aurora > > > Description > --- > > Upgrade RBT to 0.7.11 > > > Diffs > - > > rbt 5b94e53357e8934df040e603efc9ad2378ff6575 > > > Diff: https://reviews.apache.org/r/65873/diff/1/ > > > Testing > --- > > Ran ./rbt and submitted this review request using it. > > > Thanks, > > Renan DelValle > >
Re: Review Request 65873: Upgrade RBT to 0.7.11
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65873/#review198498 --- Ship it! Any notable changes? - Jordan Ly On March 2, 2018, 12:14 a.m., Renan DelValle wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/65873/ > --- > > (Updated March 2, 2018, 12:14 a.m.) > > > Review request for Aurora, Jordan Ly and Stephan Erb. > > > Repository: aurora > > > Description > --- > > Upgrade RBT to 0.7.11 > > > Diffs > - > > rbt 5b94e53357e8934df040e603efc9ad2378ff6575 > > > Diff: https://reviews.apache.org/r/65873/diff/1/ > > > Testing > --- > > Ran ./rbt and submitted this review request using it. > > > Thanks, > > Renan DelValle > >
Re: Review Request 65873: Upgrade RBT to 0.7.11
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65873/#review198495 --- Ship it! Master (a12b844) is green with this patch. ./build-support/jenkins/build.sh I will refresh this build result if you post a review containing "@ReviewBot retry" - Aurora ReviewBot On March 2, 2018, 12:14 a.m., Renan DelValle wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/65873/ > --- > > (Updated March 2, 2018, 12:14 a.m.) > > > Review request for Aurora, Jordan Ly and Stephan Erb. > > > Repository: aurora > > > Description > --- > > Upgrade RBT to 0.7.11 > > > Diffs > - > > rbt 5b94e53357e8934df040e603efc9ad2378ff6575 > > > Diff: https://reviews.apache.org/r/65873/diff/1/ > > > Testing > --- > > Ran ./rbt and submitted this review request using it. > > > Thanks, > > Renan DelValle > >
Review Request 65873: Upgrade RBT to 0.7.11
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65873/ --- Review request for Aurora, Jordan Ly and Stephan Erb. Repository: aurora Description --- Upgrade RBT to 0.7.11 Diffs - rbt 5b94e53357e8934df040e603efc9ad2378ff6575 Diff: https://reviews.apache.org/r/65873/diff/1/ Testing --- Ran ./rbt and submitted this review request using it. Thanks, Renan DelValle
