Re: Review Request 53003: Adding logic to copy network files when using the Mesos containierizer with a Docker image.

2016-10-18 Thread Joshua Cohen
uler's `-global_container_mounts` command line flag, rather than doing this for everyone where it may not be necessary/desirable? Alternately, I'm not super familiar w/ CNI, but is it possible to infer from the TaskInfo whether CNI is enabled (e.g. is NetworkInfo set somewhere)? - J

Re: Review Request 53003: Adding logic to copy network files when using the Mesos containierizer with a Docker image.

2016-10-18 Thread Joshua Cohen
> On Oct. 18, 2016, 11:59 p.m., Joshua Cohen wrote: > > src/main/python/apache/aurora/executor/common/sandbox.py, lines 308-313 > > <https://reviews.apache.org/r/53003/diff/2/?file=1541037#file1541037line308> > > > > Is this always necessary, or only ne

Re: Review Request 53003: Adding logic to copy network files when using the Mesos containierizer with a Docker image.

2016-10-19 Thread Joshua Cohen
> On Oct. 18, 2016, 11:59 p.m., Joshua Cohen wrote: > > src/main/python/apache/aurora/executor/common/sandbox.py, lines 308-313 > > <https://reviews.apache.org/r/53003/diff/2/?file=1541037#file1541037line308> > > > > Is this always necessary, or only ne

Re: Review Request 53003: Adding logic to copy network files when using the Mesos containierizer with a Docker image.

2016-10-19 Thread Joshua Cohen
this. Thanks for bearing with me while I sorted out whether there were any unforseen issues with this approach! - Joshua Cohen On Oct. 18, 2016, 11:41 p.m., Justin Pinkul wrote: > > --- > This is an automatically generated e-mail. T

Review Request 53064: Update provisioning scripts for testing aurora packages.

2016-10-20 Thread Joshua Cohen
b9807ef63f73e985adaae6583023c4f5e762c6aa Diff: https://reviews.apache.org/r/53064/diff/ Testing --- Verified the vagrant environments provision successfully w/ these changes. Thanks, Joshua Cohen

Re: Review Request 53064: Update provisioning scripts for testing aurora packages.

2016-10-20 Thread Joshua Cohen
e can use that to verify the 0.16.0 packages. After the vote succeeds I'll merge that branch to master. - Joshua Cohen On Oct. 20, 2016, 3:33 p.m., Joshua Cohen wrote: > > --- > This is an automatically generated e-ma

Review Request 53102: Clean up README for packaging repo, update release-candidate script to generate the vote email.

2016-10-21 Thread Joshua Cohen
/bintray-upload.png 5fd92961d07f14e4e14c32c4aae7e293366ccd6c Diff: https://reviews.apache.org/r/53102/diff/ Testing --- Thanks, Joshua Cohen

Re: Review Request 53102: Clean up README for packaging repo, update release-candidate script to generate the vote email.

2016-10-21 Thread Joshua Cohen
e Whoops, good catch. - Joshua --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53102/#review153603 --- On Oct. 21, 20

Re: Review Request 53102: Clean up README for packaging repo, update release-candidate script to generate the vote email.

2016-10-22 Thread Joshua Cohen
build-support/release/release-candidate c08c88529ec036989032869198da7a21dcf6ac35 docs/images/bintray-upload.png 5fd92961d07f14e4e14c32c4aae7e293366ccd6c Diff: https://reviews.apache.org/r/53102/diff/ Testing --- Thanks, Joshua Cohen

Re: Review Request 53131: Re-introduce --executor_registration_timeout.

2016-10-24 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53131/#review153689 --- Ship it! Ship It! - Joshua Cohen On Oct. 24, 2016, 12:39

Re: Review Request 53114: aurora job inspect should have a --write-json option

2016-10-24 Thread Joshua Cohen
nder (which has the slight benefit of handling indentation if the value is multi-line)? src/main/python/apache/aurora/client/cli/jobs.py (line 291) <https://reviews.apache.org/r/53114/#comment223119> We should fix this so it doesn't render `` if no contact is set. - Joshua Cohen

Re: Review Request 52479: Resolve docker tags to concrete identifiers for DockerContainerizer

2016-10-24 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/52479/#review153701 --- Ship it! Ship It! - Joshua Cohen On Oct. 24, 2016, 5:14 a.m

Re: Review Request 53258: Added short example to documentation how to use .thermos_profile

2016-10-31 Thread Joshua Cohen
s done whenever a process is forked. (Not necessary for the doc, but for the sake of completeness, the file, if it exists is set to `BASH_ENV` in the environment of the forked process (whose cmdline itself is wrapped in a `bash -c` invocation)). - Joshua Cohen On Oct. 28, 2016, 10:

Re: Review Request 53258: Added short example to documentation how to use .thermos_profile

2016-10-31 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53258/#review154285 --- Ship it! Ship It! - Joshua Cohen On Oct. 31, 2016, 5:38 p.m

Re: Review Request 53333: Add DSL and E2E changes for per task volume mounts.

2016-10-31 Thread Joshua Cohen
be a separate process in the e2e test job, rather than embedding it in `run-server`? That way it's clear that this check is being performed. - Joshua Cohen On Nov. 1, 2016, 1:48 a.m., Zameer Manji wrote: > > --- > This is a

Re: Review Request 53403: [WIP] [DO NOT MERGE] Reparent orphaned processes to thermos runner and send SIGTERM on task teardown

2016-11-02 Thread Joshua Cohen
is maybe more pythonic (assuming I got my syntax right anyway...) src/main/python/apache/thermos/core/helper.py (line 242) <https://reviews.apache.org/r/53403/#comment224297> I think this would probably make sense as an info log. - Joshua Cohen On Nov. 2, 2016, 10:16 p.m.,

Re: Review Request 52479: Resolve docker tags to concrete identifiers for DockerContainerizer

2016-11-02 Thread Joshua Cohen
s://reviews.apache.org/r/52479/ > --- > > (Updated Nov. 2, 2016, 5:44 a.m.) > > > Review request for Aurora, George Sirois, Joshua Cohen, Stephan Erb, and > Zameer Manji. > > > Bugs: AURORA-1014 > https://issue

Re: Review Request 53418: Send SIGTERM to daemonized processes on shutdown.

2016-11-03 Thread Joshua Cohen
s.apache.org/r/53418/#comment224405> I think we can skip these two tests, they're not relevant to scenario we're testing, right? - Joshua Cohen On Nov. 3, 2016, 2:53 a.m., Zameer Manji wrote: > > --- > This is an autom

Re: Review Request 53418: Send SIGTERM to daemonized processes on shutdown.

2016-11-03 Thread Joshua Cohen
Nov. 3, 2016, 2:53 a.m., Zameer Manji wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/53418/ > --- > > (Updated N

Re: Review Request 53418: Send SIGTERM to daemonized processes on shutdown.

2016-11-04 Thread Joshua Cohen
t: use `log.error` here instead of `self._log`. I want to kill that `self._log` function at some point because the logs always point to the same line in the code rather than where they actually originated. - Joshua Cohen On Nov. 3, 2016, 11:48 p.m., Z

Re: Review Request 53418: Send SIGTERM to daemonized processes on shutdown.

2016-11-04 Thread Joshua Cohen
On Nov. 3, 2016, 11:48 p.m., Zameer Manji wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/53418/ > --- > > (Updated Nov. 3, 2016, 11:48 p.m.) > > > Rev

Re: Review Request 53508: Fix regression in 5410c22.

2016-11-04 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53508/#review155038 --- Ship it! Ship It! - Joshua Cohen On Nov. 5, 2016, 12:53 a.m

Re: Review Request 53590: Change job updates to rely on `health-checks` rather than on `watch_secs`.

2016-11-09 Thread Joshua Cohen
che/aurora/executor/common/health_checker.py (lines 164 - 168) <https://reviews.apache.org/r/53590/#comment225403> Can you add a comment here explaining why this is safe vis-a-vis not having a potentially dangling health check outside of the interval that could s

Review Request 53620: Update vagrant to a working basebox.

2016-11-09 Thread Joshua Cohen
Description --- Update vagrant to a working basebox. Diffs - Vagrantfile 7b05e20b6288cf14674a68511e20b93c7cc57dd2 Diff: https://reviews.apache.org/r/53620/diff/ Testing --- Uploaded to atlas and ran e2e tests locally against this version. Thanks, Joshua Cohen

Re: Review Request 53114: aurora job inspect should have a --write-json option

2016-11-15 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53114/#review155928 --- Ship it! Ship It! - Joshua Cohen On Nov. 11, 2016, 8:16 p.m

Re: Review Request 53590: Change job updates to rely on `health-checks` rather than on `watch_secs`.

2016-11-15 Thread Joshua Cohen
or a long lived service, is there any downside to continuing to increment attempts after the callback has been dispatched? - Joshua Cohen On Nov. 14, 2016, 5:09 a.m., Santhosh Kumar Shanmugham wrote: > > -

Re: Review Request 53687: track percentile values for timed sliding stats. AURORA-118

2016-11-15 Thread Joshua Cohen
o begin with, as I agree with Zameer, the reason for making this optional is non-obvious, so perhaps it can just go away completely? - Joshua Cohen On Nov. 12, 2016, 1:49 a.m., Reza Motamedi wrote: > > --- > This i

Re: Review Request 53590: Change job updates to rely on `health-checks` rather than on `watch_secs`.

2016-11-16 Thread Joshua Cohen
). Drop the "trivially"? (I.e. "No health-check defined, task is assumed healthy.") - Joshua Cohen On Nov. 16, 2016, 8:23 a.m., Santhosh Kumar Shanmugham wrote: > > --- > This is an automatically generated e-

Re: Review Request 53796: Bump guava dependency to 20.

2016-11-16 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53796/#review156076 --- Ship it! Ship It! - Joshua Cohen On Nov. 15, 2016, 10:09

Re: Review Request 53590: Change job updates to rely on `health-checks` rather than on `watch_secs`.

2016-11-16 Thread Joshua Cohen
/health_checker.py (line 304) <https://reviews.apache.org/r/53590/#comment226268> As per our discussion this morning, should we make this explicit and name it `signal_running_callback` or something similar (here and in `from_assigned_task`)? - Joshua Cohen On Nov. 16, 2016, 9:

Re: Review Request 53831: Filter out resources that Aurora does not yet support to avoid crashing

2016-11-17 Thread Joshua Cohen
> On Nov. 17, 2016, 4:15 p.m., Joshua Cohen wrote: > > src/main/java/org/apache/aurora/scheduler/resources/ResourceManager.java, > > line 242 > > <https://reviews.apache.org/r/53831/diff/2/?file=1565780#file1565780line242> > > > > All TODOs should

Re: Review Request 53831: Filter out resources that Aurora does not yet support to avoid crashing

2016-11-17 Thread Joshua Cohen
(line 243) <https://reviews.apache.org/r/53831/#comment226369> style nit: one arg per line for continuations. src/test/java/org/apache/aurora/scheduler/resources/ResourceManagerTest.java (line 181) <https://reviews.apache.org/r/53831/#comment226368> kill blank line - Jos

Re: Review Request 53836: Get pants using the same thrift binary as gradle.

2016-11-17 Thread Joshua Cohen
3437e4039961edf. Note the failure about 80% through, followed by some strange compile issues? - Joshua Cohen On Nov. 17, 2016, 4:46 a.m., John Sirois wrote: > > --- > This is an automatically generated e-mail. To reply, visit: >

Re: Review Request 53836: Get pants using the same thrift binary as gradle.

2016-11-17 Thread Joshua Cohen
> On Nov. 17, 2016, 11:28 p.m., Joshua Cohen wrote: > > I'm seeing intermittent failures running the e2e tests with this patch > > applied. That's not necessarily alarming, as we've had reports of this on > > master as well, however the behavior in the event

Re: Review Request 53836: Get pants using the same thrift binary as gradle.

2016-11-17 Thread Joshua Cohen
> On Nov. 17, 2016, 11:28 p.m., Joshua Cohen wrote: > > I'm seeing intermittent failures running the e2e tests with this patch > > applied. That's not necessarily alarming, as we've had reports of this on > > master as well, however the behavior in the event

Re: Review Request 53923: Filter out calls to fromResource for resources that Aurora does not support yet to avoid crashing

2016-11-21 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53923/#review156463 --- Ship it! lgtm modulo Stephan's suggestion. - Joshua

Re: Review Request 53965: track percentile values for timed sliding stats

2016-11-22 Thread Joshua Cohen
- On Nov. 22, 2016, 4:35 p.m., Reza Motamedi wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/53965/ >

Re: Review Request 54011: Add benchmarks for `StateManagerImpl`.

2016-11-23 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54011/#review156791 --- Ship it! Ship It! - Joshua Cohen On Nov. 23, 2016, 7:44 p.m

Re: Review Request 54106: changes to intercept and time mybatis invocations

2016-11-28 Thread Joshua Cohen
u get these properly split up? - Joshua Cohen On Nov. 27, 2016, 10:20 p.m., Reza Motamedi wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.a

Re: Review Request 51993: Added the 'reason' to the /pendingTasks endpoint

2016-11-28 Thread Joshua Cohen
ndpoint, but the key is exposed below via the `getName` method? - Joshua Cohen On Nov. 3, 2016, 10:46 p.m., Pradyumna Kaushik wrote: > > --- > This is an automatically generated e-mail.

Re: Review Request 51993: Added the 'reason' to the /pendingTasks endpoint

2016-11-28 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51993/#review157136 --- Ship it! Ship It! - Joshua Cohen On Nov. 28, 2016, 8:34 p.m

Re: Review Request 54107: changes to intercept and time mybatis invocations

2016-11-28 Thread Joshua Cohen
rora/scheduler/storage/mem/InMemTaskStoreTest.java (line 52) <https://reviews.apache.org/r/54107/#comment227653> Use `//` style comments, no need for javadoc style here. src/test/java/org/apache/aurora/scheduler/storage/mem/MemCronJobStoreTest.java (line 42) <https://reviews.apache.org/r

Re: Review Request 54106: SlaUtil::percentile percentiles interpolation

2016-11-28 Thread Joshua Cohen
This test should move in to a new SlaUtilTest class, rather than piggybacking on SlaAlgorithmTest - Joshua Cohen On Nov. 29, 2016, 12:17 a.m., Reza Motamedi wrote: > > --- > This is an automatically generated e-mail. To

Re: Review Request 54106: SlaUtil::percentile percentiles interpolation

2016-11-30 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54106/#review157487 --- Ship it! Ship It! - Joshua Cohen On Nov. 30, 2016, 6:28 a.m

Re: Review Request 54250: Revert removal of twitter/commons/zk based leadership code

2016-12-01 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54250/#review157596 --- Ship it! Ship It! - Joshua Cohen On Dec. 1, 2016, 10:48 a.m

Re: Review Request 54255: WIP: Update to Mesos 1.1.0.

2016-12-01 Thread Joshua Cohen
curiosity, what was the build time after the changes? src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorSettingsLoader.java (line 94) <https://reviews.apache.org/r/54255/#comment228221> What's the reasoning for this change? - Joshua Cohen On Dec. 1,

Re: Review Request 54107: changes to intercept and time mybatis invocations

2016-12-01 Thread Joshua Cohen
Mehrdad requested and I can commit this. src/main/java/org/apache/aurora/scheduler/storage/db/InstrumentingInterceptor.java (lines 104 - 106) <https://reviews.apache.org/r/54107/#comment228251> indentation looks off here. - Joshua Cohen On Dec. 1, 2016, 1:53 a.m., Reza Motamedi

Re: Review Request 54107: changes to intercept and time mybatis invocations

2016-12-01 Thread Joshua Cohen
/InstrumentingInterceptorTest.java (line 40) <https://reviews.apache.org/r/54107/#comment228275> Is there a reason we need the power mock runner? - Joshua Cohen On Dec. 1, 2016, 8:39 p.m., Reza Motamedi wrote: > > --- > This is a

Re: Review Request 54107: changes to intercept and time mybatis invocations

2016-12-01 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54107/#review157662 --- @ReviewBot retry - Joshua Cohen On Dec. 1, 2016, 9:37 p.m

Re: Review Request 54288: Make leader elections resilient to ZK disconnections.

2016-12-02 Thread Joshua Cohen
test (i.e. sleep for up to N seconds then `fail()`)? - Joshua Cohen On Dec. 2, 2016, 3:19 a.m., Zameer Manji wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https:

Re: Review Request 54269: Improve scheduling throughput via logging changes.

2016-12-02 Thread Joshua Cohen
tps://reviews.apache.org/r/54269/#comment228400> Can you add a comment here explaining why we're not using `%class` and `%line`? They're definitely valuable when debugging, so I can imagine someone coming back in the future to change this. - Joshua Cohen On Dec. 1, 2016, 11:13 p.

Re: Review Request 54299: Extend warm-up time by `max_consecutive_failures` attempts.

2016-12-02 Thread Joshua Cohen
client, then the executor could only trigger the new behavior if the user opted in by setting `watch_secs` to 0 and `min_consecutive_successes` to non-zero. - Joshua Cohen On Dec. 2, 2016, 8:43 a.m., Santhosh Kumar Shanmugham wrote: > > -

Re: Review Request 54288: Make leader elections resilient to ZK disconnections.

2016-12-02 Thread Joshua Cohen
> On Dec. 2, 2016, 3:58 p.m., Joshua Cohen wrote: > > src/test/java/org/apache/aurora/scheduler/discovery/CuratorSingletonServiceTest.java, > > lines 215-218 > > <https://reviews.apache.org/r/54288/diff/1/?file=1574553#file1574553line215> > > > > Shou

Re: Review Request 53836: Get pants using the same thrift binary as gradle.

2016-12-05 Thread Joshua Cohen
> On Nov. 17, 2016, 11:28 p.m., Joshua Cohen wrote: > > I'm seeing intermittent failures running the e2e tests with this patch > > applied. That's not necessarily alarming, as we've had reports of this on > > master as well, however the behavior in the event

Review Request 54428: Fix invalid logging that was causing pmd to NPE.

2016-12-06 Thread Joshua Cohen
--- Fix invalid logging that was causing pmd to NPE. Diffs - src/main/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJob.java ae7d019097f3e7049622623b48f3731198a08fb0 Diff: https://reviews.apache.org/r/54428/diff/ Testing --- Thanks, Joshua Cohen

Review Request 54439: Add support for an mttu metric (median time to updated)

2016-12-06 Thread Joshua Cohen
54439/diff/ Testing --- ./gradlew build -Pq e2e tests. Thanks, Joshua Cohen

Re: Review Request 54439: Add support for an mttu metric (median time to updated)

2016-12-06 Thread Joshua Cohen
lTest.java 6d0e9bc6a8040393875d4f0a88e8db9d6926a88b Diff: https://reviews.apache.org/r/54439/diff/ Testing --- ./gradlew build -Pq e2e tests. Thanks, Joshua Cohen

Re: Review Request 54439: Add support for an mttu metric (median time to updated)

2016-12-07 Thread Joshua Cohen
. I debated adding that when I was writing it, thanks for the nudge. - Joshua --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54439/#review158343 --------

Re: Review Request 54439: Add support for an mttu metric (median time to updated)

2016-12-07 Thread Joshua Cohen
src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java 6d0e9bc6a8040393875d4f0a88e8db9d6926a88b Diff: https://reviews.apache.org/r/54439/diff/ Testing --- ./gradlew build -Pq e2e tests. Thanks, Joshua Cohen

Re: Review Request 54459: Add message parameter to killTasks

2016-12-07 Thread Joshua Cohen
612) <https://reviews.apache.org/r/54459/#comment229065> Fix indent. - Joshua Cohen On Dec. 7, 2016, 2:20 a.m., Cody Gibb wrote: > > --- > This is an automatically g

Re: Review Request 53836: Get pants using the same thrift binary as gradle.

2016-12-07 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53836/#review158399 --- Ship it! Ship It! - Joshua Cohen On Dec. 7, 2016, 12:30 a.m

Re: Review Request 54255: Update to Mesos 1.1.0.

2016-12-07 Thread Joshua Cohen
review? RELEASE-NOTES.md (line 5) <https://reviews.apache.org/r/54255/#comment229120> 1.1.0 ;) - Joshua Cohen On Dec. 7, 2016, 11:04 a.m., Stephan Erb wrote: > > --- > This is an automatically generated e-mail.

Re: Review Request 54299: Extend warm-up time by `max_consecutive_failures` attempts.

2016-12-07 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54299/#review158490 --- Ship it! Ship It! - Joshua Cohen On Dec. 8, 2016, 12:15 a.m

Re: Review Request 54520: Revert BUILD changes in 0c177058.

2016-12-07 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54520/#review158497 --- Ship it! Ship It! - Joshua Cohen On Dec. 8, 2016, 4:39 a.m

Re: Review Request 54439: Add support for an mttu metric (median time to updated)

2016-12-08 Thread Joshua Cohen
d e-mail. To reply, visit: https://reviews.apache.org/r/54439/#review158363 ------- On Dec. 7, 2016, 5:50 p.m., Joshua Cohen wrote: > > --- > This is an a

Re: Review Request 54439: Add support for an mttu metric (median time to updated)

2016-12-08 Thread Joshua Cohen
e-mail. To reply, visit: https://reviews.apache.org/r/54439/#review158360 ------- On Dec. 7, 2016, 5:50 p.m., Joshua Cohen wrote: > > --- > This is an aut

Review Request 54550: Specify python2.7 when extracting options from pants.ini during thrift bootstrap.

2016-12-08 Thread Joshua Cohen
sting --- Thanks, Joshua Cohen

Re: Review Request 54550: Specify python2.7 when extracting options from pants.ini during thrift bootstrap.

2016-12-08 Thread Joshua Cohen
x27;t have the `json` module, so this script fails. Diffs (updated) - build-support/thrift/prepare_binary.sh a96b33e4564604f72d8fcf284f0cf1adea1866a9 Diff: https://reviews.apache.org/r/54550/diff/ Testing --- Thanks, Joshua Cohen

Re: Review Request 54550: Specify python2.7 when extracting options from pants.ini during thrift bootstrap.

2016-12-08 Thread Joshua Cohen
`set -o xtrace`? TIL - Joshua --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54550/#review158625 --- On Dec. 9, 20

Re: Review Request 54550: Specify python2.7 when extracting options from pants.ini during thrift bootstrap.

2016-12-08 Thread Joshua Cohen
ry.sh a96b33e4564604f72d8fcf284f0cf1adea1866a9 Diff: https://reviews.apache.org/r/54550/diff/ Testing --- Thanks, Joshua Cohen

Review Request 54569: Change back to thrift directory silently after executing pants command.

2016-12-08 Thread Joshua Cohen
--- Change back to thrift directory silently after executing pants command. Diffs - build-support/thrift/prepare_binary.sh e3b46aa1f36ed3eecc9bb7794073d18dd950eb1c Diff: https://reviews.apache.org/r/54569/diff/ Testing --- Ran this in vagrant. Thanks, Joshua Cohen

Re: Review Request 54567: Fixup prepare_binary.sh to work under modern bash.

2016-12-08 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54567/#review158638 --- Ship it! Ship It! - Joshua Cohen On Dec. 9, 2016, 3:58 a.m

Re: Review Request 54439: Add support for an mttu metric (median time to updated)

2016-12-09 Thread Joshua Cohen
ple active updates, seems like we could miss data > > points? Especially for small updates. > > Joshua Cohen wrote: > My thinking was that the vast majority of updates in the store will be > completed hours or days ago, so there's no need to consider them when >

Re: Review Request 54439: Add support for an mttu metric (median time to updated)

2016-12-09 Thread Joshua Cohen
ple active updates, seems like we could miss data > > points? Especially for small updates. > > Joshua Cohen wrote: > My thinking was that the vast majority of updates in the store will be > completed hours or days ago, so there's no need to consider them when >

Re: Review Request 54624: Expose stats on ZooKeeper connection state

2016-12-10 Thread Joshua Cohen
, not just into the log. - Joshua Cohen On Dec. 10, 2016, 11:04 a.m., Jing Chen wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache

Review Request 54669: Fix thrift bootstrap to use python2.7.

2016-12-12 Thread Joshua Cohen
, Joshua Cohen

Re: Review Request 54439: Add support for an mttu metric (median time to updated)

2016-12-12 Thread Joshua Cohen
ple active updates, seems like we could miss data > > points? Especially for small updates. > > Joshua Cohen wrote: > My thinking was that the vast majority of updates in the store will be > completed hours or days ago, so there's no need to consider them when >

Re: Review Request 52669: Move the H2 database off heap.

2016-12-13 Thread Joshua Cohen
acks to disable security) and then add some e2e tests around it. - Joshua Cohen On Oct. 11, 2016, 6:17 p.m., Stephan Erb wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https:/

Re: Review Request 54773: Add finer grained timings to the Snapshot process.

2016-12-15 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54773/#review159302 --- Ship it! Ship It! - Joshua Cohen On Dec. 15, 2016, 7:14 a.m

Re: Review Request 54774: Avoid double writing job updates to the Scheduler Snapshot

2016-12-15 Thread Joshua Cohen
r not there's a `dbScript` in the snapshot. That's a relatively recent addition to snapshots. Prior to that we fetched everything from H2 and dumped it to the snapshot as individual thrift entries. - Joshua Cohen On Dec. 15, 2016, 7:57 a.m., David McLaughlin wrote: > > -

Re: Review Request 54774: Avoid double writing job updates to the Scheduler Snapshot

2016-12-15 Thread Joshua Cohen
/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java (line 128) <https://reviews.apache.org/r/54774/#comment230310> This `@Timed` annotation is on the wrong method now. - Joshua Cohen On Dec. 15, 2016, 6:16 p.m., David McLaughlin

Re: Review Request 54774: Avoid double writing job updates to the Scheduler Snapshot

2016-12-16 Thread Joshua Cohen
On Dec. 15, 2016, 8:04 p.m., David McLaughlin wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/54774/ > --- > > (Updated Dec. 15, 2016, 8:04 p.m.) > &

Re: Review Request 54847: Remove ignored snapshot stats. Add high-level timings on storage start-up lifecycle.

2016-12-19 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54847/#review159616 --- Ship it! Ship It! - Joshua Cohen On Dec. 18, 2016, 11:51

Re: Review Request 54624: Expose stats on ZooKeeper connection state

2016-12-27 Thread Joshua Cohen
o reference `Stats.STATS_PROVIDER` directly (if we ever bound another instance of `StatsProvider` in `AppModule` this class would not be aware of that, nor would we be able to use a mock stats provider for tests if we so desired. - Joshua Cohen On Dec. 20, 2016, 9:33 a.m., Jing

Re: Review Request 54883: Move snapshots into a separate log

2016-12-27 Thread Joshua Cohen
aurora/scheduler/storage/log/StreamManager.java (lines 92 - 94) <https://reviews.apache.org/r/54883/#comment231271> same here. src/main/java/org/apache/aurora/scheduler/storage/log/StreamManagerImpl.java (line 106) <https://reviews.apache.org/r/54883/#comment231272> +blank

Re: Review Request 54883: Move snapshots into a separate log

2016-12-27 Thread Joshua Cohen
> On Dec. 27, 2016, 10:50 p.m., Joshua Cohen wrote: > > Overall this looks good to me. I think some work would be required to > > productionize it (e.g. make it optional to start). Obviously want to vet > > this in a test cluster and we would need some doc changes to

Re: Review Request 55179: AURORA-1820 Reduce storage write lock contention by adopting Double-Checked Locking pattern in TimedOutTaskHandler

2017-01-04 Thread Joshua Cohen
-- > > (Updated Jan. 4, 2017, 5:16 p.m.) > > > Review request for Aurora, Joshua Cohen and Stephan Erb. > > > Bugs: AURORA-1820 > https://issues.apache.org/jira/browse/AURORA-1820 > > > Repository: aurora >

Re: Review Request 55179: AURORA-1820 Reduce storage write lock contention by adopting Double-Checked Locking pattern in TimedOutTaskHandler

2017-01-04 Thread Joshua Cohen
x27;s comments. - Joshua Cohen On Jan. 4, 2017, 5:16 p.m., Mehrdad Nurolahzade wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.a

Re: Review Request 55105: AURORA-1870 Add finer grained timings to the Snapshot process

2017-01-04 Thread Joshua Cohen
/SnapshotStoreImpl.java (lines 520 - 532) <https://reviews.apache.org/r/55105/#comment231669> Can these be default methods on the interface rather than converting to an abstract class? - Joshua Cohen On Dec. 30, 2016, 9:18 p.m., Mehrdad Nurolahzade

Review Request 55347: Ensure destination exists when mounting files into a filesystem image.

2017-01-09 Thread Joshua Cohen
pport/jenkins/build.sh Thanks, Joshua Cohen

Re: Review Request 55347: Ensure destination exists when mounting files into a filesystem image.

2017-01-09 Thread Joshua Cohen
reply, visit: https://reviews.apache.org/r/55347/#review160995 --- On Jan. 9, 2017, 5:38 p.m., Joshua Cohen wrote: > > --- > This is an automatically generated e-mail. To reply,

Re: Review Request 55347: Ensure destination exists when mounting files into a filesystem image.

2017-01-10 Thread Joshua Cohen
urora 438eb0f03128d21b3201d0a843adebe09422c75e src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 46309371a33d26bdd467b5fa131634354be2a978 Diff: https://reviews.apache.org/r/55347/diff/ Testing --- ./build-support/jenkins/build.sh Thanks, Joshua Cohen

Re: Review Request 55347: Ensure destination exists when mounting files into a filesystem image.

2017-01-10 Thread Joshua Cohen
uler.conf#L43 > > list instead? Added to startup config and added e2e test as well. - Joshua --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55347/#review160933

Re: Review Request 55347: Ensure destination exists when mounting files into a filesystem image.

2017-01-10 Thread Joshua Cohen
> > > > > > I will refresh this build result if you post a review containing > > "@ReviewBot retry" @ReviewBot retry - Joshua --- This is an automatically generated e-mail. To reply, visit: https://r

Re: Review Request 55409: AURORA-1873 CuratorServiceGroupMonitor.LOG should use its own logger name

2017-01-11 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55409/#review161250 --- Ship it! Thanks for cleaning this up! - Joshua Cohen On Jan

Review Request 55434: Reduce logging by ChainedStatusChecker and StatusManager when they're on the happy path.

2017-01-11 Thread Joshua Cohen
er.py f278825e58bba40c3b3ec735173705feb42bf165 src/main/python/apache/aurora/executor/status_manager.py 8b536a925e3f209c03b3eb44257096a0c0e497e0 Diff: https://reviews.apache.org/r/55434/diff/ Testing --- Thanks, Joshua Cohen

Re: Review Request 55434: Reduce logging by ChainedStatusChecker and StatusManager when they're on the happy path.

2017-01-11 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55434/#review161305 --- @ReviewBot retry - Joshua Cohen On Jan. 11, 2017, 9:41 p.m

Re: Review Request 55357: AURORA-1867 Consider reserving for multiple tasks per preemption round

2017-01-11 Thread Joshua Cohen
539> s/UNMATECHED/UNMATCHED - Joshua Cohen On Jan. 11, 2017, 4:48 p.m., Mehrdad Nurolahzade wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.

Re: Review Request 54967: AURORA-1856 Expose stats on deleted job updates in JobUpdateHistoryPruner

2017-01-11 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54967/#review161315 --- Ship it! Ship It! - Joshua Cohen On Dec. 22, 2016, 7:37 a.m

Re: Review Request 55217: AURORA-1847 Eliminate sequential scan in MemTaskStore.getJobKeys()

2017-01-11 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55217/#review161318 --- Ship it! Ship It! - Joshua Cohen On Jan. 5, 2017, 6:59 p.m

<    1   2   3   4   5   6   7   8   >