You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@aurora.apache.org by Bill Farner <wf...@apache.org> on 2015/09/13 19:02:39 UTC

Review Request 38336: Make async work queue gating thread-local.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38336/
-----------------------------------------------------------

Review request for Aurora and Maxim Khutornenko.


Bugs: AURORA-1459
    https://issues.apache.org/jira/browse/AURORA-1459


Repository: aurora


Description
-------

This effectively makes `GatedDelayExecutor` (renamed to `GatingDelayExecutor`) transaction-aware, and avoids unnecessarily delaying independent async work that is not part of a transaction.


Diffs
-----

  config/findbugs/excludeFilter.xml 7c653022fdc1a33e71da08c2adeaa5760153a08b 
  src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 217b9c0347b259e76180a7021c9c0f9c0c34c89c 
  src/main/java/org/apache/aurora/scheduler/async/FlushableWorkQueue.java 11a1c2a7a920eefeb92cbf230866ed107941f642 
  src/main/java/org/apache/aurora/scheduler/async/GatedDelayExecutor.java 9d4cfcf1f6b9da4261e2b0252d39aeeb1b4427bb 
  src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 6da6193c47580b3e4446371f0df798eccc24f809 
  src/main/java/org/apache/aurora/scheduler/storage/db/DbStorage.java 60365709ced5314e3563ab5d75d7a0fffda9ac91 
  src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 49411289299bfc230e8c6e0ce6fa96e619f57524 
  src/test/java/org/apache/aurora/scheduler/async/GatedDelayExecutorTest.java 28676332f74c6a4cffeb867ef5612e12d629b5b8 
  src/test/java/org/apache/aurora/scheduler/async/GatingDelayExecutorTest.java PRE-CREATION 
  src/test/java/org/apache/aurora/scheduler/pruning/TaskHistoryPrunerTest.java 0c7da074515e1fad534617f9b8f73d939caf948b 
  src/test/java/org/apache/aurora/scheduler/storage/db/DbStorageTest.java 6dd50264b7d119aa5b5f96ab153794e95c443822 

Diff: https://reviews.apache.org/r/38336/diff/


Testing
-------


Thanks,

Bill Farner

Re: Review Request 38336: Make async work queue gating thread-local.

Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38336/#review98772
-----------------------------------------------------------


Reviewer notes.


src/main/java/org/apache/aurora/scheduler/async/GatedDelayExecutor.java (line 34)
<https://reviews.apache.org/r/38336/#comment155381>

    I renamed this because i believe it's more accurate - this class is doing the _gating_, it is not _gated_.



src/main/java/org/apache/aurora/scheduler/storage/db/DbStorage.java 
<https://reviews.apache.org/r/38336/#comment155383>

    This is no longer necessary in DbStorage, as it's handled by `GatingDelayExecutor`.



src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
<https://reviews.apache.org/r/38336/#comment155384>

    Per the litmus test mentioned in the ticket - this is no longer necessary since the work queue will let this pass through.  This also makes `./gradlew run` work normally again.



src/test/java/org/apache/aurora/scheduler/pruning/TaskHistoryPrunerTest.java (line 257)
<https://reviews.apache.org/r/38336/#comment155385>

    Not strictly necessary in this patch, but i noticed this thread pool was not cleaned up.


- Bill Farner


On Sept. 13, 2015, 10:05 a.m., Bill Farner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38336/
> -----------------------------------------------------------
> 
> (Updated Sept. 13, 2015, 10:05 a.m.)
> 
> 
> Review request for Aurora and Maxim Khutornenko.
> 
> 
> Bugs: AURORA-1459
>     https://issues.apache.org/jira/browse/AURORA-1459
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> This effectively makes `GatedDelayExecutor` (renamed to `GatingDelayExecutor`) transaction-aware, and avoids unnecessarily delaying independent async work that is not part of a transaction.
> 
> 
> Diffs
> -----
> 
>   config/findbugs/excludeFilter.xml 7c653022fdc1a33e71da08c2adeaa5760153a08b 
>   src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 217b9c0347b259e76180a7021c9c0f9c0c34c89c 
>   src/main/java/org/apache/aurora/scheduler/async/FlushableWorkQueue.java 11a1c2a7a920eefeb92cbf230866ed107941f642 
>   src/main/java/org/apache/aurora/scheduler/async/GatedDelayExecutor.java 9d4cfcf1f6b9da4261e2b0252d39aeeb1b4427bb 
>   src/main/java/org/apache/aurora/scheduler/async/GatedWorkQueue.java PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 6da6193c47580b3e4446371f0df798eccc24f809 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbStorage.java 60365709ced5314e3563ab5d75d7a0fffda9ac91 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 49411289299bfc230e8c6e0ce6fa96e619f57524 
>   src/test/java/org/apache/aurora/scheduler/async/GatedDelayExecutorTest.java 28676332f74c6a4cffeb867ef5612e12d629b5b8 
>   src/test/java/org/apache/aurora/scheduler/async/GatingDelayExecutorTest.java PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/pruning/TaskHistoryPrunerTest.java 0c7da074515e1fad534617f9b8f73d939caf948b 
>   src/test/java/org/apache/aurora/scheduler/storage/db/DbStorageTest.java 6dd50264b7d119aa5b5f96ab153794e95c443822 
> 
> Diff: https://reviews.apache.org/r/38336/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 38336: Make async work queue gating thread-local.

Posted by Maxim Khutornenko <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38336/#review98920
-----------------------------------------------------------

Ship it!


Ship It!

- Maxim Khutornenko


On Sept. 13, 2015, 5:05 p.m., Bill Farner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38336/
> -----------------------------------------------------------
> 
> (Updated Sept. 13, 2015, 5:05 p.m.)
> 
> 
> Review request for Aurora and Maxim Khutornenko.
> 
> 
> Bugs: AURORA-1459
>     https://issues.apache.org/jira/browse/AURORA-1459
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> This effectively makes `GatedDelayExecutor` (renamed to `GatingDelayExecutor`) transaction-aware, and avoids unnecessarily delaying independent async work that is not part of a transaction.
> 
> 
> Diffs
> -----
> 
>   config/findbugs/excludeFilter.xml 7c653022fdc1a33e71da08c2adeaa5760153a08b 
>   src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 217b9c0347b259e76180a7021c9c0f9c0c34c89c 
>   src/main/java/org/apache/aurora/scheduler/async/FlushableWorkQueue.java 11a1c2a7a920eefeb92cbf230866ed107941f642 
>   src/main/java/org/apache/aurora/scheduler/async/GatedDelayExecutor.java 9d4cfcf1f6b9da4261e2b0252d39aeeb1b4427bb 
>   src/main/java/org/apache/aurora/scheduler/async/GatedWorkQueue.java PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 6da6193c47580b3e4446371f0df798eccc24f809 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbStorage.java 60365709ced5314e3563ab5d75d7a0fffda9ac91 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 49411289299bfc230e8c6e0ce6fa96e619f57524 
>   src/test/java/org/apache/aurora/scheduler/async/GatedDelayExecutorTest.java 28676332f74c6a4cffeb867ef5612e12d629b5b8 
>   src/test/java/org/apache/aurora/scheduler/async/GatingDelayExecutorTest.java PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/pruning/TaskHistoryPrunerTest.java 0c7da074515e1fad534617f9b8f73d939caf948b 
>   src/test/java/org/apache/aurora/scheduler/storage/db/DbStorageTest.java 6dd50264b7d119aa5b5f96ab153794e95c443822 
> 
> Diff: https://reviews.apache.org/r/38336/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 38336: Make async work queue gating thread-local.

Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38336/#review98774
-----------------------------------------------------------

Ship it!


Master (5dccf92) 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 Sept. 13, 2015, 5:05 p.m., Bill Farner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38336/
> -----------------------------------------------------------
> 
> (Updated Sept. 13, 2015, 5:05 p.m.)
> 
> 
> Review request for Aurora and Maxim Khutornenko.
> 
> 
> Bugs: AURORA-1459
>     https://issues.apache.org/jira/browse/AURORA-1459
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> This effectively makes `GatedDelayExecutor` (renamed to `GatingDelayExecutor`) transaction-aware, and avoids unnecessarily delaying independent async work that is not part of a transaction.
> 
> 
> Diffs
> -----
> 
>   config/findbugs/excludeFilter.xml 7c653022fdc1a33e71da08c2adeaa5760153a08b 
>   src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 217b9c0347b259e76180a7021c9c0f9c0c34c89c 
>   src/main/java/org/apache/aurora/scheduler/async/FlushableWorkQueue.java 11a1c2a7a920eefeb92cbf230866ed107941f642 
>   src/main/java/org/apache/aurora/scheduler/async/GatedDelayExecutor.java 9d4cfcf1f6b9da4261e2b0252d39aeeb1b4427bb 
>   src/main/java/org/apache/aurora/scheduler/async/GatedWorkQueue.java PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 6da6193c47580b3e4446371f0df798eccc24f809 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbStorage.java 60365709ced5314e3563ab5d75d7a0fffda9ac91 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 49411289299bfc230e8c6e0ce6fa96e619f57524 
>   src/test/java/org/apache/aurora/scheduler/async/GatedDelayExecutorTest.java 28676332f74c6a4cffeb867ef5612e12d629b5b8 
>   src/test/java/org/apache/aurora/scheduler/async/GatingDelayExecutorTest.java PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/pruning/TaskHistoryPrunerTest.java 0c7da074515e1fad534617f9b8f73d939caf948b 
>   src/test/java/org/apache/aurora/scheduler/storage/db/DbStorageTest.java 6dd50264b7d119aa5b5f96ab153794e95c443822 
> 
> Diff: https://reviews.apache.org/r/38336/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 38336: Make async work queue gating thread-local.

Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38336/
-----------------------------------------------------------

(Updated Sept. 13, 2015, 10:05 a.m.)


Review request for Aurora and Maxim Khutornenko.


Changes
-------

Added javadoc.


Bugs: AURORA-1459
    https://issues.apache.org/jira/browse/AURORA-1459


Repository: aurora


Description
-------

This effectively makes `GatedDelayExecutor` (renamed to `GatingDelayExecutor`) transaction-aware, and avoids unnecessarily delaying independent async work that is not part of a transaction.


Diffs (updated)
-----

  config/findbugs/excludeFilter.xml 7c653022fdc1a33e71da08c2adeaa5760153a08b 
  src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 217b9c0347b259e76180a7021c9c0f9c0c34c89c 
  src/main/java/org/apache/aurora/scheduler/async/FlushableWorkQueue.java 11a1c2a7a920eefeb92cbf230866ed107941f642 
  src/main/java/org/apache/aurora/scheduler/async/GatedDelayExecutor.java 9d4cfcf1f6b9da4261e2b0252d39aeeb1b4427bb 
  src/main/java/org/apache/aurora/scheduler/async/GatedWorkQueue.java PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 6da6193c47580b3e4446371f0df798eccc24f809 
  src/main/java/org/apache/aurora/scheduler/storage/db/DbStorage.java 60365709ced5314e3563ab5d75d7a0fffda9ac91 
  src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 49411289299bfc230e8c6e0ce6fa96e619f57524 
  src/test/java/org/apache/aurora/scheduler/async/GatedDelayExecutorTest.java 28676332f74c6a4cffeb867ef5612e12d629b5b8 
  src/test/java/org/apache/aurora/scheduler/async/GatingDelayExecutorTest.java PRE-CREATION 
  src/test/java/org/apache/aurora/scheduler/pruning/TaskHistoryPrunerTest.java 0c7da074515e1fad534617f9b8f73d939caf948b 
  src/test/java/org/apache/aurora/scheduler/storage/db/DbStorageTest.java 6dd50264b7d119aa5b5f96ab153794e95c443822 

Diff: https://reviews.apache.org/r/38336/diff/


Testing
-------


Thanks,

Bill Farner