Re: Review Request 62423: Improve in-process test ZooKeeper support

[Aurora ReviewBot]

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



Master (96a5287) is green with this patch.
  ./build-support/jenkins/build.sh

However, it appears that it might lack test coverage.

I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On Sept. 20, 2017, 7:09 p.m., Keisuke Nishimoto wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62423/
> ---
> 
> (Updated Sept. 20, 2017, 7:09 p.m.)
> 
> 
> Review request for Aurora.
> 
> 
> Bugs: AURORA-1947
> https://issues.apache.org/jira/browse/AURORA-1947
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> MesosLogStreamModule tries to connect to ZooKeeper servers specified by 
> -zk_endpoints even when -zk_in_proc=true.  I updated the module to use 
> injected server endpoints which will be based on the ephemeral port assigned 
> to ZooKeeperTestServer if -zk_in_proc=true.  This required to make 
> @ServiceDiscoveryBindings.ZooKeeper public.
> 
> I also tweaked shutdown process of ServiceDiscoveryModule.TestServerService 
> so that it won't close existing ZooKeeper connections before clients close 
> the session.  While just delaying the execution by 1 second doesn't really 
> guarantee that behavior, in practice this achieved clean shutdown of the 
> scheduler with in-process ZooKeeper server.
> 
> 
> Diffs
> -
> 
>   
> src/main/java/org/apache/aurora/scheduler/discovery/ServiceDiscoveryBindings.java
>  28cdc4b3b454b3d25008a21c6b12634173e1f878 
>   
> src/main/java/org/apache/aurora/scheduler/discovery/ServiceDiscoveryModule.java
>  c105dbdbe8339db7f5cca2e5e391fffb4cd87b07 
>   src/main/java/org/apache/aurora/scheduler/discovery/ZooKeeperConfig.java 
> 3f32a6272bb05fc5d7ffd576f6645be00114a42c 
>   
> src/main/java/org/apache/aurora/scheduler/log/mesos/MesosLogStreamModule.java 
> 6704a328a4023a178ed8f86ae4772cb04eb2fa8e 
> 
> 
> Diff: https://reviews.apache.org/r/62423/diff/2/
> 
> 
> Testing
> ---
> 
> 1. Launch Mesos master and slave on my laptop.
> 2. Launch Aurora scheduler with following arguments:
> ```
> -backup_dir=/var/lib/aurora/backups
> -cluster_name=local
> -mesos_master_address=localhost:5050
> -serverset_path=/aurora/scheduler
> -ip=127.0.0.1
> -hostname=localhost
> -http_port=8081
> -zk_in_proc=true
> -zk_endpoints=localhost:2181
> -native_log_zk_group_path=/aurora/replicated-log
> -native_log_file_path=/var/db/aurora
> ```
> 3. Observe that there are no ZooKeeper error log outputs caused by missing 
> endpoint.
> 4. Create a simple job, observer it launches normally and then kill it.
> 5. Stop the scheduler by sending /quitquitquit.
> 6. Observe that scheduler process shuts down normally.
> 
> 
> Thanks,
> 
> Keisuke Nishimoto
> 
>

Re: Review Request 62423: Improve in-process test ZooKeeper support

[Keisuke Nishimoto]

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



@ReviewBot retry

- Keisuke Nishimoto


On Sept. 20, 2017, 7:09 p.m., Keisuke Nishimoto wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62423/
> ---
> 
> (Updated Sept. 20, 2017, 7:09 p.m.)
> 
> 
> Review request for Aurora.
> 
> 
> Bugs: AURORA-1947
> https://issues.apache.org/jira/browse/AURORA-1947
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> MesosLogStreamModule tries to connect to ZooKeeper servers specified by 
> -zk_endpoints even when -zk_in_proc=true.  I updated the module to use 
> injected server endpoints which will be based on the ephemeral port assigned 
> to ZooKeeperTestServer if -zk_in_proc=true.  This required to make 
> @ServiceDiscoveryBindings.ZooKeeper public.
> 
> I also tweaked shutdown process of ServiceDiscoveryModule.TestServerService 
> so that it won't close existing ZooKeeper connections before clients close 
> the session.  While just delaying the execution by 1 second doesn't really 
> guarantee that behavior, in practice this achieved clean shutdown of the 
> scheduler with in-process ZooKeeper server.
> 
> 
> Diffs
> -
> 
>   
> src/main/java/org/apache/aurora/scheduler/discovery/ServiceDiscoveryBindings.java
>  28cdc4b3b454b3d25008a21c6b12634173e1f878 
>   
> src/main/java/org/apache/aurora/scheduler/discovery/ServiceDiscoveryModule.java
>  c105dbdbe8339db7f5cca2e5e391fffb4cd87b07 
>   src/main/java/org/apache/aurora/scheduler/discovery/ZooKeeperConfig.java 
> 3f32a6272bb05fc5d7ffd576f6645be00114a42c 
>   
> src/main/java/org/apache/aurora/scheduler/log/mesos/MesosLogStreamModule.java 
> 6704a328a4023a178ed8f86ae4772cb04eb2fa8e 
> 
> 
> Diff: https://reviews.apache.org/r/62423/diff/2/
> 
> 
> Testing
> ---
> 
> 1. Launch Mesos master and slave on my laptop.
> 2. Launch Aurora scheduler with following arguments:
> ```
> -backup_dir=/var/lib/aurora/backups
> -cluster_name=local
> -mesos_master_address=localhost:5050
> -serverset_path=/aurora/scheduler
> -ip=127.0.0.1
> -hostname=localhost
> -http_port=8081
> -zk_in_proc=true
> -zk_endpoints=localhost:2181
> -native_log_zk_group_path=/aurora/replicated-log
> -native_log_file_path=/var/db/aurora
> ```
> 3. Observe that there are no ZooKeeper error log outputs caused by missing 
> endpoint.
> 4. Create a simple job, observer it launches normally and then kill it.
> 5. Stop the scheduler by sending /quitquitquit.
> 6. Observe that scheduler process shuts down normally.
> 
> 
> Thanks,
> 
> Keisuke Nishimoto
> 
>

Re: Review Request 62423: Improve in-process test ZooKeeper support

[Bill Farner]

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


Ship it!




Ship It!

- Bill Farner


On Sept. 20, 2017, 12:09 p.m., Keisuke Nishimoto wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62423/
> ---
> 
> (Updated Sept. 20, 2017, 12:09 p.m.)
> 
> 
> Review request for Aurora.
> 
> 
> Bugs: AURORA-1947
> https://issues.apache.org/jira/browse/AURORA-1947
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> MesosLogStreamModule tries to connect to ZooKeeper servers specified by 
> -zk_endpoints even when -zk_in_proc=true.  I updated the module to use 
> injected server endpoints which will be based on the ephemeral port assigned 
> to ZooKeeperTestServer if -zk_in_proc=true.  This required to make 
> @ServiceDiscoveryBindings.ZooKeeper public.
> 
> I also tweaked shutdown process of ServiceDiscoveryModule.TestServerService 
> so that it won't close existing ZooKeeper connections before clients close 
> the session.  While just delaying the execution by 1 second doesn't really 
> guarantee that behavior, in practice this achieved clean shutdown of the 
> scheduler with in-process ZooKeeper server.
> 
> 
> Diffs
> -
> 
>   
> src/main/java/org/apache/aurora/scheduler/discovery/ServiceDiscoveryBindings.java
>  28cdc4b3b454b3d25008a21c6b12634173e1f878 
>   
> src/main/java/org/apache/aurora/scheduler/discovery/ServiceDiscoveryModule.java
>  c105dbdbe8339db7f5cca2e5e391fffb4cd87b07 
>   src/main/java/org/apache/aurora/scheduler/discovery/ZooKeeperConfig.java 
> 3f32a6272bb05fc5d7ffd576f6645be00114a42c 
>   
> src/main/java/org/apache/aurora/scheduler/log/mesos/MesosLogStreamModule.java 
> 6704a328a4023a178ed8f86ae4772cb04eb2fa8e 
> 
> 
> Diff: https://reviews.apache.org/r/62423/diff/2/
> 
> 
> Testing
> ---
> 
> 1. Launch Mesos master and slave on my laptop.
> 2. Launch Aurora scheduler with following arguments:
> ```
> -backup_dir=/var/lib/aurora/backups
> -cluster_name=local
> -mesos_master_address=localhost:5050
> -serverset_path=/aurora/scheduler
> -ip=127.0.0.1
> -hostname=localhost
> -http_port=8081
> -zk_in_proc=true
> -zk_endpoints=localhost:2181
> -native_log_zk_group_path=/aurora/replicated-log
> -native_log_file_path=/var/db/aurora
> ```
> 3. Observe that there are no ZooKeeper error log outputs caused by missing 
> endpoint.
> 4. Create a simple job, observer it launches normally and then kill it.
> 5. Stop the scheduler by sending /quitquitquit.
> 6. Observe that scheduler process shuts down normally.
> 
> 
> Thanks,
> 
> Keisuke Nishimoto
> 
>

Re: Review Request 62423: Improve in-process test ZooKeeper support

[Aurora ReviewBot]

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



Master (96a5287) is red with this patch.
  ./build-support/jenkins/build.sh

  Test coverage missing for org/apache/aurora/scheduler/storage/db/PruneVictim
  Test coverage missing for 
org/apache/aurora/scheduler/state/TaskAssigner$FirstFitTaskAssigner
  Test coverage missing for 
org/apache/aurora/scheduler/discovery/CommonsServiceGroupMonitor
  Test coverage missing for 
org/apache/aurora/scheduler/http/api/GsonMessageBodyHandler
  Test coverage missing for org/apache/aurora/scheduler/http/api/ApiBeta
  Test coverage missing for 
org/apache/aurora/scheduler/reconciliation/KillRetry$KillAttempt
  Test coverage missing for 
org/apache/aurora/scheduler/cron/quartz/CronPredictorImpl
  Test coverage missing for 
org/apache/aurora/scheduler/scheduling/RescheduleCalculator$RescheduleCalculatorImpl$1
  Test coverage missing for org/apache/aurora/scheduler/quota/QuotaInfo
  Test coverage missing for 
org/apache/aurora/scheduler/quota/QuotaManager$QuotaManagerImpl
  Test coverage missing for 
org/apache/aurora/scheduler/offers/RandomJitterReturnDelay
  Test coverage missing for 
org/apache/aurora/scheduler/mesos/FrameworkInfoFactory$FrameworkInfoFactoryImpl
  Test coverage missing for 
org/apache/aurora/scheduler/thrift/aop/LoggingInterceptor
  Test coverage missing for 
org/apache/aurora/scheduler/thrift/aop/ThriftStatsExporterInterceptor$1
  Test coverage missing for 
org/apache/aurora/scheduler/thrift/aop/ThriftStatsExporterInterceptor$2
  Test coverage missing for 
org/apache/aurora/scheduler/thrift/aop/LoggingInterceptor$1
  Test coverage missing for 
org/apache/aurora/scheduler/thrift/aop/ThriftStatsExporterInterceptor
  Test coverage missing for 
org/apache/aurora/scheduler/thrift/aop/ThriftWorkload$ThriftWorkloadCounterImpl
  Test coverage missing for 
org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl
  Test coverage missing for 
org/apache/aurora/scheduler/preemptor/PreemptionVictimFilter$PreemptionVictimFilterImpl
  Test coverage missing for org/apache/aurora/scheduler/preemptor/BiCache$1
  Test coverage missing for 
org/apache/aurora/scheduler/preemptor/PreemptionVictimFilter$PreemptionVictimFilterImpl$2
  Test coverage missing for 
org/apache/aurora/scheduler/preemptor/PreemptionVictimFilter$PreemptionVictimFilterImpl$1
  Test coverage missing for 
org/apache/aurora/scheduler/events/NotifyingSchedulingFilter
  Test coverage missing for 
org/apache/aurora/scheduler/events/PubsubEvent$HostAttributesChanged
  Test coverage missing for 
org/apache/aurora/scheduler/storage/backup/TemporaryStorage$TemporaryStorageFactory$1
  Test coverage missing for 
org/apache/aurora/scheduler/storage/backup/StorageBackup$StorageBackupImpl$BackupConfig
  Test coverage missing for 
org/apache/aurora/scheduler/storage/backup/Recovery$RecoveryImpl
  Test coverage missing for 
org/apache/aurora/scheduler/storage/backup/TemporaryStorage$TemporaryStorageFactory
  Test coverage missing for 
org/apache/aurora/scheduler/storage/backup/Recovery$RecoveryImpl$PendingRecovery
  Test coverage missing for org/apache/aurora/scheduler/HostOffer$1
  Test coverage missing for org/apache/aurora/scheduler/TaskStatusHandlerImpl$1

* Try:
Run with --stacktrace option to get the stack trace. Run with --info or --debug 
option to get more log output.
==

BUILD FAILED

Total time: 11 mins 37.608 secs


I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On Sept. 20, 2017, 7:09 p.m., Keisuke Nishimoto wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62423/
> ---
> 
> (Updated Sept. 20, 2017, 7:09 p.m.)
> 
> 
> Review request for Aurora.
> 
> 
> Bugs: AURORA-1947
> https://issues.apache.org/jira/browse/AURORA-1947
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> MesosLogStreamModule tries to connect to ZooKeeper servers specified by 
> -zk_endpoints even when -zk_in_proc=true.  I updated the module to use 
> injected server endpoints which will be based on the ephemeral port assigned 
> to ZooKeeperTestServer if -zk_in_proc=true.  This required to make 
> @ServiceDiscoveryBindings.ZooKeeper public.
> 
> I also tweaked shutdown process of ServiceDiscoveryModule.TestServerService 
> so that it won't close existing ZooKeeper connections before clients close 
> the session.  While just delaying the execution by 1 second doesn't really 
> guarantee that behavior, in practice this achieved clean shutdown of the 
> scheduler with in-process ZooKeeper server.
> 
> 
> Diffs
> -
> 
>   
> 

Re: Review Request 62423: Improve in-process test ZooKeeper support

[Keisuke Nishimoto]

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

(Updated Sept. 20, 2017, 7:09 p.m.)


Review request for Aurora.


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


Repository: aurora


Description
---

MesosLogStreamModule tries to connect to ZooKeeper servers specified by 
-zk_endpoints even when -zk_in_proc=true.  I updated the module to use injected 
server endpoints which will be based on the ephemeral port assigned to 
ZooKeeperTestServer if -zk_in_proc=true.  This required to make 
@ServiceDiscoveryBindings.ZooKeeper public.

I also tweaked shutdown process of ServiceDiscoveryModule.TestServerService so 
that it won't close existing ZooKeeper connections before clients close the 
session.  While just delaying the execution by 1 second doesn't really 
guarantee that behavior, in practice this achieved clean shutdown of the 
scheduler with in-process ZooKeeper server.


Diffs (updated)
-

  
src/main/java/org/apache/aurora/scheduler/discovery/ServiceDiscoveryBindings.java
 28cdc4b3b454b3d25008a21c6b12634173e1f878 
  
src/main/java/org/apache/aurora/scheduler/discovery/ServiceDiscoveryModule.java 
c105dbdbe8339db7f5cca2e5e391fffb4cd87b07 
  src/main/java/org/apache/aurora/scheduler/discovery/ZooKeeperConfig.java 
3f32a6272bb05fc5d7ffd576f6645be00114a42c 
  src/main/java/org/apache/aurora/scheduler/log/mesos/MesosLogStreamModule.java 
6704a328a4023a178ed8f86ae4772cb04eb2fa8e 


Diff: https://reviews.apache.org/r/62423/diff/2/

Changes: https://reviews.apache.org/r/62423/diff/1-2/


Testing
---

1. Launch Mesos master and slave on my laptop.
2. Launch Aurora scheduler with following arguments:
```
-backup_dir=/var/lib/aurora/backups
-cluster_name=local
-mesos_master_address=localhost:5050
-serverset_path=/aurora/scheduler
-ip=127.0.0.1
-hostname=localhost
-http_port=8081
-zk_in_proc=true
-zk_endpoints=localhost:2181
-native_log_zk_group_path=/aurora/replicated-log
-native_log_file_path=/var/db/aurora
```
3. Observe that there are no ZooKeeper error log outputs caused by missing 
endpoint.
4. Create a simple job, observer it launches normally and then kill it.
5. Stop the scheduler by sending /quitquitquit.
6. Observe that scheduler process shuts down normally.


Thanks,

Keisuke Nishimoto

Re: Review Request 62423: Improve in-process test ZooKeeper support

[Keisuke Nishimoto]

> On Sept. 20, 2017, 4:55 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/log/mesos/MesosLogStreamModule.java
> > Line 148 (original), 150 (patched)
> > 
> >
> > This seems to suggest that `zkClientConfig.getServers()` may return the 
> > wrong addresses.  Why is that?

This is because zkClientConfig.getServers() here returns values as specified by 
-zk_endpoints, which is supposed to be ignored when -zk_in_proc=true.  Since 
ZooKeeperConfig is not injected and we don't know the port of 
ZooKeeperTestServer when FlaggedZooKeeperConfig.create() is called, it is not 
trivial to let ZooKeeperConfig#getServer() return a correct value when 
in-process ZooKeeper server is used.  I may make ZooKeeperConfig#getServers() 
non-public instead.


- Keisuke


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


On Sept. 19, 2017, 10:49 p.m., Keisuke Nishimoto wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62423/
> ---
> 
> (Updated Sept. 19, 2017, 10:49 p.m.)
> 
> 
> Review request for Aurora.
> 
> 
> Bugs: AURORA-1947
> https://issues.apache.org/jira/browse/AURORA-1947
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> MesosLogStreamModule tries to connect to ZooKeeper servers specified by 
> -zk_endpoints even when -zk_in_proc=true.  I updated the module to use 
> injected server endpoints which will be based on the ephemeral port assigned 
> to ZooKeeperTestServer if -zk_in_proc=true.  This required to make 
> @ServiceDiscoveryBindings.ZooKeeper public.
> 
> I also tweaked shutdown process of ServiceDiscoveryModule.TestServerService 
> so that it won't close existing ZooKeeper connections before clients close 
> the session.  While just delaying the execution by 1 second doesn't really 
> guarantee that behavior, in practice this achieved clean shutdown of the 
> scheduler with in-process ZooKeeper server.
> 
> 
> Diffs
> -
> 
>   
> src/main/java/org/apache/aurora/scheduler/discovery/ServiceDiscoveryBindings.java
>  28cdc4b3b454b3d25008a21c6b12634173e1f878 
>   
> src/main/java/org/apache/aurora/scheduler/discovery/ServiceDiscoveryModule.java
>  c105dbdbe8339db7f5cca2e5e391fffb4cd87b07 
>   
> src/main/java/org/apache/aurora/scheduler/log/mesos/MesosLogStreamModule.java 
> 6704a328a4023a178ed8f86ae4772cb04eb2fa8e 
> 
> 
> Diff: https://reviews.apache.org/r/62423/diff/1/
> 
> 
> Testing
> ---
> 
> 1. Launch Mesos master and slave on my laptop.
> 2. Launch Aurora scheduler with following arguments:
> ```
> -backup_dir=/var/lib/aurora/backups
> -cluster_name=local
> -mesos_master_address=localhost:5050
> -serverset_path=/aurora/scheduler
> -ip=127.0.0.1
> -hostname=localhost
> -http_port=8081
> -zk_in_proc=true
> -zk_endpoints=localhost:2181
> -native_log_zk_group_path=/aurora/replicated-log
> -native_log_file_path=/var/db/aurora
> ```
> 3. Observe that there are no ZooKeeper error log outputs caused by missing 
> endpoint.
> 4. Create a simple job, observer it launches normally and then kill it.
> 5. Stop the scheduler by sending /quitquitquit.
> 6. Observe that scheduler process shuts down normally.
> 
> 
> Thanks,
> 
> Keisuke Nishimoto
> 
>

Re: Review Request 61249: Cron timezone should be configurable per job

[David McLaughlin]

> On Sept. 20, 2017, 4:50 p.m., Bill Farner wrote:
> > This seems to be of dubious value, and likely error-prone.  There are two 
> > perspectives here - display, and time scheduling.  Per-job configuration 
> > for TZ display seems unequivocally misguided, and better suited for API 
> > consumers to handle (e.g. javascript/browser, and the CLI client).  TZ 
> > configuration for time scheduling seems less bad, but error-prone due to 
> > inconsistency within a cluster.  Perhaps a cluster-wide setting makes more 
> > sense here, but to be honest i'm inclined to suggest that UTC is still the 
> > most sane way to eliminate time ambiguity.
> > 
> > Overall this makes me a -1 on the patch as it stands, but i wouldn't stand 
> > in the way if others disagree.

Note that we already have the ability to allow operators to define a 
cluster-wide setting:
https://github.com/apache/aurora/blob/master/src/main/java/org/apache/aurora/scheduler/cron/quartz/CronModule.java#L64

In general I agree with your sentiments.


- David


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


On Sept. 12, 2017, 8:38 a.m., Jing Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61249/
> ---
> 
> (Updated Sept. 12, 2017, 8:38 a.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Joshua Cohen, and Stephan Erb.
> 
> 
> Bugs: AURORA-1348
> https://issues.apache.org/jira/browse/AURORA-1348
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Cron timezone should be configurable per job
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md fd2618fee8ef05091849e177bd99fc321548be90 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 3749531b5412d7ca217736aa85eed8e6606225ad 
>   docs/features/cron-jobs.md 3a3edeabe23553f12074a853c2567b2b62748226 
>   docs/reference/configuration.md bc7e098bde5da1442f7ce1b0f793fa4495e21d9a 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 186fa1b3a4780c0536fb486d50a33133258110cd 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  754fde0fdc976b673d78ae15d8ccd8c85b792373 
>   src/main/java/org/apache/aurora/scheduler/cron/CronPredictor.java 
> 8957b3a04e681c653884fdc9134d74ae766677ae 
>   
> src/main/java/org/apache/aurora/scheduler/cron/quartz/CronJobManagerImpl.java 
> 90399f22c50ba3218fa3f392c3dbf24e6ea524a1 
>   src/main/java/org/apache/aurora/scheduler/cron/quartz/CronLifecycle.java 
> f18e1dcd6d34d6376d267359d2aaedff1d0c0202 
>   
> src/main/java/org/apache/aurora/scheduler/cron/quartz/CronPredictorImpl.java 
> e937667dc73c432dc0934a18f28274913ec5640d 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/views/DbJobConfiguration.java
>  40a5013b62d459d9c766c765f9e536f7042757e1 
>   src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java 
> bba1161a48738e19f10fcf72395f8d70b481ed13 
>   src/main/python/apache/aurora/client/cli/jobs.py 
> b79ae56bee0e5692cacf1e66f4a4126b06aaffdc 
>   src/main/python/apache/aurora/config/schema/base.py 
> 18ce826363009e1cc0beac5cce4edf42610d487e 
>   src/main/python/apache/aurora/config/thrift.py 
> bedf8cd6641e1b1a930602791b758d584af4891c 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/CronJobMapper.xml 
> 1434f45ca0bc188bfb0f2ef3c25fbcd102a3ccb1 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 
> 7a86f47af67adb3e488381d30ddf424549deefbc 
>   
> src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
>  50d7499f4332a3feb0e2301cb707f2cea6bb2e98 
>   src/test/java/org/apache/aurora/scheduler/cron/quartz/CronIT.java 
> 8556253fc11f6027316871eb9dc66d8627a77fe6 
>   
> src/test/java/org/apache/aurora/scheduler/cron/quartz/CronJobManagerImplTest.java
>  81440f5689f9538a4c7a9e6700bf03ca89c4ba85 
>   
> src/test/java/org/apache/aurora/scheduler/cron/quartz/CronPredictorImplTest.java
>  0f8c9839000e2e379a75fd9d10a7f22fc01f3b18 
>   src/test/java/org/apache/aurora/scheduler/cron/quartz/QuartzTestUtil.java 
> 3c5ecd698557cafdf8eeacdc472589a379018896 
>   
> src/test/java/org/apache/aurora/scheduler/storage/AbstractCronJobStoreTest.java
>  3ec6e2bf28db20f1cda26dd9862af83d479ec4bf 
>   
> src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplIT.java
>  7f41430975d46440a404ac58395582f66fd902d4 
>   src/test/java/org/apache/aurora/scheduler/thrift/Fixtures.java 
> 43e32eede27bbf26363a3fd1ca34ffe6f8c01a73 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java
>  39456cfe8b8c8e198a6f8c82e09769bd75db2adc 
>   src/test/python/apache/aurora/client/cli/test_inspect.py 
> ecefc1845350242fe5113e5c85c4e3db09937ec1 
>   

Re: Review Request 62423: Improve in-process test ZooKeeper support

[Bill Farner]

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




src/main/java/org/apache/aurora/scheduler/log/mesos/MesosLogStreamModule.java
Line 148 (original), 150 (patched)


This seems to suggest that `zkClientConfig.getServers()` may return the 
wrong addresses.  Why is that?


- Bill Farner


On Sept. 19, 2017, 3:49 p.m., Keisuke Nishimoto wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62423/
> ---
> 
> (Updated Sept. 19, 2017, 3:49 p.m.)
> 
> 
> Review request for Aurora.
> 
> 
> Bugs: AURORA-1947
> https://issues.apache.org/jira/browse/AURORA-1947
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> MesosLogStreamModule tries to connect to ZooKeeper servers specified by 
> -zk_endpoints even when -zk_in_proc=true.  I updated the module to use 
> injected server endpoints which will be based on the ephemeral port assigned 
> to ZooKeeperTestServer if -zk_in_proc=true.  This required to make 
> @ServiceDiscoveryBindings.ZooKeeper public.
> 
> I also tweaked shutdown process of ServiceDiscoveryModule.TestServerService 
> so that it won't close existing ZooKeeper connections before clients close 
> the session.  While just delaying the execution by 1 second doesn't really 
> guarantee that behavior, in practice this achieved clean shutdown of the 
> scheduler with in-process ZooKeeper server.
> 
> 
> Diffs
> -
> 
>   
> src/main/java/org/apache/aurora/scheduler/discovery/ServiceDiscoveryBindings.java
>  28cdc4b3b454b3d25008a21c6b12634173e1f878 
>   
> src/main/java/org/apache/aurora/scheduler/discovery/ServiceDiscoveryModule.java
>  c105dbdbe8339db7f5cca2e5e391fffb4cd87b07 
>   
> src/main/java/org/apache/aurora/scheduler/log/mesos/MesosLogStreamModule.java 
> 6704a328a4023a178ed8f86ae4772cb04eb2fa8e 
> 
> 
> Diff: https://reviews.apache.org/r/62423/diff/1/
> 
> 
> Testing
> ---
> 
> 1. Launch Mesos master and slave on my laptop.
> 2. Launch Aurora scheduler with following arguments:
> ```
> -backup_dir=/var/lib/aurora/backups
> -cluster_name=local
> -mesos_master_address=localhost:5050
> -serverset_path=/aurora/scheduler
> -ip=127.0.0.1
> -hostname=localhost
> -http_port=8081
> -zk_in_proc=true
> -zk_endpoints=localhost:2181
> -native_log_zk_group_path=/aurora/replicated-log
> -native_log_file_path=/var/db/aurora
> ```
> 3. Observe that there are no ZooKeeper error log outputs caused by missing 
> endpoint.
> 4. Create a simple job, observer it launches normally and then kill it.
> 5. Stop the scheduler by sending /quitquitquit.
> 6. Observe that scheduler process shuts down normally.
> 
> 
> Thanks,
> 
> Keisuke Nishimoto
> 
>

Re: Review Request 61249: Cron timezone should be configurable per job

[Bill Farner]

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



This seems to be of dubious value, and likely error-prone.  There are two 
perspectives here - display, and time scheduling.  Per-job configuration for TZ 
display seems unequivocally misguided, and better suited for API consumers to 
handle (e.g. javascript/browser, and the CLI client).  TZ configuration for 
time scheduling seems less bad, but error-prone due to inconsistency within a 
cluster.  Perhaps a cluster-wide setting makes more sense here, but to be 
honest i'm inclined to suggest that UTC is still the most sane way to eliminate 
time ambiguity.

Overall this makes me a -1 on the patch as it stands, but i wouldn't stand in 
the way if others disagree.

- Bill Farner


On Sept. 12, 2017, 1:38 a.m., Jing Chen wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61249/
> ---
> 
> (Updated Sept. 12, 2017, 1:38 a.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Joshua Cohen, and Stephan Erb.
> 
> 
> Bugs: AURORA-1348
> https://issues.apache.org/jira/browse/AURORA-1348
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Cron timezone should be configurable per job
> 
> 
> Diffs
> -
> 
>   RELEASE-NOTES.md fd2618fee8ef05091849e177bd99fc321548be90 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 
> 3749531b5412d7ca217736aa85eed8e6606225ad 
>   docs/features/cron-jobs.md 3a3edeabe23553f12074a853c2567b2b62748226 
>   docs/reference/configuration.md bc7e098bde5da1442f7ce1b0f793fa4495e21d9a 
>   src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 
> 186fa1b3a4780c0536fb486d50a33133258110cd 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  754fde0fdc976b673d78ae15d8ccd8c85b792373 
>   src/main/java/org/apache/aurora/scheduler/cron/CronPredictor.java 
> 8957b3a04e681c653884fdc9134d74ae766677ae 
>   
> src/main/java/org/apache/aurora/scheduler/cron/quartz/CronJobManagerImpl.java 
> 90399f22c50ba3218fa3f392c3dbf24e6ea524a1 
>   src/main/java/org/apache/aurora/scheduler/cron/quartz/CronLifecycle.java 
> f18e1dcd6d34d6376d267359d2aaedff1d0c0202 
>   
> src/main/java/org/apache/aurora/scheduler/cron/quartz/CronPredictorImpl.java 
> e937667dc73c432dc0934a18f28274913ec5640d 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/views/DbJobConfiguration.java
>  40a5013b62d459d9c766c765f9e536f7042757e1 
>   src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java 
> bba1161a48738e19f10fcf72395f8d70b481ed13 
>   src/main/python/apache/aurora/client/cli/jobs.py 
> b79ae56bee0e5692cacf1e66f4a4126b06aaffdc 
>   src/main/python/apache/aurora/config/schema/base.py 
> 18ce826363009e1cc0beac5cce4edf42610d487e 
>   src/main/python/apache/aurora/config/thrift.py 
> bedf8cd6641e1b1a930602791b758d584af4891c 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/CronJobMapper.xml 
> 1434f45ca0bc188bfb0f2ef3c25fbcd102a3ccb1 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 
> 7a86f47af67adb3e488381d30ddf424549deefbc 
>   
> src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
>  50d7499f4332a3feb0e2301cb707f2cea6bb2e98 
>   src/test/java/org/apache/aurora/scheduler/cron/quartz/CronIT.java 
> 8556253fc11f6027316871eb9dc66d8627a77fe6 
>   
> src/test/java/org/apache/aurora/scheduler/cron/quartz/CronJobManagerImplTest.java
>  81440f5689f9538a4c7a9e6700bf03ca89c4ba85 
>   
> src/test/java/org/apache/aurora/scheduler/cron/quartz/CronPredictorImplTest.java
>  0f8c9839000e2e379a75fd9d10a7f22fc01f3b18 
>   src/test/java/org/apache/aurora/scheduler/cron/quartz/QuartzTestUtil.java 
> 3c5ecd698557cafdf8eeacdc472589a379018896 
>   
> src/test/java/org/apache/aurora/scheduler/storage/AbstractCronJobStoreTest.java
>  3ec6e2bf28db20f1cda26dd9862af83d479ec4bf 
>   
> src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplIT.java
>  7f41430975d46440a404ac58395582f66fd902d4 
>   src/test/java/org/apache/aurora/scheduler/thrift/Fixtures.java 
> 43e32eede27bbf26363a3fd1ca34ffe6f8c01a73 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java
>  39456cfe8b8c8e198a6f8c82e09769bd75db2adc 
>   src/test/python/apache/aurora/client/cli/test_inspect.py 
> ecefc1845350242fe5113e5c85c4e3db09937ec1 
>   src/test/python/apache/aurora/config/test_thrift.py 
> 7a1567a9b67917072bb0ba3eea5857e968375f4d 
> 
> 
> Diff: https://reviews.apache.org/r/61249/diff/3/
> 
> 
> Testing
> ---
> 
> timezone infomation can be configured via configuration file, aka .aurora 
> file. A new entry **time_zone** will be introduced.
> 
> Time zone information would