Review Request 26269: Remove "UI" from page titles.

[Bill Farner]

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

Review request for Aurora and David McLaughlin.


Repository: aurora


Description
---

"Aurora UI" is a weird thing to call a website.


Diffs
-

  src/main/resources/org/apache/aurora/scheduler/http/ui/index.html 
40617aa5f84170abe8e2a2b8bc06913f90ae8eb0 
  src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js 
3a07ce7927e936d2d30b8e19b2362a073372ad7b 

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


Testing
---


Thanks,

Bill Farner

Review Request 26270: Make the client invocation code actually use the exit codes returned by the client.

[Mark Chu-Carroll]

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

Review request for Aurora, David McLaughlin and Zameer Manji.


Bugs: aurora-781
https://issues.apache.org/jira/browse/aurora-781


Repository: aurora


Description
---

Fix multiple errors involving bindings, and fix result codes.

- Cause an error to be raised by an unresolved binding in a configuration file.
- Fix incorrect structuring of processed bindings.
- Add a test to confirm that unbound parameters generate errors.

While I'm in the code, also fix a problem where result codes aren't returned 
correctly
to the shell.


Diffs
-

  src/main/python/apache/aurora/client/cli/client.py 
0cb69448cd24372067ac845eca5862bc3d3a46a9 
  src/main/python/apache/aurora/client/cli/context.py 
102d20797816788361dfdac450aac9fb8e6fbc28 
  src/main/python/apache/aurora/client/cli/options.py 
c2d422ac2bc82fc387596e93040b49f722f8310f 
  src/main/python/apache/aurora/client/cli/standalone_client.py 
30d8f750559b7811d66760741905fa8adf80fd1f 
  src/test/python/apache/aurora/client/cli/test_create.py 
31fa56f5edcfc97903725ab27ccc12c6a8f39ffc 
  src/test/python/apache/aurora/client/cli/util.py 
a50b83c571390374975accf75e31f392dbdaaa04 

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


Testing
---

Unit tests:
- verified that tests fail if the binding code is disabled, but pass with in on.
- added a new unit test to check that unresolved bindings generate an error.


Thanks,

Mark Chu-Carroll

Re: Review Request 26270: Make the client invocation code actually use the exit codes returned by the client.

[Mark Chu-Carroll]

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

(Updated Oct. 2, 2014, 12:07 p.m.)


Review request for Aurora, David McLaughlin and Zameer Manji.


Changes
---

Improve error messages, and add output testing.


Bugs: aurora-781
https://issues.apache.org/jira/browse/aurora-781


Repository: aurora


Description
---

Fix multiple errors involving bindings, and fix result codes.

- Cause an error to be raised by an unresolved binding in a configuration file.
- Fix incorrect structuring of processed bindings.
- Add a test to confirm that unbound parameters generate errors.

While I'm in the code, also fix a problem where result codes aren't returned 
correctly
to the shell.


Diffs (updated)
-

  src/main/python/apache/aurora/client/cli/client.py 
0cb69448cd24372067ac845eca5862bc3d3a46a9 
  src/main/python/apache/aurora/client/cli/context.py 
102d20797816788361dfdac450aac9fb8e6fbc28 
  src/main/python/apache/aurora/client/cli/options.py 
c2d422ac2bc82fc387596e93040b49f722f8310f 
  src/main/python/apache/aurora/client/cli/standalone_client.py 
30d8f750559b7811d66760741905fa8adf80fd1f 
  src/test/python/apache/aurora/client/cli/test_create.py 
31fa56f5edcfc97903725ab27ccc12c6a8f39ffc 
  src/test/python/apache/aurora/client/cli/util.py 
a50b83c571390374975accf75e31f392dbdaaa04 

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


Testing
---

Unit tests:
- verified that tests fail if the binding code is disabled, but pass with in on.
- added a new unit test to check that unresolved bindings generate an error.


Thanks,

Mark Chu-Carroll

Re: Review Request 26269: Remove "UI" from page titles.

[Joshua Cohen]

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

Ship it!


Ship It!

- Joshua Cohen


On Oct. 2, 2014, 3:48 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26269/
> ---
> 
> (Updated Oct. 2, 2014, 3:48 p.m.)
> 
> 
> Review request for Aurora and David McLaughlin.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> "Aurora UI" is a weird thing to call a website.
> 
> 
> Diffs
> -
> 
>   src/main/resources/org/apache/aurora/scheduler/http/ui/index.html 
> 40617aa5f84170abe8e2a2b8bc06913f90ae8eb0 
>   src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js 
> 3a07ce7927e936d2d30b8e19b2362a073372ad7b 
> 
> Diff: https://reviews.apache.org/r/26269/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 26269: Remove "UI" from page titles.

[Kevin Sweeney]

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

Ship it!


Ship It!

- Kevin Sweeney


On Oct. 2, 2014, 8:48 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26269/
> ---
> 
> (Updated Oct. 2, 2014, 8:48 a.m.)
> 
> 
> Review request for Aurora and David McLaughlin.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> "Aurora UI" is a weird thing to call a website.
> 
> 
> Diffs
> -
> 
>   src/main/resources/org/apache/aurora/scheduler/http/ui/index.html 
> 40617aa5f84170abe8e2a2b8bc06913f90ae8eb0 
>   src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js 
> 3a07ce7927e936d2d30b8e19b2362a073372ad7b 
> 
> Diff: https://reviews.apache.org/r/26269/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 26270: Make the client invocation code actually use the exit codes returned by the client.

[Zameer Manji]

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

Ship it!


Ship It!

- Zameer Manji


On Oct. 2, 2014, 9:07 a.m., Mark Chu-Carroll wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26270/
> ---
> 
> (Updated Oct. 2, 2014, 9:07 a.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Zameer Manji.
> 
> 
> Bugs: aurora-781
> https://issues.apache.org/jira/browse/aurora-781
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Fix multiple errors involving bindings, and fix result codes.
> 
> - Cause an error to be raised by an unresolved binding in a configuration 
> file.
> - Fix incorrect structuring of processed bindings.
> - Add a test to confirm that unbound parameters generate errors.
> 
> While I'm in the code, also fix a problem where result codes aren't returned 
> correctly
> to the shell.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/cli/client.py 
> 0cb69448cd24372067ac845eca5862bc3d3a46a9 
>   src/main/python/apache/aurora/client/cli/context.py 
> 102d20797816788361dfdac450aac9fb8e6fbc28 
>   src/main/python/apache/aurora/client/cli/options.py 
> c2d422ac2bc82fc387596e93040b49f722f8310f 
>   src/main/python/apache/aurora/client/cli/standalone_client.py 
> 30d8f750559b7811d66760741905fa8adf80fd1f 
>   src/test/python/apache/aurora/client/cli/test_create.py 
> 31fa56f5edcfc97903725ab27ccc12c6a8f39ffc 
>   src/test/python/apache/aurora/client/cli/util.py 
> a50b83c571390374975accf75e31f392dbdaaa04 
> 
> Diff: https://reviews.apache.org/r/26270/diff/
> 
> 
> Testing
> ---
> 
> Unit tests:
> - verified that tests fail if the binding code is disabled, but pass with in 
> on.
> - added a new unit test to check that unresolved bindings generate an error.
> 
> 
> Thanks,
> 
> Mark Chu-Carroll
> 
>

Re: Review Request 26270: Fix multiple errors involving bindings, and fix result codes.

[Mark Chu-Carroll]

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

(Updated Oct. 2, 2014, 2:24 p.m.)


Review request for Aurora, David McLaughlin and Zameer Manji.


Summary (updated)
-

Fix multiple errors involving bindings, and fix result codes.


Bugs: aurora-781
https://issues.apache.org/jira/browse/aurora-781


Repository: aurora


Description
---

Fix multiple errors involving bindings, and fix result codes.

- Cause an error to be raised by an unresolved binding in a configuration file.
- Fix incorrect structuring of processed bindings.
- Add a test to confirm that unbound parameters generate errors.

While I'm in the code, also fix a problem where result codes aren't returned 
correctly
to the shell.


Diffs
-

  src/main/python/apache/aurora/client/cli/client.py 
0cb69448cd24372067ac845eca5862bc3d3a46a9 
  src/main/python/apache/aurora/client/cli/context.py 
102d20797816788361dfdac450aac9fb8e6fbc28 
  src/main/python/apache/aurora/client/cli/options.py 
c2d422ac2bc82fc387596e93040b49f722f8310f 
  src/main/python/apache/aurora/client/cli/standalone_client.py 
30d8f750559b7811d66760741905fa8adf80fd1f 
  src/test/python/apache/aurora/client/cli/test_create.py 
31fa56f5edcfc97903725ab27ccc12c6a8f39ffc 
  src/test/python/apache/aurora/client/cli/util.py 
a50b83c571390374975accf75e31f392dbdaaa04 

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


Testing
---

Unit tests:
- verified that tests fail if the binding code is disabled, but pass with in on.
- added a new unit test to check that unresolved bindings generate an error.


Thanks,

Mark Chu-Carroll

Re: Review Request 26232: Implementing update history pruner.

[Maxim Khutornenko]

> On Oct. 2, 2014, 2:54 a.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPruner.java,
> >  line 85
> > 
> >
> > I'm always nervous when important behavior is embedded in something 
> > seemingly-less-important like logging.  Can you extract a variable to 
> > separate the two?

Kind of redundant but sure, done.


> On Oct. 2, 2014, 2:54 a.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java, line 
> > 140
> > 
> >
> > "...last completed updates that completed less than {@code 
> > historyPruneThreshold} ago.."

Done.


> On Oct. 2, 2014, 2:54 a.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java, line 
> > 147
> > 
> >
> > Keep the value rich as far down as you can, to mitigate accidental 
> > misuse:
> > Amount historyPruneThreshold

Disagree. I don't really like Amount -> long ->Amount -> long dance that would 
require. The Amount is converted into long only once now, converted into an 
absolute timestamp and passed down to SQL to compare against.


> On Oct. 2, 2014, 2:54 a.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java, 
> > line 141
> > 
> >
> > s/result/pruned/

Sure.


> On Oct. 2, 2014, 2:54 a.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java, 
> > line 143
> > 
> >
> > How would you feel about making the two pruning goals distinct at the 
> > mapper level?  Does that simplfiy anything?
> > 
> > - get UUIDs of all updates older than pruneThreshold
> > - get UUIDs of the the last >retainCount updates for each job
> > - delete above UUIDs.
> > 
> > I think i would find that easier to follow, at least.

This approach would require an extra SQL call (step 1) and potentially a lot 
more SQL calls in step 2 as we would now deal with raw job keys not 
pre-filtered for processing. The current algorithm is optimized to be less 
SQL-chatty and short circuits if there are no job keys to process:
- get job keys that have updates to be deleted (older than pruneThreshold 
and/or count > retainCount)
- for every key above get update UUIDs and delete them.


> On Oct. 2, 2014, 2:54 a.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java,
> >  line 109
> > 
> >
> > Avoid repeating the method signature in text:
> > s/Set of u/U/

Done.


> On Oct. 2, 2014, 2:54 a.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java,
> >  line 121
> > 
> >
> > How about `getPruneCandidates`?

I don't think it will be clear enough as we are selecting job keys rather then 
UUIDs here. How about selectJobKeysForPruning?


> On Oct. 2, 2014, 2:54 a.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java,
> >  line 320
> > 
> >
> > Why this rather than an explicit delete record for the affected update 
> > UUIDs?

I thought about that but decided to stick with the current solution:
- less code to maintain: deleting a set of UUIDs would require a new 
JobUpdateStore API only used by the LogStorage
- less data to store: persisting a bunch of pruned UUIDs seems redundant where 
a a single prune call restores the consistency much more effectively.


> On Oct. 2, 2014, 2:54 a.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java,
> >  line 318
> > 
> >
> > If this comment remains, please elaborate on why it is not strictly 
> > necessary. (i know, but a future developer might not.)

Done.


> On Oct. 2, 2014, 2:54 a.m., Bill Farner wrote:
> > src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java,
> >  line 45
> > 
> >
> > Can you take a stab at using FakeScheduledExecutor instead of a real 
> > thread?  I don't insist on it, but i'd like to see if that pattern works 
> > out in other situations.
> > 
> > You'll have to modify FakeScheduledExecutor a bit to add support for 
> > `scheduleAtFixedRate`, but at that point all you should have to do in the 
> > test after `repla

Re: Review Request 26232: Implementing update history pruner.

[Maxim Khutornenko]

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

(Updated Oct. 2, 2014, 7:40 p.m.)


Review request for Aurora, David McLaughlin and Bill Farner.


Changes
---

CR comments.


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


Repository: aurora


Description
---

The pruner runs on periodic basis and trims completed updates up to the 
guranteed per job retention count.


Diffs (updated)
-

  src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 
aa45d27391b1786ca3d5e8c928045f1b6f3cf5ef 
  src/main/java/org/apache/aurora/scheduler/async/HistoryPruner.java 
ebae58a04e8857c5f26d4b57c27dfcda9e14c82c 
  src/main/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPruner.java 
PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
c3abffe575e801cebec3572cf4aceac83a238b55 
  src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 
3db0114c5432a5b7d0b01d97c75494be9e3e99a3 
  
src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java
 c583e085e0458835d51ebf740a3b5f01b428bb25 
  src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 
66c91644677e7176ccf53dcfcf29a6792ec398bc 
  
src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
 631ab2543dadb77b7fd1cae96adc9c0bd2f94b39 
  src/main/thrift/org/apache/aurora/gen/storage.thrift 
7e502450f06abb449d06af09cc59185c6a9a2963 
  src/test/java/org/apache/aurora/scheduler/async/HistoryPrunerTest.java 
011d9ec5d84c658c2b2e39bb0b3f9d20b5440cee 
  
src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java 
PRE-CREATION 
  
src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java 
1b1160f349d64bbcd4d20103a82f3b8eb1ca75d9 
  src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
68df0d542e41438c0844f76fc5b9ec6996a00e8d 
  src/test/java/org/apache/aurora/scheduler/updater/FakeScheduledExecutor.java 
e35fe23f023f5accfb2270a3634c91bec657 
  src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 
7254588b55df9a12217c8ec172abc5976892497e 

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


Testing
---

./gradlew -Pq build


Thanks,

Maxim Khutornenko

Re: Review Request 26232: Implementing update history pruner.

[Bill Farner]

> On Oct. 2, 2014, 2:54 a.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java, line 
> > 147
> > 
> >
> > Keep the value rich as far down as you can, to mitigate accidental 
> > misuse:
> > Amount historyPruneThreshold
> 
> Maxim Khutornenko wrote:
> Disagree. I don't really like Amount -> long ->Amount -> long dance that 
> would require. The Amount is converted into long only once now, converted 
> into an absolute timestamp and passed down to SQL to compare against.

I'm actually suggesting you avoid the dance completely.  Keep Amount in the settings object, and only translate it to a long when you need a 
long.  The only argument i can see against this is performance, which doesn't 
hold here.


- Bill


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


On Oct. 2, 2014, 7:40 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26232/
> ---
> 
> (Updated Oct. 2, 2014, 7:40 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Bill Farner.
> 
> 
> Bugs: AURORA-743
> https://issues.apache.org/jira/browse/AURORA-743
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The pruner runs on periodic basis and trims completed updates up to the 
> guranteed per job retention count.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 
> aa45d27391b1786ca3d5e8c928045f1b6f3cf5ef 
>   src/main/java/org/apache/aurora/scheduler/async/HistoryPruner.java 
> ebae58a04e8857c5f26d4b57c27dfcda9e14c82c 
>   src/main/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPruner.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
> c3abffe575e801cebec3572cf4aceac83a238b55 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 
> 3db0114c5432a5b7d0b01d97c75494be9e3e99a3 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java
>  c583e085e0458835d51ebf740a3b5f01b428bb25 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 
> 66c91644677e7176ccf53dcfcf29a6792ec398bc 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
>  631ab2543dadb77b7fd1cae96adc9c0bd2f94b39 
>   src/main/thrift/org/apache/aurora/gen/storage.thrift 
> 7e502450f06abb449d06af09cc59185c6a9a2963 
>   src/test/java/org/apache/aurora/scheduler/async/HistoryPrunerTest.java 
> 011d9ec5d84c658c2b2e39bb0b3f9d20b5440cee 
>   
> src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java
>  PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java
>  1b1160f349d64bbcd4d20103a82f3b8eb1ca75d9 
>   src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
> 68df0d542e41438c0844f76fc5b9ec6996a00e8d 
>   
> src/test/java/org/apache/aurora/scheduler/updater/FakeScheduledExecutor.java 
> e35fe23f023f5accfb2270a3634c91bec657 
>   src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 
> 7254588b55df9a12217c8ec172abc5976892497e 
> 
> Diff: https://reviews.apache.org/r/26232/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 26270: Fix multiple errors involving bindings, and fix result codes.

[David McLaughlin]

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

Ship it!


Ship It!

- David McLaughlin


On Oct. 2, 2014, 6:24 p.m., Mark Chu-Carroll wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26270/
> ---
> 
> (Updated Oct. 2, 2014, 6:24 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Zameer Manji.
> 
> 
> Bugs: aurora-781
> https://issues.apache.org/jira/browse/aurora-781
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Fix multiple errors involving bindings, and fix result codes.
> 
> - Cause an error to be raised by an unresolved binding in a configuration 
> file.
> - Fix incorrect structuring of processed bindings.
> - Add a test to confirm that unbound parameters generate errors.
> 
> While I'm in the code, also fix a problem where result codes aren't returned 
> correctly
> to the shell.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/cli/client.py 
> 0cb69448cd24372067ac845eca5862bc3d3a46a9 
>   src/main/python/apache/aurora/client/cli/context.py 
> 102d20797816788361dfdac450aac9fb8e6fbc28 
>   src/main/python/apache/aurora/client/cli/options.py 
> c2d422ac2bc82fc387596e93040b49f722f8310f 
>   src/main/python/apache/aurora/client/cli/standalone_client.py 
> 30d8f750559b7811d66760741905fa8adf80fd1f 
>   src/test/python/apache/aurora/client/cli/test_create.py 
> 31fa56f5edcfc97903725ab27ccc12c6a8f39ffc 
>   src/test/python/apache/aurora/client/cli/util.py 
> a50b83c571390374975accf75e31f392dbdaaa04 
> 
> Diff: https://reviews.apache.org/r/26270/diff/
> 
> 
> Testing
> ---
> 
> Unit tests:
> - verified that tests fail if the binding code is disabled, but pass with in 
> on.
> - added a new unit test to check that unresolved bindings generate an error.
> 
> 
> Thanks,
> 
> Mark Chu-Carroll
> 
>

Re: Review Request 26232: Implementing update history pruner.

[Maxim Khutornenko]

> On Oct. 2, 2014, 2:54 a.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java, line 
> > 147
> > 
> >
> > Keep the value rich as far down as you can, to mitigate accidental 
> > misuse:
> > Amount historyPruneThreshold
> 
> Maxim Khutornenko wrote:
> Disagree. I don't really like Amount -> long ->Amount -> long dance that 
> would require. The Amount is converted into long only once now, converted 
> into an absolute timestamp and passed down to SQL to compare against.
> 
> Bill Farner wrote:
> I'm actually suggesting you avoid the dance completely.  Keep 
> Amount in the settings object, and only translate it to a long 
> when you need a long.  The only argument i can see against this is 
> performance, which doesn't hold here.

Ah, in that case the comment was misplaced as I was reading that you wanted to 
make JobUpdateStore API Amount-aware :)


- Maxim


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


On Oct. 2, 2014, 7:40 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26232/
> ---
> 
> (Updated Oct. 2, 2014, 7:40 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Bill Farner.
> 
> 
> Bugs: AURORA-743
> https://issues.apache.org/jira/browse/AURORA-743
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The pruner runs on periodic basis and trims completed updates up to the 
> guranteed per job retention count.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 
> aa45d27391b1786ca3d5e8c928045f1b6f3cf5ef 
>   src/main/java/org/apache/aurora/scheduler/async/HistoryPruner.java 
> ebae58a04e8857c5f26d4b57c27dfcda9e14c82c 
>   src/main/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPruner.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
> c3abffe575e801cebec3572cf4aceac83a238b55 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 
> 3db0114c5432a5b7d0b01d97c75494be9e3e99a3 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java
>  c583e085e0458835d51ebf740a3b5f01b428bb25 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 
> 66c91644677e7176ccf53dcfcf29a6792ec398bc 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
>  631ab2543dadb77b7fd1cae96adc9c0bd2f94b39 
>   src/main/thrift/org/apache/aurora/gen/storage.thrift 
> 7e502450f06abb449d06af09cc59185c6a9a2963 
>   src/test/java/org/apache/aurora/scheduler/async/HistoryPrunerTest.java 
> 011d9ec5d84c658c2b2e39bb0b3f9d20b5440cee 
>   
> src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java
>  PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java
>  1b1160f349d64bbcd4d20103a82f3b8eb1ca75d9 
>   src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
> 68df0d542e41438c0844f76fc5b9ec6996a00e8d 
>   
> src/test/java/org/apache/aurora/scheduler/updater/FakeScheduledExecutor.java 
> e35fe23f023f5accfb2270a3634c91bec657 
>   src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 
> 7254588b55df9a12217c8ec172abc5976892497e 
> 
> Diff: https://reviews.apache.org/r/26232/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 26232: Implementing update history pruner.

[Maxim Khutornenko]

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

(Updated Oct. 2, 2014, 7:54 p.m.)


Review request for Aurora, David McLaughlin and Bill Farner.


Changes
---

CR comments.


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


Repository: aurora


Description
---

The pruner runs on periodic basis and trims completed updates up to the 
guranteed per job retention count.


Diffs (updated)
-

  src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 
aa45d27391b1786ca3d5e8c928045f1b6f3cf5ef 
  src/main/java/org/apache/aurora/scheduler/async/HistoryPruner.java 
ebae58a04e8857c5f26d4b57c27dfcda9e14c82c 
  src/main/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPruner.java 
PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
c3abffe575e801cebec3572cf4aceac83a238b55 
  src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 
3db0114c5432a5b7d0b01d97c75494be9e3e99a3 
  
src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java
 c583e085e0458835d51ebf740a3b5f01b428bb25 
  src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 
66c91644677e7176ccf53dcfcf29a6792ec398bc 
  
src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
 631ab2543dadb77b7fd1cae96adc9c0bd2f94b39 
  src/main/thrift/org/apache/aurora/gen/storage.thrift 
7e502450f06abb449d06af09cc59185c6a9a2963 
  src/test/java/org/apache/aurora/scheduler/async/HistoryPrunerTest.java 
011d9ec5d84c658c2b2e39bb0b3f9d20b5440cee 
  
src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java 
PRE-CREATION 
  
src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java 
1b1160f349d64bbcd4d20103a82f3b8eb1ca75d9 
  src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
68df0d542e41438c0844f76fc5b9ec6996a00e8d 
  src/test/java/org/apache/aurora/scheduler/updater/FakeScheduledExecutor.java 
e35fe23f023f5accfb2270a3634c91bec657 
  src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 
7254588b55df9a12217c8ec172abc5976892497e 

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


Testing
---

./gradlew -Pq build


Thanks,

Maxim Khutornenko

Re: Review Request 26137: Fix help for new update command.

[Mark Chu-Carroll]

> On Sept. 30, 2014, 2:32 a.m., Joe Smith wrote:
> > src/main/python/apache/aurora/client/cli/update.py, line 45
> > 
> >
> > Could you update a test case to catch accessing these as properties to 
> > catch accidental regressions?
> 
> David McLaughlin wrote:
> Piggy backing this issue to add that my ship it is pending a test for 
> this command at least?

I don't know of any way to write a single test that would always catch this.


- Mark


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


On Sept. 29, 2014, 11:05 a.m., Mark Chu-Carroll wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26137/
> ---
> 
> (Updated Sept. 29, 2014, 11:05 a.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Zameer Manji.
> 
> 
> Bugs: aurora-748
> https://issues.apache.org/jira/browse/aurora-748
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Fix help for new update command.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/cli/update.py 
> b4dd792dc12f19424c620f4d91748113e272f0c9 
> 
> Diff: https://reviews.apache.org/r/26137/diff/
> 
> 
> Testing
> ---
> 
> manual testing.
> 
> 
> Thanks,
> 
> Mark Chu-Carroll
> 
>

Re: Review Request 26004: Add "aurora update list" and "aurora update status" commands.

[Mark Chu-Carroll]

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

(Updated Oct. 2, 2014, 4:41 p.m.)


Review request for Aurora, David McLaughlin and Zameer Manji.


Changes
---

Add the IDs to "update list".


Bugs: aurora-742
https://issues.apache.org/jira/browse/aurora-742


Repository: aurora


Description
---

Add support for commands to query and display active updates being
managed by the scheduler. Two commands are added:

* "aurora update list", which shows all active updates that are being processed 
by the server.
* "aurora update status", which shows detailed status information about an 
update in-progress on the server.


Diffs (updated)
-

  src/main/python/apache/aurora/client/cli/context.py 
f639af7de93a069b278dc494b6f92a2f6b10de9c 
  src/main/python/apache/aurora/client/cli/options.py 
9a81a1d7d04c6648e94ff117429278317a9dbed8 
  src/main/python/apache/aurora/client/cli/update.py 
b4dd792dc12f19424c620f4d91748113e272f0c9 
  src/test/python/apache/aurora/client/cli/test_supdate.py 
2782fee2867c6ef79349240299de082f07f7967a 
  src/test/python/apache/aurora/client/cli/util.py 
e1ee884c06f3bc22bcd9e908ff61af9459a0b535 

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


Testing
---

Added new unit tests; all tests pass.


Thanks,

Mark Chu-Carroll

Re: Review Request 26137: Fix help for new update command.

[Joe Smith]

> On Sept. 29, 2014, 11:32 p.m., Joe Smith wrote:
> > src/main/python/apache/aurora/client/cli/update.py, line 45
> > 
> >
> > Could you update a test case to catch accessing these as properties to 
> > catch accidental regressions?
> 
> David McLaughlin wrote:
> Piggy backing this issue to add that my ship it is pending a test for 
> this command at least?
> 
> Mark Chu-Carroll wrote:
> I don't know of any way to write a single test that would always catch 
> this.

Rebased off your diff:

[tw-172-25-132-201 aurora (yasumoto/test_update_help)]$ git diff
diff --git a/src/main/python/apache/aurora/client/cli/update.py 
b/src/main/python/apache/aurora/client/cli/update.py
index 41475a7..ef07a11 100644
--- a/src/main/python/apache/aurora/client/cli/update.py
+++ b/src/main/python/apache/aurora/client/cli/update.py
@@ -42,7 +42,6 @@ class StartUpdate(Verb):
   INSTANCES_SPEC_ARGUMENT, CONFIG_ARGUMENT
 ]
 
-  @property
   def help(self):
 return textwrap.dedent("""\
 Start a scheduler-driven rolling upgrade on a running job, using the 
update
diff --git a/src/test/python/apache/aurora/client/cli/test_update.py 
b/src/test/python/apache/aurora/client/cli/test_update.py
index eeed774..1a38ffe 100644
--- a/src/test/python/apache/aurora/client/cli/test_update.py
+++ b/src/test/python/apache/aurora/client/cli/test_update.py
@@ -23,6 +23,7 @@ from apache.aurora.client.api.quota_check import QuotaCheck
 from apache.aurora.client.api.scheduler_mux import SchedulerMux
 from apache.aurora.client.cli import EXIT_INVALID_CONFIGURATION, EXIT_OK
 from apache.aurora.client.cli.client import AuroraCommandLine
+from apache.aurora.client.cli.update import StartUpdate
 from apache.aurora.client.cli.util import AuroraClientCommandTest, 
FakeAuroraCommandContext, IOMock
 from apache.aurora.config import AuroraConfig
 
@@ -301,6 +302,10 @@ class TestUpdateCommand(AuroraClientCommandTest):
   'Update completed successfully']
   assert mock_err.get() == []
 
+  def test_update_start_help(self):
+start = StartUpdate()
+assert 'Start a scheduler-driven rolling' in start.help
+
   @classmethod
   def assert_correct_addinstance_calls(cls, api):
 assert api.addInstances.call_count == 20
[tw-172-25-132-201 aurora (yasumoto/test_update_help)]$ ./pants 
./src/test/python/apache/aurora/client/cli:job
Build operating on top level addresses: 
set([BuildFileAddress(/Users/jsmith/workspace/aurora/src/test/python/apache/aurora/client/cli/BUILD,
 job)])

 test session starts 
=
platform darwin -- Python 2.6.8 -- py-1.4.25 -- pytest-2.6.3
plugins: cov, timeout
collected 61 items 

src/test/python/apache/aurora/client/cli/test_cancel_update.py ..
src/test/python/apache/aurora/client/cli/test_create.py ..
src/test/python/apache/aurora/client/cli/test_diff.py ...
src/test/python/apache/aurora/client/cli/test_kill.py 
src/test/python/apache/aurora/client/cli/test_open.py .
src/test/python/apache/aurora/client/cli/test_restart.py 
src/test/python/apache/aurora/client/cli/test_status.py ...
src/test/python/apache/aurora/client/cli/test_update.py ..F...

==
 FAILURES 
==
__
 TestUpdateCommand.test_update_start_help 
__

self = 

def test_update_start_help(self):
  start = StartUpdate()
> assert 'Start a scheduler-driven rolling' in start.help
E TypeError: argument of type 'instancemethod' is not iterable

src/test/python/apache/aurora/client/cli/test_update.py:307: TypeError

 1 failed, 60 passed in 6.07 seconds 
=
src.test.python.apache.aurora.client.cli.job
.   FAILURE


[tw-172-25-132-201 aurora (yasumoto/test_update_help)]$ git diff
diff --git a/src/test/python/apache/aurora/client/cli/test_update

Review Request 26283: Adding authz check into descheduleCronJob RPC.

[Maxim Khutornenko]

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

Review request for Aurora and Bill Farner.


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


Repository: aurora


Description
---

Adding authz check into descheduleCronJob RPC.


Diffs
-

  
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 
23115fb64fc6c2324b416e9527b0f3952c253977 
  
src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
 b21dce6588d8d1d9fde85e8ab7ab1ee6e2587008 

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


Testing
---

./gradlew -Pq build


Thanks,

Maxim Khutornenko

Re: Review Request 25974: Prevent initial ZK timeouts from killing the executor.

[Zameer Manji]

> On Oct. 1, 2014, 4:34 p.m., Kevin Sweeney wrote:
> > src/main/python/apache/aurora/executor/common/announcer.py, line 228
> > 
> >
> > push this into `__start`, out of the constructor?
> > 
> > At least on the Java side we try to avoid doing any I/O in constructors 
> > (as they're more for wiring) but instead delegate to explicit lifecycle 
> > methods like `start`.

I'd rather not. Keeping it in the constructor means it stays in the 
from_assigned_task method code path. Moving it to the start method moves it out 
of that code path and really changes when we do this sort of I/O.


- Zameer


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


On Sept. 30, 2014, 5:17 p.m., Zameer Manji wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25974/
> ---
> 
> (Updated Sept. 30, 2014, 5:17 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney, Bill Farner, and Brian Wickman.
> 
> 
> Bugs: AURORA-728
> https://issues.apache.org/jira/browse/AURORA-728
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Prevent initial ZK timeouts from killing the executor. In addition, prevent 
> uncaught exceptions from killing the executor.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/executor/aurora_executor.py 
> 79a24855b2a68271b7478395dfdadab8755c3af2 
>   src/main/python/apache/aurora/executor/common/announcer.py 
> c466da8d48bbc2aa227c2db157cab84665ad6602 
>   src/test/python/apache/aurora/executor/common/test_announcer.py 
> 4f6e200ecb1a4ea7cb45acd466a57f19d5815326 
> 
> Diff: https://reviews.apache.org/r/25974/diff/
> 
> 
> Testing
> ---
> 
> ./pants src/test/python/apache/aurora/executor:executor-small
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Zameer Manji
> 
>

Re: Review Request 25974: Prevent initial ZK timeouts from killing the executor.

[Zameer Manji]

> On Oct. 1, 2014, 4:14 p.m., Brian Wickman wrote:
> > src/test/python/apache/aurora/executor/common/test_announcer.py, line 239
> > 
> >
> > out of scope for this review, but there's a new-ish pypi project called 
> > 'zake' that allows the kazoo client to be stubbed out with a mock zk tree 
> > (while preserving certain operations like create/delete.)  i'd love to 
> > explore using it here in the future.

It looks really nice for this sort of testing. It is definately out of the 
scope of this review. If we ever need to do more work on the Announcer/Service 
Discovery we should first migrate to zake.


- Zameer


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


On Sept. 30, 2014, 5:17 p.m., Zameer Manji wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25974/
> ---
> 
> (Updated Sept. 30, 2014, 5:17 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney, Bill Farner, and Brian Wickman.
> 
> 
> Bugs: AURORA-728
> https://issues.apache.org/jira/browse/AURORA-728
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Prevent initial ZK timeouts from killing the executor. In addition, prevent 
> uncaught exceptions from killing the executor.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/executor/aurora_executor.py 
> 79a24855b2a68271b7478395dfdadab8755c3af2 
>   src/main/python/apache/aurora/executor/common/announcer.py 
> c466da8d48bbc2aa227c2db157cab84665ad6602 
>   src/test/python/apache/aurora/executor/common/test_announcer.py 
> 4f6e200ecb1a4ea7cb45acd466a57f19d5815326 
> 
> Diff: https://reviews.apache.org/r/25974/diff/
> 
> 
> Testing
> ---
> 
> ./pants src/test/python/apache/aurora/executor:executor-small
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Zameer Manji
> 
>

Review Request 26288: Fixing log_response in context.py

[Maxim Khutornenko]

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

Review request for Aurora and Mark Chu-Carroll.


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


Repository: aurora


Description
---

Fixing log_response in context.py


Diffs
-

  src/main/python/apache/aurora/client/cli/context.py 
f639af7de93a069b278dc494b6f92a2f6b10de9c 
  src/test/python/apache/aurora/client/cli/BUILD 
8ce6bd5b7faa1579372fb6935180ea982af64af8 
  src/test/python/apache/aurora/client/cli/test_context.py PRE-CREATION 

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


Testing
---

./pants src/tests/python:all


Thanks,

Maxim Khutornenko

Re: Review Request 26283: Adding authz check into descheduleCronJob RPC.

[Bill Farner]

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

Ship it!


Ship It!

- Bill Farner


On Oct. 2, 2014, 9:44 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26283/
> ---
> 
> (Updated Oct. 2, 2014, 9:44 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Bugs: AURORA-566
> https://issues.apache.org/jira/browse/AURORA-566
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Adding authz check into descheduleCronJob RPC.
> 
> 
> Diffs
> -
> 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  23115fb64fc6c2324b416e9527b0f3952c253977 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  b21dce6588d8d1d9fde85e8ab7ab1ee6e2587008 
> 
> Diff: https://reviews.apache.org/r/26283/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 26232: Implementing update history pruner.

[Bill Farner]

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

Ship it!



src/main/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPruner.java


if (!prunedUpdates.isEmpty()) {
  LOG.info(...);
}



src/test/java/org/apache/aurora/scheduler/updater/FakeScheduledExecutor.java


line break above


- Bill Farner


On Oct. 2, 2014, 7:54 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26232/
> ---
> 
> (Updated Oct. 2, 2014, 7:54 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Bill Farner.
> 
> 
> Bugs: AURORA-743
> https://issues.apache.org/jira/browse/AURORA-743
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The pruner runs on periodic basis and trims completed updates up to the 
> guranteed per job retention count.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 
> aa45d27391b1786ca3d5e8c928045f1b6f3cf5ef 
>   src/main/java/org/apache/aurora/scheduler/async/HistoryPruner.java 
> ebae58a04e8857c5f26d4b57c27dfcda9e14c82c 
>   src/main/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPruner.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
> c3abffe575e801cebec3572cf4aceac83a238b55 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 
> 3db0114c5432a5b7d0b01d97c75494be9e3e99a3 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java
>  c583e085e0458835d51ebf740a3b5f01b428bb25 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 
> 66c91644677e7176ccf53dcfcf29a6792ec398bc 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
>  631ab2543dadb77b7fd1cae96adc9c0bd2f94b39 
>   src/main/thrift/org/apache/aurora/gen/storage.thrift 
> 7e502450f06abb449d06af09cc59185c6a9a2963 
>   src/test/java/org/apache/aurora/scheduler/async/HistoryPrunerTest.java 
> 011d9ec5d84c658c2b2e39bb0b3f9d20b5440cee 
>   
> src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java
>  PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java
>  1b1160f349d64bbcd4d20103a82f3b8eb1ca75d9 
>   src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
> 68df0d542e41438c0844f76fc5b9ec6996a00e8d 
>   
> src/test/java/org/apache/aurora/scheduler/updater/FakeScheduledExecutor.java 
> e35fe23f023f5accfb2270a3634c91bec657 
>   src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 
> 7254588b55df9a12217c8ec172abc5976892497e 
> 
> Diff: https://reviews.apache.org/r/26232/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 26232: Implementing update history pruner.

[Maxim Khutornenko]

> On Oct. 2, 2014, 10:39 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPruner.java,
> >  line 89
> > 
> >
> > if (!prunedUpdates.isEmpty()) {
> >   LOG.info(...);
> > }

I actually want to log in either case for better transparency. Modified to 
support both cases.


> On Oct. 2, 2014, 10:39 p.m., Bill Farner wrote:
> > src/test/java/org/apache/aurora/scheduler/updater/FakeScheduledExecutor.java,
> >  line 75
> > 
> >
> > line break above

Done.


- Maxim


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


On Oct. 2, 2014, 7:54 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26232/
> ---
> 
> (Updated Oct. 2, 2014, 7:54 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Bill Farner.
> 
> 
> Bugs: AURORA-743
> https://issues.apache.org/jira/browse/AURORA-743
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The pruner runs on periodic basis and trims completed updates up to the 
> guranteed per job retention count.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 
> aa45d27391b1786ca3d5e8c928045f1b6f3cf5ef 
>   src/main/java/org/apache/aurora/scheduler/async/HistoryPruner.java 
> ebae58a04e8857c5f26d4b57c27dfcda9e14c82c 
>   src/main/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPruner.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
> c3abffe575e801cebec3572cf4aceac83a238b55 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 
> 3db0114c5432a5b7d0b01d97c75494be9e3e99a3 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java
>  c583e085e0458835d51ebf740a3b5f01b428bb25 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 
> 66c91644677e7176ccf53dcfcf29a6792ec398bc 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
>  631ab2543dadb77b7fd1cae96adc9c0bd2f94b39 
>   src/main/thrift/org/apache/aurora/gen/storage.thrift 
> 7e502450f06abb449d06af09cc59185c6a9a2963 
>   src/test/java/org/apache/aurora/scheduler/async/HistoryPrunerTest.java 
> 011d9ec5d84c658c2b2e39bb0b3f9d20b5440cee 
>   
> src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java
>  PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java
>  1b1160f349d64bbcd4d20103a82f3b8eb1ca75d9 
>   src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
> 68df0d542e41438c0844f76fc5b9ec6996a00e8d 
>   
> src/test/java/org/apache/aurora/scheduler/updater/FakeScheduledExecutor.java 
> e35fe23f023f5accfb2270a3634c91bec657 
>   src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 
> 7254588b55df9a12217c8ec172abc5976892497e 
> 
> Diff: https://reviews.apache.org/r/26232/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 26232: Implementing update history pruner.

[Maxim Khutornenko]

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

(Updated Oct. 2, 2014, 10:51 p.m.)


Review request for Aurora, David McLaughlin and Bill Farner.


Changes
---

CR comments.


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


Repository: aurora


Description
---

The pruner runs on periodic basis and trims completed updates up to the 
guranteed per job retention count.


Diffs (updated)
-

  src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 
aa45d27391b1786ca3d5e8c928045f1b6f3cf5ef 
  src/main/java/org/apache/aurora/scheduler/async/HistoryPruner.java 
ebae58a04e8857c5f26d4b57c27dfcda9e14c82c 
  src/main/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPruner.java 
PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
c3abffe575e801cebec3572cf4aceac83a238b55 
  src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 
3db0114c5432a5b7d0b01d97c75494be9e3e99a3 
  
src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java
 c583e085e0458835d51ebf740a3b5f01b428bb25 
  src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 
66c91644677e7176ccf53dcfcf29a6792ec398bc 
  
src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
 2a81a94f99f242bbe400d8428ad1f1ce38a06a86 
  src/main/thrift/org/apache/aurora/gen/storage.thrift 
7e502450f06abb449d06af09cc59185c6a9a2963 
  src/test/java/org/apache/aurora/scheduler/async/HistoryPrunerTest.java 
011d9ec5d84c658c2b2e39bb0b3f9d20b5440cee 
  
src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java 
PRE-CREATION 
  
src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java 
dbf0badbfcc19f40d9b9eeec22b7024d95a02884 
  src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
68df0d542e41438c0844f76fc5b9ec6996a00e8d 
  src/test/java/org/apache/aurora/scheduler/updater/FakeScheduledExecutor.java 
e35fe23f023f5accfb2270a3634c91bec657 
  src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 
6758b7b56e77882c67be2e39481ff76893ad1610 

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


Testing
---

./gradlew -Pq build


Thanks,

Maxim Khutornenko

Re: Review Request 25974: Prevent initial ZK timeouts from killing the executor.

[Kevin Sweeney]

> On Oct. 1, 2014, 4:34 p.m., Kevin Sweeney wrote:
> > src/main/python/apache/aurora/executor/common/announcer.py, line 228
> > 
> >
> > push this into `__start`, out of the constructor?
> > 
> > At least on the Java side we try to avoid doing any I/O in constructors 
> > (as they're more for wiring) but instead delegate to explicit lifecycle 
> > methods like `start`.
> 
> Zameer Manji wrote:
> I'd rather not. Keeping it in the constructor means it stays in the 
> from_assigned_task method code path. Moving it to the start method moves it 
> out of that code path and really changes when we do this sort of I/O.

is it common practice to do I/O in constructors elsewhere in this codebase? 
I'll defer to wickman here


- Kevin


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


On Sept. 30, 2014, 5:17 p.m., Zameer Manji wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25974/
> ---
> 
> (Updated Sept. 30, 2014, 5:17 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney, Bill Farner, and Brian Wickman.
> 
> 
> Bugs: AURORA-728
> https://issues.apache.org/jira/browse/AURORA-728
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Prevent initial ZK timeouts from killing the executor. In addition, prevent 
> uncaught exceptions from killing the executor.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/executor/aurora_executor.py 
> 79a24855b2a68271b7478395dfdadab8755c3af2 
>   src/main/python/apache/aurora/executor/common/announcer.py 
> c466da8d48bbc2aa227c2db157cab84665ad6602 
>   src/test/python/apache/aurora/executor/common/test_announcer.py 
> 4f6e200ecb1a4ea7cb45acd466a57f19d5815326 
> 
> Diff: https://reviews.apache.org/r/25974/diff/
> 
> 
> Testing
> ---
> 
> ./pants src/test/python/apache/aurora/executor:executor-small
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Zameer Manji
> 
>

Re: Review Request 26232: Implementing update history pruner.

[David McLaughlin]

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



src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml


It'd be nice if this didn't just blanket delete all old updates, especially 
for active jobs. There are probably a certain class of service/cron that are 
rarely touched and it would be nice to keep around that change history. Would 
it add too much complexity to try and solve this?


- David McLaughlin


On Oct. 2, 2014, 10:51 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26232/
> ---
> 
> (Updated Oct. 2, 2014, 10:51 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Bill Farner.
> 
> 
> Bugs: AURORA-743
> https://issues.apache.org/jira/browse/AURORA-743
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The pruner runs on periodic basis and trims completed updates up to the 
> guranteed per job retention count.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 
> aa45d27391b1786ca3d5e8c928045f1b6f3cf5ef 
>   src/main/java/org/apache/aurora/scheduler/async/HistoryPruner.java 
> ebae58a04e8857c5f26d4b57c27dfcda9e14c82c 
>   src/main/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPruner.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
> c3abffe575e801cebec3572cf4aceac83a238b55 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 
> 3db0114c5432a5b7d0b01d97c75494be9e3e99a3 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java
>  c583e085e0458835d51ebf740a3b5f01b428bb25 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 
> 66c91644677e7176ccf53dcfcf29a6792ec398bc 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
>  2a81a94f99f242bbe400d8428ad1f1ce38a06a86 
>   src/main/thrift/org/apache/aurora/gen/storage.thrift 
> 7e502450f06abb449d06af09cc59185c6a9a2963 
>   src/test/java/org/apache/aurora/scheduler/async/HistoryPrunerTest.java 
> 011d9ec5d84c658c2b2e39bb0b3f9d20b5440cee 
>   
> src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java
>  PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java
>  dbf0badbfcc19f40d9b9eeec22b7024d95a02884 
>   src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
> 68df0d542e41438c0844f76fc5b9ec6996a00e8d 
>   
> src/test/java/org/apache/aurora/scheduler/updater/FakeScheduledExecutor.java 
> e35fe23f023f5accfb2270a3634c91bec657 
>   src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 
> 6758b7b56e77882c67be2e39481ff76893ad1610 
> 
> Diff: https://reviews.apache.org/r/26232/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 25974: Prevent initial ZK timeouts from killing the executor.

[Zameer Manji]

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

(Updated Oct. 2, 2014, 4:20 p.m.)


Review request for Aurora, Kevin Sweeney, Bill Farner, and Brian Wickman.


Changes
---

Brian's feedback.


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


Repository: aurora


Description
---

Prevent initial ZK timeouts from killing the executor. In addition, prevent 
uncaught exceptions from killing the executor.


Diffs (updated)
-

  src/main/python/apache/aurora/executor/aurora_executor.py 
79a24855b2a68271b7478395dfdadab8755c3af2 
  src/main/python/apache/aurora/executor/common/announcer.py 
c466da8d48bbc2aa227c2db157cab84665ad6602 
  src/test/python/apache/aurora/executor/common/test_announcer.py 
4f6e200ecb1a4ea7cb45acd466a57f19d5815326 

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


Testing
---

./pants src/test/python/apache/aurora/executor:executor-small
./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh


Thanks,

Zameer Manji

Re: Review Request 26232: Implementing update history pruner.

[Maxim Khutornenko]

> On Oct. 2, 2014, 11:16 p.m., David McLaughlin wrote:
> > src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml,
> >  lines 468-490
> > 
> >
> > It'd be nice if this didn't just blanket delete all old updates, 
> > especially for active jobs. There are probably a certain class of 
> > service/cron that are rarely touched and it would be nice to keep around 
> > that change history. Would it add too much complexity to try and solve this?

Well, that would require active task queries and implementation leaking outside 
DB layer. I'd rather not attempt anything like that until we move TaskStore 
into SQL.


- Maxim


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


On Oct. 2, 2014, 10:51 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26232/
> ---
> 
> (Updated Oct. 2, 2014, 10:51 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Bill Farner.
> 
> 
> Bugs: AURORA-743
> https://issues.apache.org/jira/browse/AURORA-743
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The pruner runs on periodic basis and trims completed updates up to the 
> guranteed per job retention count.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 
> aa45d27391b1786ca3d5e8c928045f1b6f3cf5ef 
>   src/main/java/org/apache/aurora/scheduler/async/HistoryPruner.java 
> ebae58a04e8857c5f26d4b57c27dfcda9e14c82c 
>   src/main/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPruner.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
> c3abffe575e801cebec3572cf4aceac83a238b55 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 
> 3db0114c5432a5b7d0b01d97c75494be9e3e99a3 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java
>  c583e085e0458835d51ebf740a3b5f01b428bb25 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 
> 66c91644677e7176ccf53dcfcf29a6792ec398bc 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
>  2a81a94f99f242bbe400d8428ad1f1ce38a06a86 
>   src/main/thrift/org/apache/aurora/gen/storage.thrift 
> 7e502450f06abb449d06af09cc59185c6a9a2963 
>   src/test/java/org/apache/aurora/scheduler/async/HistoryPrunerTest.java 
> 011d9ec5d84c658c2b2e39bb0b3f9d20b5440cee 
>   
> src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java
>  PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java
>  dbf0badbfcc19f40d9b9eeec22b7024d95a02884 
>   src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
> 68df0d542e41438c0844f76fc5b9ec6996a00e8d 
>   
> src/test/java/org/apache/aurora/scheduler/updater/FakeScheduledExecutor.java 
> e35fe23f023f5accfb2270a3634c91bec657 
>   src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 
> 6758b7b56e77882c67be2e39481ff76893ad1610 
> 
> Diff: https://reviews.apache.org/r/26232/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Review Request 26300: Sleep 10sec instead of 5sec during GC shutdown

[Kevin Sweeney]

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

Review request for Aurora, Vinod Kone and Brian Wickman.


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


Repository: aurora


Description
---

Sleep 10sec instead of 5sec during GC shutdown


Diffs
-

  src/main/python/apache/aurora/executor/gc_executor.py 
44eb0da984a126536f0d277da3da128089201a47 

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


Testing
---


Thanks,

Kevin Sweeney

Review Request 26298: Use a less broad retry loop for RPCs.

[Bill Farner]

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

Review request for Aurora, Mark Chu-Carroll and Maxim Khutornenko.


Repository: aurora


Description
---

A few times when making changes, i've found myself confused at a stalled test 
and spiked CPU, only to find that my test should have failed, but an exception 
is trapped in this retry loop.  The key change here is that unknown exceptions 
will break the loop.

Making this change pointed out what should have been a test failure in 
`test_transient_error`, where an exception caused by an unexpected call to 
`getVersion` was swallowed.

I also changed the test to avoid sleeping, which was eating a fixed 10 seconds 
of unit test time.


Diffs
-

  src/main/python/apache/aurora/client/api/scheduler_client.py 
b400cb2dbdb35077fc2c4a6e161c2959a9217317 
  src/test/python/apache/aurora/client/api/test_scheduler_client.py 
1cbfbf86e903d890baac7d34461109f9beaff442 

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


Testing
---

./pants src/test/python:all -vxs


Thanks,

Bill Farner

Re: Review Request 26232: Implementing update history pruner.

[Bill Farner]

> On Oct. 2, 2014, 11:16 p.m., David McLaughlin wrote:
> > src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml,
> >  lines 468-490
> > 
> >
> > It'd be nice if this didn't just blanket delete all old updates, 
> > especially for active jobs. There are probably a certain class of 
> > service/cron that are rarely touched and it would be nice to keep around 
> > that change history. Would it add too much complexity to try and solve this?
> 
> Maxim Khutornenko wrote:
> Well, that would require active task queries and implementation leaking 
> outside DB layer. I'd rather not attempt anything like that until we move 
> TaskStore into SQL.

Let's start with this policy and revisit.  Maxim - mind dropping a TODO in 
DbJobUpdateStore to consider retaining at least the most recent update for all 
jobs?


- Bill


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


On Oct. 2, 2014, 10:51 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26232/
> ---
> 
> (Updated Oct. 2, 2014, 10:51 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Bill Farner.
> 
> 
> Bugs: AURORA-743
> https://issues.apache.org/jira/browse/AURORA-743
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The pruner runs on periodic basis and trims completed updates up to the 
> guranteed per job retention count.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 
> aa45d27391b1786ca3d5e8c928045f1b6f3cf5ef 
>   src/main/java/org/apache/aurora/scheduler/async/HistoryPruner.java 
> ebae58a04e8857c5f26d4b57c27dfcda9e14c82c 
>   src/main/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPruner.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
> c3abffe575e801cebec3572cf4aceac83a238b55 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 
> 3db0114c5432a5b7d0b01d97c75494be9e3e99a3 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java
>  c583e085e0458835d51ebf740a3b5f01b428bb25 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 
> 66c91644677e7176ccf53dcfcf29a6792ec398bc 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
>  2a81a94f99f242bbe400d8428ad1f1ce38a06a86 
>   src/main/thrift/org/apache/aurora/gen/storage.thrift 
> 7e502450f06abb449d06af09cc59185c6a9a2963 
>   src/test/java/org/apache/aurora/scheduler/async/HistoryPrunerTest.java 
> 011d9ec5d84c658c2b2e39bb0b3f9d20b5440cee 
>   
> src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java
>  PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java
>  dbf0badbfcc19f40d9b9eeec22b7024d95a02884 
>   src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
> 68df0d542e41438c0844f76fc5b9ec6996a00e8d 
>   
> src/test/java/org/apache/aurora/scheduler/updater/FakeScheduledExecutor.java 
> e35fe23f023f5accfb2270a3634c91bec657 
>   src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 
> 6758b7b56e77882c67be2e39481ff76893ad1610 
> 
> Diff: https://reviews.apache.org/r/26232/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 26298: Use a less broad retry loop for RPCs.

[Maxim Khutornenko]

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


| I also changed the test to avoid sleeping, which was eating a fixed 10 
seconds of unit test time.
Not really sure why it takes that long for you:

$ time ./pants src/test/python/apache/aurora/client/api:scheduler_client -k 
test_transient_error
...
real0m3.602s
user0m2.679s
sys 0m0.696s

- Maxim Khutornenko


On Oct. 3, 2014, 12:05 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26298/
> ---
> 
> (Updated Oct. 3, 2014, 12:05 a.m.)
> 
> 
> Review request for Aurora, Mark Chu-Carroll and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> A few times when making changes, i've found myself confused at a stalled test 
> and spiked CPU, only to find that my test should have failed, but an 
> exception is trapped in this retry loop.  The key change here is that unknown 
> exceptions will break the loop.
> 
> Making this change pointed out what should have been a test failure in 
> `test_transient_error`, where an exception caused by an unexpected call to 
> `getVersion` was swallowed.
> 
> I also changed the test to avoid sleeping, which was eating a fixed 10 
> seconds of unit test time.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/api/scheduler_client.py 
> b400cb2dbdb35077fc2c4a6e161c2959a9217317 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 
> 1cbfbf86e903d890baac7d34461109f9beaff442 
> 
> Diff: https://reviews.apache.org/r/26298/diff/
> 
> 
> Testing
> ---
> 
> ./pants src/test/python:all -vxs
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 26298: Use a less broad retry loop for RPCs.

[Bill Farner]

> On Oct. 3, 2014, 12:13 a.m., Maxim Khutornenko wrote:
> > | I also changed the test to avoid sleeping, which was eating a fixed 10 
> > seconds of unit test time.
> > Not really sure why it takes that long for you:
> > 
> > $ time ./pants src/test/python/apache/aurora/client/api:scheduler_client -k 
> > test_transient_error
> > ...
> > real0m3.602s
> > user0m2.679s
> > sys 0m0.696s

You're right, my own change added another sleep when a transient error is 
encountered:

self._terminating.wait(self.RPC_RETRY_INTERVAL.as_(Time.SECONDS))

That's what was causing the delay.  It's the right behavior to do this, rather 
than a tight retry loop, but you're correct that this change is not speeding up 
our tests.


- Bill


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


On Oct. 3, 2014, 12:05 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26298/
> ---
> 
> (Updated Oct. 3, 2014, 12:05 a.m.)
> 
> 
> Review request for Aurora, Mark Chu-Carroll and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> A few times when making changes, i've found myself confused at a stalled test 
> and spiked CPU, only to find that my test should have failed, but an 
> exception is trapped in this retry loop.  The key change here is that unknown 
> exceptions will break the loop.
> 
> Making this change pointed out what should have been a test failure in 
> `test_transient_error`, where an exception caused by an unexpected call to 
> `getVersion` was swallowed.
> 
> I also changed the test to avoid sleeping, which was eating a fixed 10 
> seconds of unit test time.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/api/scheduler_client.py 
> b400cb2dbdb35077fc2c4a6e161c2959a9217317 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 
> 1cbfbf86e903d890baac7d34461109f9beaff442 
> 
> Diff: https://reviews.apache.org/r/26298/diff/
> 
> 
> Testing
> ---
> 
> ./pants src/test/python:all -vxs
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 26298: Use a less broad retry loop for RPCs.

[Bill Farner]

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

(Updated Oct. 3, 2014, 12:18 a.m.)


Review request for Aurora, Mark Chu-Carroll and Maxim Khutornenko.


Changes
---

Corrected description.


Repository: aurora


Description (updated)
---

A few times when making changes, i've found myself confused at a stalled test 
and spiked CPU, only to find that my test should have failed, but an exception 
is trapped in this retry loop.  The key change here is that unknown exceptions 
will break the loop.

Making this change pointed out what should have been a test failure in 
`test_transient_error`, where an exception caused by an unexpected call to 
`getVersion` was swallowed.


Diffs
-

  src/main/python/apache/aurora/client/api/scheduler_client.py 
b400cb2dbdb35077fc2c4a6e161c2959a9217317 
  src/test/python/apache/aurora/client/api/test_scheduler_client.py 
1cbfbf86e903d890baac7d34461109f9beaff442 

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


Testing
---

./pants src/test/python:all -vxs


Thanks,

Bill Farner

Re: Review Request 26244: Add a doc about dedicated hosts.

[Bill Farner]

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


Kevin - ping?

- Bill Farner


On Oct. 2, 2014, 2:25 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26244/
> ---
> 
> (Updated Oct. 2, 2014, 2:25 a.m.)
> 
> 
> Review request for Aurora and Kevin Sweeney.
> 
> 
> Bugs: AURORA-703
> https://issues.apache.org/jira/browse/AURORA-703
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add a doc about dedicated hosts.
> 
> 
> Diffs
> -
> 
>   docs/deploying-aurora-scheduler.md 93ff746038df14f2abeea85acf48d02b217af522 
> 
> Diff: https://reviews.apache.org/r/26244/diff/
> 
> 
> Testing
> ---
> 
> Rendered here: 
> https://github.com/wfarner/incubator-aurora/blob/wfarner/doc_dedicated_machines/docs/deploying-aurora-scheduler.md
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 26298: Use a less broad retry loop for RPCs.

[Maxim Khutornenko]

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



src/main/python/apache/aurora/client/api/scheduler_client.py


Why not just add a break into the Exception catcher instead?


- Maxim Khutornenko


On Oct. 3, 2014, 12:18 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26298/
> ---
> 
> (Updated Oct. 3, 2014, 12:18 a.m.)
> 
> 
> Review request for Aurora, Mark Chu-Carroll and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> A few times when making changes, i've found myself confused at a stalled test 
> and spiked CPU, only to find that my test should have failed, but an 
> exception is trapped in this retry loop.  The key change here is that unknown 
> exceptions will break the loop.
> 
> Making this change pointed out what should have been a test failure in 
> `test_transient_error`, where an exception caused by an unexpected call to 
> `getVersion` was swallowed.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/api/scheduler_client.py 
> b400cb2dbdb35077fc2c4a6e161c2959a9217317 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 
> 1cbfbf86e903d890baac7d34461109f9beaff442 
> 
> Diff: https://reviews.apache.org/r/26298/diff/
> 
> 
> Testing
> ---
> 
> ./pants src/test/python:all -vxs
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 26244: Add a doc about dedicated hosts.

[Kevin Sweeney]

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

Ship it!


Ship It!

- Kevin Sweeney


On Oct. 1, 2014, 7:25 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26244/
> ---
> 
> (Updated Oct. 1, 2014, 7:25 p.m.)
> 
> 
> Review request for Aurora and Kevin Sweeney.
> 
> 
> Bugs: AURORA-703
> https://issues.apache.org/jira/browse/AURORA-703
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add a doc about dedicated hosts.
> 
> 
> Diffs
> -
> 
>   docs/deploying-aurora-scheduler.md 93ff746038df14f2abeea85acf48d02b217af522 
> 
> Diff: https://reviews.apache.org/r/26244/diff/
> 
> 
> Testing
> ---
> 
> Rendered here: 
> https://github.com/wfarner/incubator-aurora/blob/wfarner/doc_dedicated_machines/docs/deploying-aurora-scheduler.md
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 26298: Use a less broad retry loop for RPCs.

[Maxim Khutornenko]

> On Oct. 3, 2014, 12:21 a.m., Maxim Khutornenko wrote:
> > src/main/python/apache/aurora/client/api/scheduler_client.py, line 286
> > 
> >
> > Why not just add a break into the Exception catcher instead?

Actually, we are throwing there already. Wait, how is the exception is trapped 
in that loop?


- Maxim


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


On Oct. 3, 2014, 12:18 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26298/
> ---
> 
> (Updated Oct. 3, 2014, 12:18 a.m.)
> 
> 
> Review request for Aurora, Mark Chu-Carroll and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> A few times when making changes, i've found myself confused at a stalled test 
> and spiked CPU, only to find that my test should have failed, but an 
> exception is trapped in this retry loop.  The key change here is that unknown 
> exceptions will break the loop.
> 
> Making this change pointed out what should have been a test failure in 
> `test_transient_error`, where an exception caused by an unexpected call to 
> `getVersion` was swallowed.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/api/scheduler_client.py 
> b400cb2dbdb35077fc2c4a6e161c2959a9217317 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 
> 1cbfbf86e903d890baac7d34461109f9beaff442 
> 
> Diff: https://reviews.apache.org/r/26298/diff/
> 
> 
> Testing
> ---
> 
> ./pants src/test/python:all -vxs
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 26300: Sleep 10sec instead of 5sec during GC shutdown

[Brian Wickman]

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

Ship it!


http://media.giphy.com/media/7LG6PqAubrWBa/giphy.gif

- Brian Wickman


On Oct. 3, 2014, 12:03 a.m., Kevin Sweeney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26300/
> ---
> 
> (Updated Oct. 3, 2014, 12:03 a.m.)
> 
> 
> Review request for Aurora, Vinod Kone and Brian Wickman.
> 
> 
> Bugs: AURORA-788
> https://issues.apache.org/jira/browse/AURORA-788
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Sleep 10sec instead of 5sec during GC shutdown
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/executor/gc_executor.py 
> 44eb0da984a126536f0d277da3da128089201a47 
> 
> Diff: https://reviews.apache.org/r/26300/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kevin Sweeney
> 
>

Re: Review Request 25974: Prevent initial ZK timeouts from killing the executor.

[Brian Wickman]

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



src/test/python/apache/aurora/executor/common/test_announcer.py


I'm confused -- nobody calls client.connected anymore, right?  In theory, 
this test will pass, except it will take 30-60 seconds to run since 
client_connect_event.wait(timeout=...) will be called with the health check 
timeout.


- Brian Wickman


On Oct. 2, 2014, 11:20 p.m., Zameer Manji wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25974/
> ---
> 
> (Updated Oct. 2, 2014, 11:20 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney, Bill Farner, and Brian Wickman.
> 
> 
> Bugs: AURORA-728
> https://issues.apache.org/jira/browse/AURORA-728
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Prevent initial ZK timeouts from killing the executor. In addition, prevent 
> uncaught exceptions from killing the executor.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/executor/aurora_executor.py 
> 79a24855b2a68271b7478395dfdadab8755c3af2 
>   src/main/python/apache/aurora/executor/common/announcer.py 
> c466da8d48bbc2aa227c2db157cab84665ad6602 
>   src/test/python/apache/aurora/executor/common/test_announcer.py 
> 4f6e200ecb1a4ea7cb45acd466a57f19d5815326 
> 
> Diff: https://reviews.apache.org/r/25974/diff/
> 
> 
> Testing
> ---
> 
> ./pants src/test/python/apache/aurora/executor:executor-small
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Zameer Manji
> 
>

Re: Review Request 25974: Prevent initial ZK timeouts from killing the executor.

[Zameer Manji]

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

(Updated Oct. 2, 2014, 6:44 p.m.)


Review request for Aurora, Kevin Sweeney, Bill Farner, and Brian Wickman.


Changes
---

Brian's feedback.


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


Repository: aurora


Description
---

Prevent initial ZK timeouts from killing the executor. In addition, prevent 
uncaught exceptions from killing the executor.


Diffs (updated)
-

  src/main/python/apache/aurora/executor/aurora_executor.py 
79a24855b2a68271b7478395dfdadab8755c3af2 
  src/main/python/apache/aurora/executor/common/announcer.py 
c466da8d48bbc2aa227c2db157cab84665ad6602 
  src/test/python/apache/aurora/executor/common/test_announcer.py 
4f6e200ecb1a4ea7cb45acd466a57f19d5815326 

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


Testing
---

./pants src/test/python/apache/aurora/executor:executor-small
./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh


Thanks,

Zameer Manji

Re: Review Request 26300: Sleep 10sec instead of 5sec during GC shutdown

[Kevin Sweeney]

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


Upon discussion with Vinod Kone and Ben Mahler this doesn't seem like the right 
approach - driver.stop() doesn't actually send a signal to the slave that it 
should not expect further communications from the executor, so the race will 
always exist.

Upon further dissection of this code I'm questioning the need to quit after 15 
minutes of inactivity. Updated patch incoming.

- Kevin Sweeney


On Oct. 2, 2014, 5:03 p.m., Kevin Sweeney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26300/
> ---
> 
> (Updated Oct. 2, 2014, 5:03 p.m.)
> 
> 
> Review request for Aurora, Vinod Kone and Brian Wickman.
> 
> 
> Bugs: AURORA-788
> https://issues.apache.org/jira/browse/AURORA-788
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Sleep 10sec instead of 5sec during GC shutdown
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/executor/gc_executor.py 
> 44eb0da984a126536f0d277da3da128089201a47 
> 
> Diff: https://reviews.apache.org/r/26300/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Kevin Sweeney
> 
>

Re: Review Request 25974: Prevent initial ZK timeouts from killing the executor.

[Brian Wickman]

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

Ship it!


Ship It!

- Brian Wickman


On Oct. 3, 2014, 1:44 a.m., Zameer Manji wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25974/
> ---
> 
> (Updated Oct. 3, 2014, 1:44 a.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney, Bill Farner, and Brian Wickman.
> 
> 
> Bugs: AURORA-728
> https://issues.apache.org/jira/browse/AURORA-728
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Prevent initial ZK timeouts from killing the executor. In addition, prevent 
> uncaught exceptions from killing the executor.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/executor/aurora_executor.py 
> 79a24855b2a68271b7478395dfdadab8755c3af2 
>   src/main/python/apache/aurora/executor/common/announcer.py 
> c466da8d48bbc2aa227c2db157cab84665ad6602 
>   src/test/python/apache/aurora/executor/common/test_announcer.py 
> 4f6e200ecb1a4ea7cb45acd466a57f19d5815326 
> 
> Diff: https://reviews.apache.org/r/25974/diff/
> 
> 
> Testing
> ---
> 
> ./pants src/test/python/apache/aurora/executor:executor-small
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Zameer Manji
> 
>

Re: Review Request 26300: Don't kill GC Executor after period of inactivity

[Kevin Sweeney]

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

(Updated Oct. 2, 2014, 6:59 p.m.)


Review request for Aurora, Ben Mahler, Vinod Kone, and Brian Wickman.


Changes
---

update patch and description, +bmahler


Summary (updated)
-

Don't kill GC Executor after period of inactivity


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


Repository: aurora


Description (updated)
---

The GC executor is configured to exit after 15 minutes of inactivity. This 
leads to a race where the mesos slave gets a launchTask message for a GC 
executor just as the executor has exited, causing TASK_LOST noise. This also 
increases the risk that a slave will lose its GC executor due to the scheduler 
not being able to find a slot for it (since GC executors will have a higher 
churn rate).

Cluster operators will still be able to deploy new versions of the GC executor 
as the 24-hour max lifetime limit is still in place. This patch only removes 
the inactivity limit.


Diffs (updated)
-

  src/main/python/apache/aurora/executor/gc_executor.py 
44eb0da984a126536f0d277da3da128089201a47 
  src/test/python/apache/aurora/executor/test_gc_executor.py 
774c9ba0d5c31fc4c46dbe257579e013460fa943 

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


Testing
---


Thanks,

Kevin Sweeney

Re: Review Request 26300: Don't kill GC Executor after period of inactivity

[Kevin Sweeney]

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

(Updated Oct. 2, 2014, 6:59 p.m.)


Review request for Aurora, Ben Mahler, Vinod Kone, and Brian Wickman.


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


Repository: aurora


Description
---

The GC executor is configured to exit after 15 minutes of inactivity. This 
leads to a race where the mesos slave gets a launchTask message for a GC 
executor just as the executor has exited, causing TASK_LOST noise. This also 
increases the risk that a slave will lose its GC executor due to the scheduler 
not being able to find a slot for it (since GC executors will have a higher 
churn rate).

Cluster operators will still be able to deploy new versions of the GC executor 
as the 24-hour max lifetime limit is still in place. This patch only removes 
the inactivity limit.


Diffs
-

  src/main/python/apache/aurora/executor/gc_executor.py 
44eb0da984a126536f0d277da3da128089201a47 
  src/test/python/apache/aurora/executor/test_gc_executor.py 
774c9ba0d5c31fc4c46dbe257579e013460fa943 

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


Testing (updated)
---

./pants src/test/python/apache/aurora/executor:gc_executor


Thanks,

Kevin Sweeney

Review Request 26308: Fix exit condition for RPC loop, fix test_status_api_failure test.

[Bill Farner]

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

Review request for Aurora, Kevin Sweeney and Mark Chu-Carroll.


Repository: aurora


Description
---

Fixes two problems:

- mocking was incorrect in `test_status_api_failure`.  Turns out that Mock 
objects were being passed around and somehow resulted in the test case passing.
- use of `threading.Event()` was broken in scheduler_client.py.  I don't think 
it's possible to enter those branches.


Diffs
-

  src/main/python/apache/aurora/client/api/scheduler_client.py 
b400cb2dbdb35077fc2c4a6e161c2959a9217317 
  src/test/python/apache/aurora/client/api/test_scheduler_client.py 
1cbfbf86e903d890baac7d34461109f9beaff442 
  src/test/python/apache/aurora/client/cli/test_api_from_cli.py 
78f21d2f20cf71fa2dfe0614885d44d2948decd2 

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


Testing
---

./pants src/test/python:all -vxs


Thanks,

Bill Farner

Re: Review Request 26298: Use a less broad retry loop for RPCs.

[Bill Farner]

> On Oct. 3, 2014, 12:21 a.m., Maxim Khutornenko wrote:
> > src/main/python/apache/aurora/client/api/scheduler_client.py, line 286
> > 
> >
> > Why not just add a break into the Exception catcher instead?
> 
> Maxim Khutornenko wrote:
> Actually, we are throwing there already. Wait, how is the exception is 
> trapped in that loop?

I scratched my head at this for a while, and wound up with this review: 
https://reviews.apache.org/r/26308/

AFAICT `threading.Event` does not override `__bool__`, so those branches are 
never entered, and we loop until the timer is up.


- Bill


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


On Oct. 3, 2014, 12:18 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26298/
> ---
> 
> (Updated Oct. 3, 2014, 12:18 a.m.)
> 
> 
> Review request for Aurora, Mark Chu-Carroll and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> A few times when making changes, i've found myself confused at a stalled test 
> and spiked CPU, only to find that my test should have failed, but an 
> exception is trapped in this retry loop.  The key change here is that unknown 
> exceptions will break the loop.
> 
> Making this change pointed out what should have been a test failure in 
> `test_transient_error`, where an exception caused by an unexpected call to 
> `getVersion` was swallowed.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/api/scheduler_client.py 
> b400cb2dbdb35077fc2c4a6e161c2959a9217317 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 
> 1cbfbf86e903d890baac7d34461109f9beaff442 
> 
> Diff: https://reviews.apache.org/r/26298/diff/
> 
> 
> Testing
> ---
> 
> ./pants src/test/python:all -vxs
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 26308: Fix exit condition for RPC loop, fix test_status_api_failure test.

[Bill Farner]

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

(Updated Oct. 3, 2014, 2:38 a.m.)


Review request for Aurora, Kevin Sweeney and Mark Chu-Carroll.


Changes
---

Missed one.


Repository: aurora


Description
---

Fixes two problems:

- mocking was incorrect in `test_status_api_failure`.  Turns out that Mock 
objects were being passed around and somehow resulted in the test case passing.
- use of `threading.Event()` was broken in scheduler_client.py.  I don't think 
it's possible to enter those branches.


Diffs (updated)
-

  src/main/python/apache/aurora/client/api/scheduler_client.py 
b400cb2dbdb35077fc2c4a6e161c2959a9217317 
  src/test/python/apache/aurora/client/api/test_scheduler_client.py 
1cbfbf86e903d890baac7d34461109f9beaff442 
  src/test/python/apache/aurora/client/cli/test_api_from_cli.py 
78f21d2f20cf71fa2dfe0614885d44d2948decd2 

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


Testing
---

./pants src/test/python:all -vxs


Thanks,

Bill Farner

Re: Review Request 26300: Don't kill GC Executor after period of inactivity

[Ben Mahler]

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


Thanks Kevin! Looks good to me, will defer to wickman for a Ship It.

- Ben Mahler


On Oct. 3, 2014, 1:59 a.m., Kevin Sweeney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26300/
> ---
> 
> (Updated Oct. 3, 2014, 1:59 a.m.)
> 
> 
> Review request for Aurora, Ben Mahler, Vinod Kone, and Brian Wickman.
> 
> 
> Bugs: AURORA-788
> https://issues.apache.org/jira/browse/AURORA-788
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The GC executor is configured to exit after 15 minutes of inactivity. This 
> leads to a race where the mesos slave gets a launchTask message for a GC 
> executor just as the executor has exited, causing TASK_LOST noise. This also 
> increases the risk that a slave will lose its GC executor due to the 
> scheduler not being able to find a slot for it (since GC executors will have 
> a higher churn rate).
> 
> Cluster operators will still be able to deploy new versions of the GC 
> executor as the 24-hour max lifetime limit is still in place. This patch only 
> removes the inactivity limit.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/executor/gc_executor.py 
> 44eb0da984a126536f0d277da3da128089201a47 
>   src/test/python/apache/aurora/executor/test_gc_executor.py 
> 774c9ba0d5c31fc4c46dbe257579e013460fa943 
> 
> Diff: https://reviews.apache.org/r/26300/diff/
> 
> 
> Testing
> ---
> 
> ./pants src/test/python/apache/aurora/executor:gc_executor
> 
> 
> Thanks,
> 
> Kevin Sweeney
> 
>

Re: Review Request 26308: Fix exit condition for RPC loop, fix test_status_api_failure test.

[Joe Smith]

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



src/test/python/apache/aurora/client/cli/test_api_from_cli.py


you need to set a spec here

Mock(SchedulerClient)


- Joe Smith


On Oct. 2, 2014, 7:38 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26308/
> ---
> 
> (Updated Oct. 2, 2014, 7:38 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Mark Chu-Carroll.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Fixes two problems:
> 
> - mocking was incorrect in `test_status_api_failure`.  Turns out that Mock 
> objects were being passed around and somehow resulted in the test case 
> passing.
> - use of `threading.Event()` was broken in scheduler_client.py.  I don't 
> think it's possible to enter those branches.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/api/scheduler_client.py 
> b400cb2dbdb35077fc2c4a6e161c2959a9217317 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 
> 1cbfbf86e903d890baac7d34461109f9beaff442 
>   src/test/python/apache/aurora/client/cli/test_api_from_cli.py 
> 78f21d2f20cf71fa2dfe0614885d44d2948decd2 
> 
> Diff: https://reviews.apache.org/r/26308/diff/
> 
> 
> Testing
> ---
> 
> ./pants src/test/python:all -vxs
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 26232: Implementing update history pruner.

[David McLaughlin]

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

Ship it!


Ship It!

- David McLaughlin


On Oct. 2, 2014, 10:51 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26232/
> ---
> 
> (Updated Oct. 2, 2014, 10:51 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Bill Farner.
> 
> 
> Bugs: AURORA-743
> https://issues.apache.org/jira/browse/AURORA-743
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The pruner runs on periodic basis and trims completed updates up to the 
> guranteed per job retention count.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 
> aa45d27391b1786ca3d5e8c928045f1b6f3cf5ef 
>   src/main/java/org/apache/aurora/scheduler/async/HistoryPruner.java 
> ebae58a04e8857c5f26d4b57c27dfcda9e14c82c 
>   src/main/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPruner.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
> c3abffe575e801cebec3572cf4aceac83a238b55 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 
> 3db0114c5432a5b7d0b01d97c75494be9e3e99a3 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java
>  c583e085e0458835d51ebf740a3b5f01b428bb25 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 
> 66c91644677e7176ccf53dcfcf29a6792ec398bc 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
>  2a81a94f99f242bbe400d8428ad1f1ce38a06a86 
>   src/main/thrift/org/apache/aurora/gen/storage.thrift 
> 7e502450f06abb449d06af09cc59185c6a9a2963 
>   src/test/java/org/apache/aurora/scheduler/async/HistoryPrunerTest.java 
> 011d9ec5d84c658c2b2e39bb0b3f9d20b5440cee 
>   
> src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java
>  PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java
>  dbf0badbfcc19f40d9b9eeec22b7024d95a02884 
>   src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
> 68df0d542e41438c0844f76fc5b9ec6996a00e8d 
>   
> src/test/java/org/apache/aurora/scheduler/updater/FakeScheduledExecutor.java 
> e35fe23f023f5accfb2270a3634c91bec657 
>   src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 
> 6758b7b56e77882c67be2e39481ff76893ad1610 
> 
> Diff: https://reviews.apache.org/r/26232/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 26308: Fix exit condition for RPC loop, fix test_status_api_failure test.

[Bill Farner]

> On Oct. 3, 2014, 2:52 a.m., Joe Smith wrote:
> > src/test/python/apache/aurora/client/cli/test_api_from_cli.py, line 133
> > 
> >
> > you need to set a spec here
> > 
> > Mock(SchedulerClient)

Thanks for the nudge!  Went ahead and cleared out all the unspecified mocks.


- Bill


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


On Oct. 3, 2014, 2:38 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26308/
> ---
> 
> (Updated Oct. 3, 2014, 2:38 a.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Mark Chu-Carroll.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Fixes two problems:
> 
> - mocking was incorrect in `test_status_api_failure`.  Turns out that Mock 
> objects were being passed around and somehow resulted in the test case 
> passing.
> - use of `threading.Event()` was broken in scheduler_client.py.  I don't 
> think it's possible to enter those branches.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/api/scheduler_client.py 
> b400cb2dbdb35077fc2c4a6e161c2959a9217317 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 
> 1cbfbf86e903d890baac7d34461109f9beaff442 
>   src/test/python/apache/aurora/client/cli/test_api_from_cli.py 
> 78f21d2f20cf71fa2dfe0614885d44d2948decd2 
> 
> Diff: https://reviews.apache.org/r/26308/diff/
> 
> 
> Testing
> ---
> 
> ./pants src/test/python:all -vxs
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 26308: Fix exit condition for RPC loop, fix test_status_api_failure test.

[Bill Farner]

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

(Updated Oct. 3, 2014, 4:52 a.m.)


Review request for Aurora, Kevin Sweeney and Mark Chu-Carroll.


Repository: aurora


Description
---

Fixes two problems:

- mocking was incorrect in `test_status_api_failure`.  Turns out that Mock 
objects were being passed around and somehow resulted in the test case passing.
- use of `threading.Event()` was broken in scheduler_client.py.  I don't think 
it's possible to enter those branches.


Diffs (updated)
-

  src/main/python/apache/aurora/client/api/scheduler_client.py 
b400cb2dbdb35077fc2c4a6e161c2959a9217317 
  src/test/python/apache/aurora/client/api/test_scheduler_client.py 
1cbfbf86e903d890baac7d34461109f9beaff442 
  src/test/python/apache/aurora/client/cli/test_api_from_cli.py 
78f21d2f20cf71fa2dfe0614885d44d2948decd2 

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


Testing
---

./pants src/test/python:all -vxs


Thanks,

Bill Farner

Re: Review Request 26300: Don't kill GC Executor after period of inactivity

[Vinod Kone]

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


lgtm. i'll let wickman give a shipit.

- Vinod Kone


On Oct. 3, 2014, 1:59 a.m., Kevin Sweeney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26300/
> ---
> 
> (Updated Oct. 3, 2014, 1:59 a.m.)
> 
> 
> Review request for Aurora, Ben Mahler, Vinod Kone, and Brian Wickman.
> 
> 
> Bugs: AURORA-788
> https://issues.apache.org/jira/browse/AURORA-788
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The GC executor is configured to exit after 15 minutes of inactivity. This 
> leads to a race where the mesos slave gets a launchTask message for a GC 
> executor just as the executor has exited, causing TASK_LOST noise. This also 
> increases the risk that a slave will lose its GC executor due to the 
> scheduler not being able to find a slot for it (since GC executors will have 
> a higher churn rate).
> 
> Cluster operators will still be able to deploy new versions of the GC 
> executor as the 24-hour max lifetime limit is still in place. This patch only 
> removes the inactivity limit.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/executor/gc_executor.py 
> 44eb0da984a126536f0d277da3da128089201a47 
>   src/test/python/apache/aurora/executor/test_gc_executor.py 
> 774c9ba0d5c31fc4c46dbe257579e013460fa943 
> 
> Diff: https://reviews.apache.org/r/26300/diff/
> 
> 
> Testing
> ---
> 
> ./pants src/test/python/apache/aurora/executor:gc_executor
> 
> 
> Thanks,
> 
> Kevin Sweeney
> 
>