You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@aurora.apache.org by Keisuke Nishimoto <ke...@gmail.com> on 2017/09/19 22:49:31 UTC
Review Request 62423: Improve in-process test ZooKeeper support
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62423/
-----------------------------------------------------------
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 62423: Improve in-process test ZooKeeper support
Posted by Stephan Erb <se...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62423/#review185927
-----------------------------------------------------------
Ship it!
Thanks for the patch! It looks good to me.
I cannot commit this atm though. Bill, could you handle that?
- Stephan Erb
On Sept. 20, 2017, 9: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, 9: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
Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
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
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
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
> -----
>
> 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
Posted by Keisuke Nishimoto <ke...@gmail.com>.
-----------------------------------------------------------
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
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
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
Posted by Keisuke Nishimoto <ke...@gmail.com>.
-----------------------------------------------------------
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
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62423/#review185745
-----------------------------------------------------------
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. 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 62423: Improve in-process test ZooKeeper support
Posted by Keisuke Nishimoto <ke...@gmail.com>.
> 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)
> > <https://reviews.apache.org/r/62423/diff/1/?file=1830036#file1830036line150>
> >
> > 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 62423: Improve in-process test ZooKeeper support
Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
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)
<https://reviews.apache.org/r/62423/#comment262140>
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
>
>