You are viewing a plain text version of this content. The canonical link for it is here.

Posted to reviews@aurora.apache.org by Bill Farner <wf...@apache.org> on 2017/09/28 03:14:51 UTC

Review Request 62652: Remove legacy commons ZK code

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

Review request for Aurora, David McLaughlin and John Sirois.


Repository: aurora


Description
-------

This is the result removing the deprecated `-zk_use_curator` arg in `FlaggedZooKeeperConfig.java`, and following the trail of unused code.  Thanks for the clean separation, John!


Diffs
-----

  RELEASE-NOTES.md fd2618fee8ef05091849e177bd99fc321548be90 
  commons/src/main/java/org/apache/aurora/common/net/pool/DynamicHostSet.java df469efac2d994517d9b931cb2c1582e09d0cab7 
  commons/src/main/java/org/apache/aurora/common/zookeeper/Candidate.java 75c1b1493eb178ea035a0e60f20df694626c7f50 
  commons/src/main/java/org/apache/aurora/common/zookeeper/CandidateImpl.java 98b5ee4690482683526f2e8ce4c965ed052b0232 
  commons/src/main/java/org/apache/aurora/common/zookeeper/Group.java 2720dd13a3cc5e39f7242664fbe98f00fb75eb08 
  commons/src/main/java/org/apache/aurora/common/zookeeper/ServerSet.java aeea02d81faa3c8841edb9002498515dd0b52242 
  commons/src/main/java/org/apache/aurora/common/zookeeper/ServerSetImpl.java ace4980d3e15dc830fea48b83f0688347c71d900 
  commons/src/main/java/org/apache/aurora/common/zookeeper/ServerSets.java 01a54a5718388df7f02992388653d7b049422787 
  commons/src/main/java/org/apache/aurora/common/zookeeper/SingletonServiceImpl.java d9978a926e05e4303a168c7c51ee226a7dc94336 
  commons/src/main/java/org/apache/aurora/common/zookeeper/ZooKeeperClient.java ce243fb44266b741ff3f14eb5438ff38cf46133f 
  commons/src/main/java/org/apache/aurora/common/zookeeper/ZooKeeperUtils.java 2ada2646da7bdc3276803336cc638e23b9e61d26 
  commons/src/main/java/org/apache/aurora/common/zookeeper/testing/BaseZooKeeperClientTest.java ba09279be9d8bfde79b7610a184bb472ab0396b5 
  commons/src/main/java/org/apache/aurora/common/zookeeper/testing/ZooKeeperTestServer.java 29204cda66343c5b3c80d167eccd36abfb6ee641 
  commons/src/test/java/org/apache/aurora/common/zookeeper/CandidateImplTest.java 9c0cebe888d966fd64d17778c354136df0e13c7e 
  commons/src/test/java/org/apache/aurora/common/zookeeper/GroupTest.java 97a42d1bd6c60598d7936a2c065e163c1505ef3d 
  commons/src/test/java/org/apache/aurora/common/zookeeper/JsonCodecTest.java 2166123e9acd701e55e10b450a9eb89054c7afdd 
  commons/src/test/java/org/apache/aurora/common/zookeeper/ServerSetImplTest.java f0c0cb4491c7b5c34103864ee244a27db3972a94 
  commons/src/test/java/org/apache/aurora/common/zookeeper/ServerSetsTest.java 0e6719154ee5195111db51d323fa35c3bc56ec2b 
  commons/src/test/java/org/apache/aurora/common/zookeeper/SingletonServiceImplTest.java 5f6cdd83ca6c6338f8c7a0a3c93b960c036cb05e 
  commons/src/test/java/org/apache/aurora/common/zookeeper/ZooKeeperClientTest.java 5eee235162e938ecdfdb7d3c4d31d848a34a648b 
  commons/src/test/java/org/apache/aurora/common/zookeeper/ZooKeeperUtilsTest.java 9e482a6162abe5f9864d0ca0094cd97dc8036496 
  src/main/java/org/apache/aurora/scheduler/discovery/CommonsServiceDiscoveryModule.java 339f63b94e996e13514e6a6dcd7bbeef3a0f09f8 
  src/main/java/org/apache/aurora/scheduler/discovery/CommonsServiceGroupMonitor.java 9161455c3101fb0d83633883a67ce0ffe22fcdf8 
  src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceDiscoveryModule.java ea167a89a95f0f35d53a34631ae44e0a911a6817 
  src/main/java/org/apache/aurora/scheduler/discovery/FlaggedZooKeeperConfig.java e8aafe4a47e3f0e6312c8b93dbc32e1c25445dd3 
  src/main/java/org/apache/aurora/scheduler/discovery/ServiceDiscoveryModule.java 917a5679699581fb345d51d2837f372411d4beda 
  src/main/java/org/apache/aurora/scheduler/discovery/ZooKeeperConfig.java 4014a91299d863337c5f58a0495775c5f4aa6c09 
  src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java a7c9c83eebbbea7ae8a6c807f98d3ce8bd050137 
  src/test/java/org/apache/aurora/scheduler/discovery/AbstractDiscoveryModuleTest.java 0f2121ef1453b41286f9713d5ce89a26d1cb1b4f 
  src/test/java/org/apache/aurora/scheduler/discovery/BaseCuratorDiscoveryTest.java 226b0686f91539decd7d421a28d431209bcec790 
  src/test/java/org/apache/aurora/scheduler/discovery/CommonsDiscoveryModuleTest.java 7a4c4dd932fa0a69f22bcbb5cba7750bba52e16d 
  src/test/java/org/apache/aurora/scheduler/discovery/CommonsServiceGroupMonitorTest.java 42a2224839f00e6938100fc315874c531cb37096 
  src/test/java/org/apache/aurora/scheduler/discovery/CuratorDiscoveryModuleTest.java f1a02e4becc5865e72aaf96334c53cb9c395d09a 
  src/test/java/org/apache/aurora/scheduler/discovery/ZooKeeperConfigTest.java a065505134c1ad30ba9f4d4cd829355b2acc9a13 
  src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java d21a38ed66253eb7a2ceefaf6c0ec1b788036552 


Diff: https://reviews.apache.org/r/62652/diff/1/


Testing
-------

`./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh`


Thanks,

Bill Farner

Re: Review Request 62652: Remove legacy commons ZK code

Posted by John Sirois <js...@apache.org>.

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


Ship it!




Ship It!

- John Sirois


On Oct. 8, 2017, 11:29 a.m., Bill Farner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62652/
> -----------------------------------------------------------
> 
> (Updated Oct. 8, 2017, 11:29 a.m.)
> 
> 
> Review request for Aurora, David McLaughlin and John Sirois.
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> This is the result removing the deprecated `-zk_use_curator` arg in `FlaggedZooKeeperConfig.java`, and following the trail of unused code.  Thanks for the clean separation, John!
> 
> 
> Diffs
> -----
> 
>   RELEASE-NOTES.md c58e68080beb1740d92639601ef7cc29c63be37e 
>   commons/src/main/java/org/apache/aurora/common/net/pool/DynamicHostSet.java df469efac2d994517d9b931cb2c1582e09d0cab7 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/Candidate.java 75c1b1493eb178ea035a0e60f20df694626c7f50 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/CandidateImpl.java 98b5ee4690482683526f2e8ce4c965ed052b0232 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/Group.java 2720dd13a3cc5e39f7242664fbe98f00fb75eb08 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ServerSet.java aeea02d81faa3c8841edb9002498515dd0b52242 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ServerSetImpl.java ace4980d3e15dc830fea48b83f0688347c71d900 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ServerSets.java 01a54a5718388df7f02992388653d7b049422787 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/SingletonServiceImpl.java d9978a926e05e4303a168c7c51ee226a7dc94336 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ZooKeeperClient.java ce243fb44266b741ff3f14eb5438ff38cf46133f 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ZooKeeperUtils.java 2ada2646da7bdc3276803336cc638e23b9e61d26 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/testing/BaseZooKeeperClientTest.java ba09279be9d8bfde79b7610a184bb472ab0396b5 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/testing/ZooKeeperTestServer.java 29204cda66343c5b3c80d167eccd36abfb6ee641 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/CandidateImplTest.java 9c0cebe888d966fd64d17778c354136df0e13c7e 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/GroupTest.java 97a42d1bd6c60598d7936a2c065e163c1505ef3d 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/JsonCodecTest.java 2166123e9acd701e55e10b450a9eb89054c7afdd 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/ServerSetImplTest.java f0c0cb4491c7b5c34103864ee244a27db3972a94 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/ServerSetsTest.java 0e6719154ee5195111db51d323fa35c3bc56ec2b 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/SingletonServiceImplTest.java 5f6cdd83ca6c6338f8c7a0a3c93b960c036cb05e 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/ZooKeeperClientTest.java 5eee235162e938ecdfdb7d3c4d31d848a34a648b 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/ZooKeeperUtilsTest.java 9e482a6162abe5f9864d0ca0094cd97dc8036496 
>   src/main/java/org/apache/aurora/scheduler/discovery/CommonsServiceDiscoveryModule.java 339f63b94e996e13514e6a6dcd7bbeef3a0f09f8 
>   src/main/java/org/apache/aurora/scheduler/discovery/CommonsServiceGroupMonitor.java 9161455c3101fb0d83633883a67ce0ffe22fcdf8 
>   src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceDiscoveryModule.java ea167a89a95f0f35d53a34631ae44e0a911a6817 
>   src/main/java/org/apache/aurora/scheduler/discovery/FlaggedZooKeeperConfig.java e8aafe4a47e3f0e6312c8b93dbc32e1c25445dd3 
>   src/main/java/org/apache/aurora/scheduler/discovery/ServiceDiscoveryModule.java 917a5679699581fb345d51d2837f372411d4beda 
>   src/main/java/org/apache/aurora/scheduler/discovery/ZooKeeperConfig.java 4014a91299d863337c5f58a0495775c5f4aa6c09 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java a7c9c83eebbbea7ae8a6c807f98d3ce8bd050137 
>   src/test/java/org/apache/aurora/scheduler/discovery/AbstractDiscoveryModuleTest.java 0f2121ef1453b41286f9713d5ce89a26d1cb1b4f 
>   src/test/java/org/apache/aurora/scheduler/discovery/BaseCuratorDiscoveryTest.java 226b0686f91539decd7d421a28d431209bcec790 
>   src/test/java/org/apache/aurora/scheduler/discovery/CommonsDiscoveryModuleTest.java 7a4c4dd932fa0a69f22bcbb5cba7750bba52e16d 
>   src/test/java/org/apache/aurora/scheduler/discovery/CommonsServiceGroupMonitorTest.java 42a2224839f00e6938100fc315874c531cb37096 
>   src/test/java/org/apache/aurora/scheduler/discovery/CuratorDiscoveryModuleTest.java f1a02e4becc5865e72aaf96334c53cb9c395d09a 
>   src/test/java/org/apache/aurora/scheduler/discovery/ZooKeeperConfigTest.java a065505134c1ad30ba9f4d4cd829355b2acc9a13 
>   src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java d21a38ed66253eb7a2ceefaf6c0ec1b788036552 
> 
> 
> Diff: https://reviews.apache.org/r/62652/diff/2/
> 
> 
> Testing
> -------
> 
> `./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh`
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 62652: Remove legacy commons ZK code

Posted by Jordan Ly <jo...@gmail.com>.

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


Ship it!




Ship It!

- Jordan Ly


On Oct. 8, 2017, 5:29 p.m., Bill Farner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62652/
> -----------------------------------------------------------
> 
> (Updated Oct. 8, 2017, 5:29 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and John Sirois.
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> This is the result removing the deprecated `-zk_use_curator` arg in `FlaggedZooKeeperConfig.java`, and following the trail of unused code.  Thanks for the clean separation, John!
> 
> 
> Diffs
> -----
> 
>   RELEASE-NOTES.md c58e68080beb1740d92639601ef7cc29c63be37e 
>   commons/src/main/java/org/apache/aurora/common/net/pool/DynamicHostSet.java df469efac2d994517d9b931cb2c1582e09d0cab7 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/Candidate.java 75c1b1493eb178ea035a0e60f20df694626c7f50 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/CandidateImpl.java 98b5ee4690482683526f2e8ce4c965ed052b0232 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/Group.java 2720dd13a3cc5e39f7242664fbe98f00fb75eb08 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ServerSet.java aeea02d81faa3c8841edb9002498515dd0b52242 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ServerSetImpl.java ace4980d3e15dc830fea48b83f0688347c71d900 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ServerSets.java 01a54a5718388df7f02992388653d7b049422787 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/SingletonServiceImpl.java d9978a926e05e4303a168c7c51ee226a7dc94336 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ZooKeeperClient.java ce243fb44266b741ff3f14eb5438ff38cf46133f 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ZooKeeperUtils.java 2ada2646da7bdc3276803336cc638e23b9e61d26 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/testing/BaseZooKeeperClientTest.java ba09279be9d8bfde79b7610a184bb472ab0396b5 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/testing/ZooKeeperTestServer.java 29204cda66343c5b3c80d167eccd36abfb6ee641 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/CandidateImplTest.java 9c0cebe888d966fd64d17778c354136df0e13c7e 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/GroupTest.java 97a42d1bd6c60598d7936a2c065e163c1505ef3d 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/JsonCodecTest.java 2166123e9acd701e55e10b450a9eb89054c7afdd 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/ServerSetImplTest.java f0c0cb4491c7b5c34103864ee244a27db3972a94 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/ServerSetsTest.java 0e6719154ee5195111db51d323fa35c3bc56ec2b 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/SingletonServiceImplTest.java 5f6cdd83ca6c6338f8c7a0a3c93b960c036cb05e 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/ZooKeeperClientTest.java 5eee235162e938ecdfdb7d3c4d31d848a34a648b 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/ZooKeeperUtilsTest.java 9e482a6162abe5f9864d0ca0094cd97dc8036496 
>   src/main/java/org/apache/aurora/scheduler/discovery/CommonsServiceDiscoveryModule.java 339f63b94e996e13514e6a6dcd7bbeef3a0f09f8 
>   src/main/java/org/apache/aurora/scheduler/discovery/CommonsServiceGroupMonitor.java 9161455c3101fb0d83633883a67ce0ffe22fcdf8 
>   src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceDiscoveryModule.java ea167a89a95f0f35d53a34631ae44e0a911a6817 
>   src/main/java/org/apache/aurora/scheduler/discovery/FlaggedZooKeeperConfig.java c2e8ce24bbb029a1c52cbdabce19a98029bc33f2 
>   src/main/java/org/apache/aurora/scheduler/discovery/ServiceDiscoveryModule.java 917a5679699581fb345d51d2837f372411d4beda 
>   src/main/java/org/apache/aurora/scheduler/discovery/ZooKeeperConfig.java f6faca56cbfceeb9e1226c0e8feec13c7c772d94 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java a363e70176881518653d0774e9d0c4be0f7f6d78 
>   src/test/java/org/apache/aurora/scheduler/config/CommandLineTest.java 9b4f2ad15ab5b61d4cccfad38ba48f17e7853425 
>   src/test/java/org/apache/aurora/scheduler/discovery/AbstractDiscoveryModuleTest.java 0f2121ef1453b41286f9713d5ce89a26d1cb1b4f 
>   src/test/java/org/apache/aurora/scheduler/discovery/BaseCuratorDiscoveryTest.java 226b0686f91539decd7d421a28d431209bcec790 
>   src/test/java/org/apache/aurora/scheduler/discovery/CommonsDiscoveryModuleTest.java 7a4c4dd932fa0a69f22bcbb5cba7750bba52e16d 
>   src/test/java/org/apache/aurora/scheduler/discovery/CommonsServiceGroupMonitorTest.java 42a2224839f00e6938100fc315874c531cb37096 
>   src/test/java/org/apache/aurora/scheduler/discovery/CuratorDiscoveryModuleTest.java f1a02e4becc5865e72aaf96334c53cb9c395d09a 
>   src/test/java/org/apache/aurora/scheduler/discovery/ZooKeeperConfigTest.java a065505134c1ad30ba9f4d4cd829355b2acc9a13 
>   src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java d21a38ed66253eb7a2ceefaf6c0ec1b788036552 
> 
> 
> Diff: https://reviews.apache.org/r/62652/diff/3/
> 
> 
> Testing
> -------
> 
> `./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh`
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 62652: Remove legacy commons ZK code

Posted by Aurora ReviewBot <wf...@apache.org>.

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



Master (2cb257b) 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 Oct. 8, 2017, 5:29 p.m., Bill Farner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62652/
> -----------------------------------------------------------
> 
> (Updated Oct. 8, 2017, 5:29 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and John Sirois.
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> This is the result removing the deprecated `-zk_use_curator` arg in `FlaggedZooKeeperConfig.java`, and following the trail of unused code.  Thanks for the clean separation, John!
> 
> 
> Diffs
> -----
> 
>   RELEASE-NOTES.md c58e68080beb1740d92639601ef7cc29c63be37e 
>   commons/src/main/java/org/apache/aurora/common/net/pool/DynamicHostSet.java df469efac2d994517d9b931cb2c1582e09d0cab7 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/Candidate.java 75c1b1493eb178ea035a0e60f20df694626c7f50 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/CandidateImpl.java 98b5ee4690482683526f2e8ce4c965ed052b0232 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/Group.java 2720dd13a3cc5e39f7242664fbe98f00fb75eb08 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ServerSet.java aeea02d81faa3c8841edb9002498515dd0b52242 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ServerSetImpl.java ace4980d3e15dc830fea48b83f0688347c71d900 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ServerSets.java 01a54a5718388df7f02992388653d7b049422787 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/SingletonServiceImpl.java d9978a926e05e4303a168c7c51ee226a7dc94336 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ZooKeeperClient.java ce243fb44266b741ff3f14eb5438ff38cf46133f 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ZooKeeperUtils.java 2ada2646da7bdc3276803336cc638e23b9e61d26 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/testing/BaseZooKeeperClientTest.java ba09279be9d8bfde79b7610a184bb472ab0396b5 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/testing/ZooKeeperTestServer.java 29204cda66343c5b3c80d167eccd36abfb6ee641 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/CandidateImplTest.java 9c0cebe888d966fd64d17778c354136df0e13c7e 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/GroupTest.java 97a42d1bd6c60598d7936a2c065e163c1505ef3d 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/JsonCodecTest.java 2166123e9acd701e55e10b450a9eb89054c7afdd 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/ServerSetImplTest.java f0c0cb4491c7b5c34103864ee244a27db3972a94 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/ServerSetsTest.java 0e6719154ee5195111db51d323fa35c3bc56ec2b 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/SingletonServiceImplTest.java 5f6cdd83ca6c6338f8c7a0a3c93b960c036cb05e 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/ZooKeeperClientTest.java 5eee235162e938ecdfdb7d3c4d31d848a34a648b 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/ZooKeeperUtilsTest.java 9e482a6162abe5f9864d0ca0094cd97dc8036496 
>   src/main/java/org/apache/aurora/scheduler/discovery/CommonsServiceDiscoveryModule.java 339f63b94e996e13514e6a6dcd7bbeef3a0f09f8 
>   src/main/java/org/apache/aurora/scheduler/discovery/CommonsServiceGroupMonitor.java 9161455c3101fb0d83633883a67ce0ffe22fcdf8 
>   src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceDiscoveryModule.java ea167a89a95f0f35d53a34631ae44e0a911a6817 
>   src/main/java/org/apache/aurora/scheduler/discovery/FlaggedZooKeeperConfig.java e8aafe4a47e3f0e6312c8b93dbc32e1c25445dd3 
>   src/main/java/org/apache/aurora/scheduler/discovery/ServiceDiscoveryModule.java 917a5679699581fb345d51d2837f372411d4beda 
>   src/main/java/org/apache/aurora/scheduler/discovery/ZooKeeperConfig.java 4014a91299d863337c5f58a0495775c5f4aa6c09 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java a7c9c83eebbbea7ae8a6c807f98d3ce8bd050137 
>   src/test/java/org/apache/aurora/scheduler/discovery/AbstractDiscoveryModuleTest.java 0f2121ef1453b41286f9713d5ce89a26d1cb1b4f 
>   src/test/java/org/apache/aurora/scheduler/discovery/BaseCuratorDiscoveryTest.java 226b0686f91539decd7d421a28d431209bcec790 
>   src/test/java/org/apache/aurora/scheduler/discovery/CommonsDiscoveryModuleTest.java 7a4c4dd932fa0a69f22bcbb5cba7750bba52e16d 
>   src/test/java/org/apache/aurora/scheduler/discovery/CommonsServiceGroupMonitorTest.java 42a2224839f00e6938100fc315874c531cb37096 
>   src/test/java/org/apache/aurora/scheduler/discovery/CuratorDiscoveryModuleTest.java f1a02e4becc5865e72aaf96334c53cb9c395d09a 
>   src/test/java/org/apache/aurora/scheduler/discovery/ZooKeeperConfigTest.java a065505134c1ad30ba9f4d4cd829355b2acc9a13 
>   src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java d21a38ed66253eb7a2ceefaf6c0ec1b788036552 
> 
> 
> Diff: https://reviews.apache.org/r/62652/diff/2/
> 
> 
> Testing
> -------
> 
> `./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh`
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 62652: Remove legacy commons ZK code

Posted by Jordan Ly <jo...@gmail.com>.

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


Ship it!




Ship It!

- Jordan Ly


On Oct. 8, 2017, 5:29 p.m., Bill Farner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62652/
> -----------------------------------------------------------
> 
> (Updated Oct. 8, 2017, 5:29 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and John Sirois.
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> This is the result removing the deprecated `-zk_use_curator` arg in `FlaggedZooKeeperConfig.java`, and following the trail of unused code.  Thanks for the clean separation, John!
> 
> 
> Diffs
> -----
> 
>   RELEASE-NOTES.md c58e68080beb1740d92639601ef7cc29c63be37e 
>   commons/src/main/java/org/apache/aurora/common/net/pool/DynamicHostSet.java df469efac2d994517d9b931cb2c1582e09d0cab7 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/Candidate.java 75c1b1493eb178ea035a0e60f20df694626c7f50 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/CandidateImpl.java 98b5ee4690482683526f2e8ce4c965ed052b0232 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/Group.java 2720dd13a3cc5e39f7242664fbe98f00fb75eb08 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ServerSet.java aeea02d81faa3c8841edb9002498515dd0b52242 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ServerSetImpl.java ace4980d3e15dc830fea48b83f0688347c71d900 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ServerSets.java 01a54a5718388df7f02992388653d7b049422787 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/SingletonServiceImpl.java d9978a926e05e4303a168c7c51ee226a7dc94336 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ZooKeeperClient.java ce243fb44266b741ff3f14eb5438ff38cf46133f 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ZooKeeperUtils.java 2ada2646da7bdc3276803336cc638e23b9e61d26 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/testing/BaseZooKeeperClientTest.java ba09279be9d8bfde79b7610a184bb472ab0396b5 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/testing/ZooKeeperTestServer.java 29204cda66343c5b3c80d167eccd36abfb6ee641 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/CandidateImplTest.java 9c0cebe888d966fd64d17778c354136df0e13c7e 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/GroupTest.java 97a42d1bd6c60598d7936a2c065e163c1505ef3d 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/JsonCodecTest.java 2166123e9acd701e55e10b450a9eb89054c7afdd 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/ServerSetImplTest.java f0c0cb4491c7b5c34103864ee244a27db3972a94 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/ServerSetsTest.java 0e6719154ee5195111db51d323fa35c3bc56ec2b 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/SingletonServiceImplTest.java 5f6cdd83ca6c6338f8c7a0a3c93b960c036cb05e 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/ZooKeeperClientTest.java 5eee235162e938ecdfdb7d3c4d31d848a34a648b 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/ZooKeeperUtilsTest.java 9e482a6162abe5f9864d0ca0094cd97dc8036496 
>   src/main/java/org/apache/aurora/scheduler/discovery/CommonsServiceDiscoveryModule.java 339f63b94e996e13514e6a6dcd7bbeef3a0f09f8 
>   src/main/java/org/apache/aurora/scheduler/discovery/CommonsServiceGroupMonitor.java 9161455c3101fb0d83633883a67ce0ffe22fcdf8 
>   src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceDiscoveryModule.java ea167a89a95f0f35d53a34631ae44e0a911a6817 
>   src/main/java/org/apache/aurora/scheduler/discovery/FlaggedZooKeeperConfig.java c2e8ce24bbb029a1c52cbdabce19a98029bc33f2 
>   src/main/java/org/apache/aurora/scheduler/discovery/ServiceDiscoveryModule.java 917a5679699581fb345d51d2837f372411d4beda 
>   src/main/java/org/apache/aurora/scheduler/discovery/ZooKeeperConfig.java f6faca56cbfceeb9e1226c0e8feec13c7c772d94 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java a363e70176881518653d0774e9d0c4be0f7f6d78 
>   src/test/java/org/apache/aurora/scheduler/config/CommandLineTest.java 9b4f2ad15ab5b61d4cccfad38ba48f17e7853425 
>   src/test/java/org/apache/aurora/scheduler/discovery/AbstractDiscoveryModuleTest.java 0f2121ef1453b41286f9713d5ce89a26d1cb1b4f 
>   src/test/java/org/apache/aurora/scheduler/discovery/BaseCuratorDiscoveryTest.java 226b0686f91539decd7d421a28d431209bcec790 
>   src/test/java/org/apache/aurora/scheduler/discovery/CommonsDiscoveryModuleTest.java 7a4c4dd932fa0a69f22bcbb5cba7750bba52e16d 
>   src/test/java/org/apache/aurora/scheduler/discovery/CommonsServiceGroupMonitorTest.java 42a2224839f00e6938100fc315874c531cb37096 
>   src/test/java/org/apache/aurora/scheduler/discovery/CuratorDiscoveryModuleTest.java f1a02e4becc5865e72aaf96334c53cb9c395d09a 
>   src/test/java/org/apache/aurora/scheduler/discovery/ZooKeeperConfigTest.java a065505134c1ad30ba9f4d4cd829355b2acc9a13 
>   src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java d21a38ed66253eb7a2ceefaf6c0ec1b788036552 
> 
> 
> Diff: https://reviews.apache.org/r/62652/diff/3/
> 
> 
> Testing
> -------
> 
> `./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh`
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 62652: Remove legacy commons ZK code

Posted by Aurora ReviewBot <wf...@apache.org>.

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



Master (519e3df) 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 Oct. 8, 2017, 5:29 p.m., Bill Farner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62652/
> -----------------------------------------------------------
> 
> (Updated Oct. 8, 2017, 5:29 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and John Sirois.
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> This is the result removing the deprecated `-zk_use_curator` arg in `FlaggedZooKeeperConfig.java`, and following the trail of unused code.  Thanks for the clean separation, John!
> 
> 
> Diffs
> -----
> 
>   RELEASE-NOTES.md c58e68080beb1740d92639601ef7cc29c63be37e 
>   commons/src/main/java/org/apache/aurora/common/net/pool/DynamicHostSet.java df469efac2d994517d9b931cb2c1582e09d0cab7 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/Candidate.java 75c1b1493eb178ea035a0e60f20df694626c7f50 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/CandidateImpl.java 98b5ee4690482683526f2e8ce4c965ed052b0232 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/Group.java 2720dd13a3cc5e39f7242664fbe98f00fb75eb08 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ServerSet.java aeea02d81faa3c8841edb9002498515dd0b52242 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ServerSetImpl.java ace4980d3e15dc830fea48b83f0688347c71d900 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ServerSets.java 01a54a5718388df7f02992388653d7b049422787 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/SingletonServiceImpl.java d9978a926e05e4303a168c7c51ee226a7dc94336 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ZooKeeperClient.java ce243fb44266b741ff3f14eb5438ff38cf46133f 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ZooKeeperUtils.java 2ada2646da7bdc3276803336cc638e23b9e61d26 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/testing/BaseZooKeeperClientTest.java ba09279be9d8bfde79b7610a184bb472ab0396b5 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/testing/ZooKeeperTestServer.java 29204cda66343c5b3c80d167eccd36abfb6ee641 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/CandidateImplTest.java 9c0cebe888d966fd64d17778c354136df0e13c7e 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/GroupTest.java 97a42d1bd6c60598d7936a2c065e163c1505ef3d 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/JsonCodecTest.java 2166123e9acd701e55e10b450a9eb89054c7afdd 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/ServerSetImplTest.java f0c0cb4491c7b5c34103864ee244a27db3972a94 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/ServerSetsTest.java 0e6719154ee5195111db51d323fa35c3bc56ec2b 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/SingletonServiceImplTest.java 5f6cdd83ca6c6338f8c7a0a3c93b960c036cb05e 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/ZooKeeperClientTest.java 5eee235162e938ecdfdb7d3c4d31d848a34a648b 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/ZooKeeperUtilsTest.java 9e482a6162abe5f9864d0ca0094cd97dc8036496 
>   src/main/java/org/apache/aurora/scheduler/discovery/CommonsServiceDiscoveryModule.java 339f63b94e996e13514e6a6dcd7bbeef3a0f09f8 
>   src/main/java/org/apache/aurora/scheduler/discovery/CommonsServiceGroupMonitor.java 9161455c3101fb0d83633883a67ce0ffe22fcdf8 
>   src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceDiscoveryModule.java ea167a89a95f0f35d53a34631ae44e0a911a6817 
>   src/main/java/org/apache/aurora/scheduler/discovery/FlaggedZooKeeperConfig.java c2e8ce24bbb029a1c52cbdabce19a98029bc33f2 
>   src/main/java/org/apache/aurora/scheduler/discovery/ServiceDiscoveryModule.java 917a5679699581fb345d51d2837f372411d4beda 
>   src/main/java/org/apache/aurora/scheduler/discovery/ZooKeeperConfig.java f6faca56cbfceeb9e1226c0e8feec13c7c772d94 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java a363e70176881518653d0774e9d0c4be0f7f6d78 
>   src/test/java/org/apache/aurora/scheduler/config/CommandLineTest.java 9b4f2ad15ab5b61d4cccfad38ba48f17e7853425 
>   src/test/java/org/apache/aurora/scheduler/discovery/AbstractDiscoveryModuleTest.java 0f2121ef1453b41286f9713d5ce89a26d1cb1b4f 
>   src/test/java/org/apache/aurora/scheduler/discovery/BaseCuratorDiscoveryTest.java 226b0686f91539decd7d421a28d431209bcec790 
>   src/test/java/org/apache/aurora/scheduler/discovery/CommonsDiscoveryModuleTest.java 7a4c4dd932fa0a69f22bcbb5cba7750bba52e16d 
>   src/test/java/org/apache/aurora/scheduler/discovery/CommonsServiceGroupMonitorTest.java 42a2224839f00e6938100fc315874c531cb37096 
>   src/test/java/org/apache/aurora/scheduler/discovery/CuratorDiscoveryModuleTest.java f1a02e4becc5865e72aaf96334c53cb9c395d09a 
>   src/test/java/org/apache/aurora/scheduler/discovery/ZooKeeperConfigTest.java a065505134c1ad30ba9f4d4cd829355b2acc9a13 
>   src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java d21a38ed66253eb7a2ceefaf6c0ec1b788036552 
> 
> 
> Diff: https://reviews.apache.org/r/62652/diff/3/
> 
> 
> Testing
> -------
> 
> `./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh`
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 62652: Remove legacy commons ZK code

Posted by Stephan Erb <se...@apache.org>.

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


Ship it!




Twitter should test this internally. However, from my (limited) perspectice Curator is ready.

- Stephan Erb


On Oct. 8, 2017, 7:29 p.m., Bill Farner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62652/
> -----------------------------------------------------------
> 
> (Updated Oct. 8, 2017, 7:29 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and John Sirois.
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> This is the result removing the deprecated `-zk_use_curator` arg in `FlaggedZooKeeperConfig.java`, and following the trail of unused code.  Thanks for the clean separation, John!
> 
> 
> Diffs
> -----
> 
>   RELEASE-NOTES.md c58e68080beb1740d92639601ef7cc29c63be37e 
>   commons/src/main/java/org/apache/aurora/common/net/pool/DynamicHostSet.java df469efac2d994517d9b931cb2c1582e09d0cab7 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/Candidate.java 75c1b1493eb178ea035a0e60f20df694626c7f50 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/CandidateImpl.java 98b5ee4690482683526f2e8ce4c965ed052b0232 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/Group.java 2720dd13a3cc5e39f7242664fbe98f00fb75eb08 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ServerSet.java aeea02d81faa3c8841edb9002498515dd0b52242 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ServerSetImpl.java ace4980d3e15dc830fea48b83f0688347c71d900 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ServerSets.java 01a54a5718388df7f02992388653d7b049422787 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/SingletonServiceImpl.java d9978a926e05e4303a168c7c51ee226a7dc94336 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ZooKeeperClient.java ce243fb44266b741ff3f14eb5438ff38cf46133f 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ZooKeeperUtils.java 2ada2646da7bdc3276803336cc638e23b9e61d26 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/testing/BaseZooKeeperClientTest.java ba09279be9d8bfde79b7610a184bb472ab0396b5 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/testing/ZooKeeperTestServer.java 29204cda66343c5b3c80d167eccd36abfb6ee641 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/CandidateImplTest.java 9c0cebe888d966fd64d17778c354136df0e13c7e 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/GroupTest.java 97a42d1bd6c60598d7936a2c065e163c1505ef3d 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/JsonCodecTest.java 2166123e9acd701e55e10b450a9eb89054c7afdd 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/ServerSetImplTest.java f0c0cb4491c7b5c34103864ee244a27db3972a94 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/ServerSetsTest.java 0e6719154ee5195111db51d323fa35c3bc56ec2b 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/SingletonServiceImplTest.java 5f6cdd83ca6c6338f8c7a0a3c93b960c036cb05e 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/ZooKeeperClientTest.java 5eee235162e938ecdfdb7d3c4d31d848a34a648b 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/ZooKeeperUtilsTest.java 9e482a6162abe5f9864d0ca0094cd97dc8036496 
>   src/main/java/org/apache/aurora/scheduler/discovery/CommonsServiceDiscoveryModule.java 339f63b94e996e13514e6a6dcd7bbeef3a0f09f8 
>   src/main/java/org/apache/aurora/scheduler/discovery/CommonsServiceGroupMonitor.java 9161455c3101fb0d83633883a67ce0ffe22fcdf8 
>   src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceDiscoveryModule.java ea167a89a95f0f35d53a34631ae44e0a911a6817 
>   src/main/java/org/apache/aurora/scheduler/discovery/FlaggedZooKeeperConfig.java c2e8ce24bbb029a1c52cbdabce19a98029bc33f2 
>   src/main/java/org/apache/aurora/scheduler/discovery/ServiceDiscoveryModule.java 917a5679699581fb345d51d2837f372411d4beda 
>   src/main/java/org/apache/aurora/scheduler/discovery/ZooKeeperConfig.java f6faca56cbfceeb9e1226c0e8feec13c7c772d94 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java a363e70176881518653d0774e9d0c4be0f7f6d78 
>   src/test/java/org/apache/aurora/scheduler/config/CommandLineTest.java 9b4f2ad15ab5b61d4cccfad38ba48f17e7853425 
>   src/test/java/org/apache/aurora/scheduler/discovery/AbstractDiscoveryModuleTest.java 0f2121ef1453b41286f9713d5ce89a26d1cb1b4f 
>   src/test/java/org/apache/aurora/scheduler/discovery/BaseCuratorDiscoveryTest.java 226b0686f91539decd7d421a28d431209bcec790 
>   src/test/java/org/apache/aurora/scheduler/discovery/CommonsDiscoveryModuleTest.java 7a4c4dd932fa0a69f22bcbb5cba7750bba52e16d 
>   src/test/java/org/apache/aurora/scheduler/discovery/CommonsServiceGroupMonitorTest.java 42a2224839f00e6938100fc315874c531cb37096 
>   src/test/java/org/apache/aurora/scheduler/discovery/CuratorDiscoveryModuleTest.java f1a02e4becc5865e72aaf96334c53cb9c395d09a 
>   src/test/java/org/apache/aurora/scheduler/discovery/ZooKeeperConfigTest.java a065505134c1ad30ba9f4d4cd829355b2acc9a13 
>   src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java d21a38ed66253eb7a2ceefaf6c0ec1b788036552 
> 
> 
> Diff: https://reviews.apache.org/r/62652/diff/3/
> 
> 
> Testing
> -------
> 
> `./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh`
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 62652: Remove legacy commons ZK code

Posted by Aurora ReviewBot <wf...@apache.org>.

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



Master (c638877) 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 Oct. 8, 2017, 5:29 p.m., Bill Farner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62652/
> -----------------------------------------------------------
> 
> (Updated Oct. 8, 2017, 5:29 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and John Sirois.
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> This is the result removing the deprecated `-zk_use_curator` arg in `FlaggedZooKeeperConfig.java`, and following the trail of unused code.  Thanks for the clean separation, John!
> 
> 
> Diffs
> -----
> 
>   RELEASE-NOTES.md 079f495d2f41272456daec5caca0944aa2d7fafc 
>   commons/src/main/java/org/apache/aurora/common/net/pool/DynamicHostSet.java df469efac2d994517d9b931cb2c1582e09d0cab7 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/Candidate.java 75c1b1493eb178ea035a0e60f20df694626c7f50 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/CandidateImpl.java 98b5ee4690482683526f2e8ce4c965ed052b0232 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/Group.java 2720dd13a3cc5e39f7242664fbe98f00fb75eb08 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ServerSet.java aeea02d81faa3c8841edb9002498515dd0b52242 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ServerSetImpl.java ace4980d3e15dc830fea48b83f0688347c71d900 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ServerSets.java 01a54a5718388df7f02992388653d7b049422787 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/SingletonServiceImpl.java d9978a926e05e4303a168c7c51ee226a7dc94336 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ZooKeeperClient.java ce243fb44266b741ff3f14eb5438ff38cf46133f 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ZooKeeperUtils.java 93ddd89ec433ed2ca7254063272c578156fcc215 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/testing/BaseZooKeeperClientTest.java ba09279be9d8bfde79b7610a184bb472ab0396b5 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/testing/ZooKeeperTestServer.java 29204cda66343c5b3c80d167eccd36abfb6ee641 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/CandidateImplTest.java 9c0cebe888d966fd64d17778c354136df0e13c7e 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/GroupTest.java 97a42d1bd6c60598d7936a2c065e163c1505ef3d 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/JsonCodecTest.java 2166123e9acd701e55e10b450a9eb89054c7afdd 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/ServerSetImplTest.java f0c0cb4491c7b5c34103864ee244a27db3972a94 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/ServerSetsTest.java 0e6719154ee5195111db51d323fa35c3bc56ec2b 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/SingletonServiceImplTest.java 5f6cdd83ca6c6338f8c7a0a3c93b960c036cb05e 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/ZooKeeperClientTest.java 5eee235162e938ecdfdb7d3c4d31d848a34a648b 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/ZooKeeperUtilsTest.java 9e482a6162abe5f9864d0ca0094cd97dc8036496 
>   src/main/java/org/apache/aurora/scheduler/discovery/CommonsServiceDiscoveryModule.java 339f63b94e996e13514e6a6dcd7bbeef3a0f09f8 
>   src/main/java/org/apache/aurora/scheduler/discovery/CommonsServiceGroupMonitor.java 9161455c3101fb0d83633883a67ce0ffe22fcdf8 
>   src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceDiscoveryModule.java 40cda8ceb0702b4a5417968e62597d717a79e020 
>   src/main/java/org/apache/aurora/scheduler/discovery/FlaggedZooKeeperConfig.java 1e7b9ce235c693b7c653484fe654242aecbfd3b8 
>   src/main/java/org/apache/aurora/scheduler/discovery/ServiceDiscoveryModule.java 917a5679699581fb345d51d2837f372411d4beda 
>   src/main/java/org/apache/aurora/scheduler/discovery/ZooKeeperConfig.java 433ed3174d14b5317f055e204b5bdd932682baeb 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java a363e70176881518653d0774e9d0c4be0f7f6d78 
>   src/test/java/org/apache/aurora/scheduler/config/CommandLineTest.java 1e2e01d2b2cec3ce4a1bfb6f415c92de44a7982e 
>   src/test/java/org/apache/aurora/scheduler/discovery/AbstractDiscoveryModuleTest.java cec54e553d283b23f8466034d4f055478bc5c948 
>   src/test/java/org/apache/aurora/scheduler/discovery/BaseCuratorDiscoveryTest.java 226b0686f91539decd7d421a28d431209bcec790 
>   src/test/java/org/apache/aurora/scheduler/discovery/CommonsDiscoveryModuleTest.java 7a4c4dd932fa0a69f22bcbb5cba7750bba52e16d 
>   src/test/java/org/apache/aurora/scheduler/discovery/CommonsServiceGroupMonitorTest.java 42a2224839f00e6938100fc315874c531cb37096 
>   src/test/java/org/apache/aurora/scheduler/discovery/CuratorDiscoveryModuleTest.java f1a02e4becc5865e72aaf96334c53cb9c395d09a 
>   src/test/java/org/apache/aurora/scheduler/discovery/ZooKeeperConfigTest.java baee123abad35bf509a64d34ba2e4166cef5b5fc 
>   src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java d21a38ed66253eb7a2ceefaf6c0ec1b788036552 
> 
> 
> Diff: https://reviews.apache.org/r/62652/diff/4/
> 
> 
> Testing
> -------
> 
> `./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh`
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 62652: Remove legacy commons ZK code

Posted by Jordan Ly <jo...@gmail.com>.

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


Ship it!




Ship It!

- Jordan Ly


On Oct. 8, 2017, 5:29 p.m., Bill Farner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62652/
> -----------------------------------------------------------
> 
> (Updated Oct. 8, 2017, 5:29 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and John Sirois.
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> This is the result removing the deprecated `-zk_use_curator` arg in `FlaggedZooKeeperConfig.java`, and following the trail of unused code.  Thanks for the clean separation, John!
> 
> 
> Diffs
> -----
> 
>   RELEASE-NOTES.md c58e68080beb1740d92639601ef7cc29c63be37e 
>   commons/src/main/java/org/apache/aurora/common/net/pool/DynamicHostSet.java df469efac2d994517d9b931cb2c1582e09d0cab7 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/Candidate.java 75c1b1493eb178ea035a0e60f20df694626c7f50 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/CandidateImpl.java 98b5ee4690482683526f2e8ce4c965ed052b0232 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/Group.java 2720dd13a3cc5e39f7242664fbe98f00fb75eb08 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ServerSet.java aeea02d81faa3c8841edb9002498515dd0b52242 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ServerSetImpl.java ace4980d3e15dc830fea48b83f0688347c71d900 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ServerSets.java 01a54a5718388df7f02992388653d7b049422787 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/SingletonServiceImpl.java d9978a926e05e4303a168c7c51ee226a7dc94336 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ZooKeeperClient.java ce243fb44266b741ff3f14eb5438ff38cf46133f 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ZooKeeperUtils.java 2ada2646da7bdc3276803336cc638e23b9e61d26 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/testing/BaseZooKeeperClientTest.java ba09279be9d8bfde79b7610a184bb472ab0396b5 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/testing/ZooKeeperTestServer.java 29204cda66343c5b3c80d167eccd36abfb6ee641 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/CandidateImplTest.java 9c0cebe888d966fd64d17778c354136df0e13c7e 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/GroupTest.java 97a42d1bd6c60598d7936a2c065e163c1505ef3d 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/JsonCodecTest.java 2166123e9acd701e55e10b450a9eb89054c7afdd 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/ServerSetImplTest.java f0c0cb4491c7b5c34103864ee244a27db3972a94 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/ServerSetsTest.java 0e6719154ee5195111db51d323fa35c3bc56ec2b 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/SingletonServiceImplTest.java 5f6cdd83ca6c6338f8c7a0a3c93b960c036cb05e 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/ZooKeeperClientTest.java 5eee235162e938ecdfdb7d3c4d31d848a34a648b 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/ZooKeeperUtilsTest.java 9e482a6162abe5f9864d0ca0094cd97dc8036496 
>   src/main/java/org/apache/aurora/scheduler/discovery/CommonsServiceDiscoveryModule.java 339f63b94e996e13514e6a6dcd7bbeef3a0f09f8 
>   src/main/java/org/apache/aurora/scheduler/discovery/CommonsServiceGroupMonitor.java 9161455c3101fb0d83633883a67ce0ffe22fcdf8 
>   src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceDiscoveryModule.java ea167a89a95f0f35d53a34631ae44e0a911a6817 
>   src/main/java/org/apache/aurora/scheduler/discovery/FlaggedZooKeeperConfig.java c2e8ce24bbb029a1c52cbdabce19a98029bc33f2 
>   src/main/java/org/apache/aurora/scheduler/discovery/ServiceDiscoveryModule.java 917a5679699581fb345d51d2837f372411d4beda 
>   src/main/java/org/apache/aurora/scheduler/discovery/ZooKeeperConfig.java f6faca56cbfceeb9e1226c0e8feec13c7c772d94 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java a363e70176881518653d0774e9d0c4be0f7f6d78 
>   src/test/java/org/apache/aurora/scheduler/config/CommandLineTest.java 9b4f2ad15ab5b61d4cccfad38ba48f17e7853425 
>   src/test/java/org/apache/aurora/scheduler/discovery/AbstractDiscoveryModuleTest.java 0f2121ef1453b41286f9713d5ce89a26d1cb1b4f 
>   src/test/java/org/apache/aurora/scheduler/discovery/BaseCuratorDiscoveryTest.java 226b0686f91539decd7d421a28d431209bcec790 
>   src/test/java/org/apache/aurora/scheduler/discovery/CommonsDiscoveryModuleTest.java 7a4c4dd932fa0a69f22bcbb5cba7750bba52e16d 
>   src/test/java/org/apache/aurora/scheduler/discovery/CommonsServiceGroupMonitorTest.java 42a2224839f00e6938100fc315874c531cb37096 
>   src/test/java/org/apache/aurora/scheduler/discovery/CuratorDiscoveryModuleTest.java f1a02e4becc5865e72aaf96334c53cb9c395d09a 
>   src/test/java/org/apache/aurora/scheduler/discovery/ZooKeeperConfigTest.java a065505134c1ad30ba9f4d4cd829355b2acc9a13 
>   src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java d21a38ed66253eb7a2ceefaf6c0ec1b788036552 
> 
> 
> Diff: https://reviews.apache.org/r/62652/diff/3/
> 
> 
> Testing
> -------
> 
> `./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh`
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 62652: Remove legacy commons ZK code

Posted by Bill Farner <wf...@apache.org>.

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

(Updated Oct. 8, 2017, 10:29 a.m.)


Review request for Aurora, David McLaughlin and John Sirois.


Changes
-------

rebase + green build


Repository: aurora


Description
-------

This is the result removing the deprecated `-zk_use_curator` arg in `FlaggedZooKeeperConfig.java`, and following the trail of unused code.  Thanks for the clean separation, John!


Diffs (updated)
-----

  RELEASE-NOTES.md c58e68080beb1740d92639601ef7cc29c63be37e 
  commons/src/main/java/org/apache/aurora/common/net/pool/DynamicHostSet.java df469efac2d994517d9b931cb2c1582e09d0cab7 
  commons/src/main/java/org/apache/aurora/common/zookeeper/Candidate.java 75c1b1493eb178ea035a0e60f20df694626c7f50 
  commons/src/main/java/org/apache/aurora/common/zookeeper/CandidateImpl.java 98b5ee4690482683526f2e8ce4c965ed052b0232 
  commons/src/main/java/org/apache/aurora/common/zookeeper/Group.java 2720dd13a3cc5e39f7242664fbe98f00fb75eb08 
  commons/src/main/java/org/apache/aurora/common/zookeeper/ServerSet.java aeea02d81faa3c8841edb9002498515dd0b52242 
  commons/src/main/java/org/apache/aurora/common/zookeeper/ServerSetImpl.java ace4980d3e15dc830fea48b83f0688347c71d900 
  commons/src/main/java/org/apache/aurora/common/zookeeper/ServerSets.java 01a54a5718388df7f02992388653d7b049422787 
  commons/src/main/java/org/apache/aurora/common/zookeeper/SingletonServiceImpl.java d9978a926e05e4303a168c7c51ee226a7dc94336 
  commons/src/main/java/org/apache/aurora/common/zookeeper/ZooKeeperClient.java ce243fb44266b741ff3f14eb5438ff38cf46133f 
  commons/src/main/java/org/apache/aurora/common/zookeeper/ZooKeeperUtils.java 2ada2646da7bdc3276803336cc638e23b9e61d26 
  commons/src/main/java/org/apache/aurora/common/zookeeper/testing/BaseZooKeeperClientTest.java ba09279be9d8bfde79b7610a184bb472ab0396b5 
  commons/src/main/java/org/apache/aurora/common/zookeeper/testing/ZooKeeperTestServer.java 29204cda66343c5b3c80d167eccd36abfb6ee641 
  commons/src/test/java/org/apache/aurora/common/zookeeper/CandidateImplTest.java 9c0cebe888d966fd64d17778c354136df0e13c7e 
  commons/src/test/java/org/apache/aurora/common/zookeeper/GroupTest.java 97a42d1bd6c60598d7936a2c065e163c1505ef3d 
  commons/src/test/java/org/apache/aurora/common/zookeeper/JsonCodecTest.java 2166123e9acd701e55e10b450a9eb89054c7afdd 
  commons/src/test/java/org/apache/aurora/common/zookeeper/ServerSetImplTest.java f0c0cb4491c7b5c34103864ee244a27db3972a94 
  commons/src/test/java/org/apache/aurora/common/zookeeper/ServerSetsTest.java 0e6719154ee5195111db51d323fa35c3bc56ec2b 
  commons/src/test/java/org/apache/aurora/common/zookeeper/SingletonServiceImplTest.java 5f6cdd83ca6c6338f8c7a0a3c93b960c036cb05e 
  commons/src/test/java/org/apache/aurora/common/zookeeper/ZooKeeperClientTest.java 5eee235162e938ecdfdb7d3c4d31d848a34a648b 
  commons/src/test/java/org/apache/aurora/common/zookeeper/ZooKeeperUtilsTest.java 9e482a6162abe5f9864d0ca0094cd97dc8036496 
  src/main/java/org/apache/aurora/scheduler/discovery/CommonsServiceDiscoveryModule.java 339f63b94e996e13514e6a6dcd7bbeef3a0f09f8 
  src/main/java/org/apache/aurora/scheduler/discovery/CommonsServiceGroupMonitor.java 9161455c3101fb0d83633883a67ce0ffe22fcdf8 
  src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceDiscoveryModule.java ea167a89a95f0f35d53a34631ae44e0a911a6817 
  src/main/java/org/apache/aurora/scheduler/discovery/FlaggedZooKeeperConfig.java e8aafe4a47e3f0e6312c8b93dbc32e1c25445dd3 
  src/main/java/org/apache/aurora/scheduler/discovery/ServiceDiscoveryModule.java 917a5679699581fb345d51d2837f372411d4beda 
  src/main/java/org/apache/aurora/scheduler/discovery/ZooKeeperConfig.java 4014a91299d863337c5f58a0495775c5f4aa6c09 
  src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java a7c9c83eebbbea7ae8a6c807f98d3ce8bd050137 
  src/test/java/org/apache/aurora/scheduler/discovery/AbstractDiscoveryModuleTest.java 0f2121ef1453b41286f9713d5ce89a26d1cb1b4f 
  src/test/java/org/apache/aurora/scheduler/discovery/BaseCuratorDiscoveryTest.java 226b0686f91539decd7d421a28d431209bcec790 
  src/test/java/org/apache/aurora/scheduler/discovery/CommonsDiscoveryModuleTest.java 7a4c4dd932fa0a69f22bcbb5cba7750bba52e16d 
  src/test/java/org/apache/aurora/scheduler/discovery/CommonsServiceGroupMonitorTest.java 42a2224839f00e6938100fc315874c531cb37096 
  src/test/java/org/apache/aurora/scheduler/discovery/CuratorDiscoveryModuleTest.java f1a02e4becc5865e72aaf96334c53cb9c395d09a 
  src/test/java/org/apache/aurora/scheduler/discovery/ZooKeeperConfigTest.java a065505134c1ad30ba9f4d4cd829355b2acc9a13 
  src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java d21a38ed66253eb7a2ceefaf6c0ec1b788036552 


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

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


Testing
-------

`./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh`


Thanks,

Bill Farner

Re: Review Request 62652: Remove legacy commons ZK code

Posted by Aurora ReviewBot <wf...@apache.org>.

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

Master (7c78519) is red with this patch.
  ./build-support/jenkins/build.sh

:testClasses
:compileJmhJavaNote: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/jmh/java/org/apache/aurora/benchmark/fakes/FakeSchedulerDriver.java uses or overrides a deprecated API.
Note: Recompile with -Xlint:deprecation for details.

:processJmhResources NO-SOURCE
:jmhClasses
:checkstyleJmh
:checkstyleMain
:checkstyleTest[ant:checkstyle] [ERROR] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/java/org/apache/aurora/scheduler/discovery/AbstractDiscoveryModuleTest.java:16:8: Unused import - java.net.InetSocketAddress. [UnusedImports]
[ant:checkstyle] [ERROR] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/java/org/apache/aurora/scheduler/discovery/AbstractDiscoveryModuleTest.java:18:8: Unused import - com.google.common.base.Optional. [UnusedImports]
[ant:checkstyle] [ERROR] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/java/org/apache/aurora/scheduler/discovery/AbstractDiscoveryModuleTest.java:19:8: Unused import - com.google.common.collect.ImmutableList. [UnusedImports]
[ant:checkstyle] [ERROR] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/java/org/apache/aurora/scheduler/discovery/AbstractDiscoveryModuleTest.java:20:8: Unused import - com.google.inject.AbstractModule. [UnusedImports]
[ant:checkstyle] [ERROR] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/java/org/apache/aurora/scheduler/discovery/AbstractDiscoveryModuleTest.java:22:8: Unused import - com.google.inject.Guice. [UnusedImports]
[ant:checkstyle] [ERROR] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/java/org/apache/aurora/scheduler/discovery/AbstractDiscoveryModuleTest.java:23:8: Unused import - com.google.inject.Injector. [UnusedImports]
[ant:checkstyle] [ERROR] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/java/org/apache/aurora/scheduler/discovery/AbstractDiscoveryModuleTest.java:26:8: Unused import - org.apache.aurora.common.quantity.Amount. [UnusedImports]
[ant:checkstyle] [ERROR] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/java/org/apache/aurora/scheduler/discovery/AbstractDiscoveryModuleTest.java:27:8: Unused import - org.apache.aurora.common.quantity.Time. [UnusedImports]
[ant:checkstyle] [ERROR] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/java/org/apache/aurora/scheduler/discovery/AbstractDiscoveryModuleTest.java:28:8: Unused import - org.apache.aurora.common.stats.StatsProvider. [UnusedImports]
[ant:checkstyle] [ERROR] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/java/org/apache/aurora/scheduler/discovery/AbstractDiscoveryModuleTest.java:30:8: Unused import - org.apache.aurora.common.zookeeper.Credentials. [UnusedImports]
[ant:checkstyle] [ERROR] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/java/org/apache/aurora/scheduler/discovery/AbstractDiscoveryModuleTest.java:31:8: Unused import - org.apache.aurora.common.zookeeper.SingletonService. [UnusedImports]
[ant:checkstyle] [ERROR] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/java/org/apache/aurora/scheduler/discovery/AbstractDiscoveryModuleTest.java:32:8: Unused import - org.apache.aurora.common.zookeeper.ZooKeeperUtils. [UnusedImports]
[ant:checkstyle] [ERROR] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/java/org/apache/aurora/scheduler/discovery/AbstractDiscoveryModuleTest.java:33:8: Unused import - org.apache.aurora.scheduler.app.ServiceGroupMonitor. [UnusedImports]
[ant:checkstyle] [ERROR] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/java/org/apache/aurora/scheduler/discovery/AbstractDiscoveryModuleTest.java:34:8: Unused import - org.apache.aurora.scheduler.testing.FakeStatsProvider. [UnusedImports]
[ant:checkstyle] [ERROR] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/java/org/apache/aurora/scheduler/discovery/AbstractDiscoveryModuleTest.java:35:8: Unused import - org.junit.Test. [UnusedImports]
[ant:checkstyle] [ERROR] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/java/org/apache/aurora/scheduler/discovery/AbstractDiscoveryModuleTest.java:37:15: Unused import - org.junit.Assert.assertNotNull. [UnusedImports]
[ant:checkstyle] [ERROR] /home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/java/org/apache/aurora/scheduler/discovery/AbstractDiscoveryModuleTest.java:39: File contains a sequence of empty lines. [RegexpMultiline]
 FAILED

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':checkstyleTest'.
> Checkstyle rule violations were found. See the report at: file:///home/jenkins/jenkins-slave/workspace/AuroraBot/dist/reports/checkstyle/test.html

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

* Get more help at https://help.gradle.org

BUILD FAILED in 2m 56s
38 actionable tasks: 32 executed, 6 up-to-date

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

- Aurora ReviewBot

On Sept. 28, 2017, 11:14 a.m., Bill Farner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62652/
> -----------------------------------------------------------
> 
> (Updated Sept. 28, 2017, 11:14 a.m.)
> 
> 
> Review request for Aurora, David McLaughlin and John Sirois.
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> This is the result removing the deprecated `-zk_use_curator` arg in `FlaggedZooKeeperConfig.java`, and following the trail of unused code.  Thanks for the clean separation, John!
> 
> 
> Diffs
> -----
> 
>   RELEASE-NOTES.md fd2618fee8ef05091849e177bd99fc321548be90 
>   commons/src/main/java/org/apache/aurora/common/net/pool/DynamicHostSet.java df469efac2d994517d9b931cb2c1582e09d0cab7 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/Candidate.java 75c1b1493eb178ea035a0e60f20df694626c7f50 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/CandidateImpl.java 98b5ee4690482683526f2e8ce4c965ed052b0232 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/Group.java 2720dd13a3cc5e39f7242664fbe98f00fb75eb08 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ServerSet.java aeea02d81faa3c8841edb9002498515dd0b52242 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ServerSetImpl.java ace4980d3e15dc830fea48b83f0688347c71d900 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ServerSets.java 01a54a5718388df7f02992388653d7b049422787 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/SingletonServiceImpl.java d9978a926e05e4303a168c7c51ee226a7dc94336 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ZooKeeperClient.java ce243fb44266b741ff3f14eb5438ff38cf46133f 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ZooKeeperUtils.java 2ada2646da7bdc3276803336cc638e23b9e61d26 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/testing/BaseZooKeeperClientTest.java ba09279be9d8bfde79b7610a184bb472ab0396b5 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/testing/ZooKeeperTestServer.java 29204cda66343c5b3c80d167eccd36abfb6ee641 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/CandidateImplTest.java 9c0cebe888d966fd64d17778c354136df0e13c7e 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/GroupTest.java 97a42d1bd6c60598d7936a2c065e163c1505ef3d 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/JsonCodecTest.java 2166123e9acd701e55e10b450a9eb89054c7afdd 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/ServerSetImplTest.java f0c0cb4491c7b5c34103864ee244a27db3972a94 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/ServerSetsTest.java 0e6719154ee5195111db51d323fa35c3bc56ec2b 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/SingletonServiceImplTest.java 5f6cdd83ca6c6338f8c7a0a3c93b960c036cb05e 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/ZooKeeperClientTest.java 5eee235162e938ecdfdb7d3c4d31d848a34a648b 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/ZooKeeperUtilsTest.java 9e482a6162abe5f9864d0ca0094cd97dc8036496 
>   src/main/java/org/apache/aurora/scheduler/discovery/CommonsServiceDiscoveryModule.java 339f63b94e996e13514e6a6dcd7bbeef3a0f09f8 
>   src/main/java/org/apache/aurora/scheduler/discovery/CommonsServiceGroupMonitor.java 9161455c3101fb0d83633883a67ce0ffe22fcdf8 
>   src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceDiscoveryModule.java ea167a89a95f0f35d53a34631ae44e0a911a6817 
>   src/main/java/org/apache/aurora/scheduler/discovery/FlaggedZooKeeperConfig.java e8aafe4a47e3f0e6312c8b93dbc32e1c25445dd3 
>   src/main/java/org/apache/aurora/scheduler/discovery/ServiceDiscoveryModule.java 917a5679699581fb345d51d2837f372411d4beda 
>   src/main/java/org/apache/aurora/scheduler/discovery/ZooKeeperConfig.java 4014a91299d863337c5f58a0495775c5f4aa6c09 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java a7c9c83eebbbea7ae8a6c807f98d3ce8bd050137 
>   src/test/java/org/apache/aurora/scheduler/discovery/AbstractDiscoveryModuleTest.java 0f2121ef1453b41286f9713d5ce89a26d1cb1b4f 
>   src/test/java/org/apache/aurora/scheduler/discovery/BaseCuratorDiscoveryTest.java 226b0686f91539decd7d421a28d431209bcec790 
>   src/test/java/org/apache/aurora/scheduler/discovery/CommonsDiscoveryModuleTest.java 7a4c4dd932fa0a69f22bcbb5cba7750bba52e16d 
>   src/test/java/org/apache/aurora/scheduler/discovery/CommonsServiceGroupMonitorTest.java 42a2224839f00e6938100fc315874c531cb37096 
>   src/test/java/org/apache/aurora/scheduler/discovery/CuratorDiscoveryModuleTest.java f1a02e4becc5865e72aaf96334c53cb9c395d09a 
>   src/test/java/org/apache/aurora/scheduler/discovery/ZooKeeperConfigTest.java a065505134c1ad30ba9f4d4cd829355b2acc9a13 
>   src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java d21a38ed66253eb7a2ceefaf6c0ec1b788036552 
> 
> 
> Diff: https://reviews.apache.org/r/62652/diff/1/
> 
> 
> Testing
> -------
> 
> `./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh`
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 62652: Remove legacy commons ZK code

Posted by Bill Farner <wf...@apache.org>.


> On Sept. 27, 2017, 8:22 p.m., David McLaughlin wrote:
> > -1. 
> > 
> > Please see here for details on the current status of curator in production: https://issues.apache.org/jira/browse/AURORA-1840

Aha, there was no trace of this in the code, thanks for the pointer.  I'll take a look.


- Bill


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


On Sept. 27, 2017, 8:14 p.m., Bill Farner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62652/
> -----------------------------------------------------------
> 
> (Updated Sept. 27, 2017, 8:14 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and John Sirois.
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> This is the result removing the deprecated `-zk_use_curator` arg in `FlaggedZooKeeperConfig.java`, and following the trail of unused code.  Thanks for the clean separation, John!
> 
> 
> Diffs
> -----
> 
>   RELEASE-NOTES.md fd2618fee8ef05091849e177bd99fc321548be90 
>   commons/src/main/java/org/apache/aurora/common/net/pool/DynamicHostSet.java df469efac2d994517d9b931cb2c1582e09d0cab7 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/Candidate.java 75c1b1493eb178ea035a0e60f20df694626c7f50 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/CandidateImpl.java 98b5ee4690482683526f2e8ce4c965ed052b0232 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/Group.java 2720dd13a3cc5e39f7242664fbe98f00fb75eb08 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ServerSet.java aeea02d81faa3c8841edb9002498515dd0b52242 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ServerSetImpl.java ace4980d3e15dc830fea48b83f0688347c71d900 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ServerSets.java 01a54a5718388df7f02992388653d7b049422787 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/SingletonServiceImpl.java d9978a926e05e4303a168c7c51ee226a7dc94336 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ZooKeeperClient.java ce243fb44266b741ff3f14eb5438ff38cf46133f 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ZooKeeperUtils.java 2ada2646da7bdc3276803336cc638e23b9e61d26 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/testing/BaseZooKeeperClientTest.java ba09279be9d8bfde79b7610a184bb472ab0396b5 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/testing/ZooKeeperTestServer.java 29204cda66343c5b3c80d167eccd36abfb6ee641 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/CandidateImplTest.java 9c0cebe888d966fd64d17778c354136df0e13c7e 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/GroupTest.java 97a42d1bd6c60598d7936a2c065e163c1505ef3d 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/JsonCodecTest.java 2166123e9acd701e55e10b450a9eb89054c7afdd 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/ServerSetImplTest.java f0c0cb4491c7b5c34103864ee244a27db3972a94 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/ServerSetsTest.java 0e6719154ee5195111db51d323fa35c3bc56ec2b 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/SingletonServiceImplTest.java 5f6cdd83ca6c6338f8c7a0a3c93b960c036cb05e 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/ZooKeeperClientTest.java 5eee235162e938ecdfdb7d3c4d31d848a34a648b 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/ZooKeeperUtilsTest.java 9e482a6162abe5f9864d0ca0094cd97dc8036496 
>   src/main/java/org/apache/aurora/scheduler/discovery/CommonsServiceDiscoveryModule.java 339f63b94e996e13514e6a6dcd7bbeef3a0f09f8 
>   src/main/java/org/apache/aurora/scheduler/discovery/CommonsServiceGroupMonitor.java 9161455c3101fb0d83633883a67ce0ffe22fcdf8 
>   src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceDiscoveryModule.java ea167a89a95f0f35d53a34631ae44e0a911a6817 
>   src/main/java/org/apache/aurora/scheduler/discovery/FlaggedZooKeeperConfig.java e8aafe4a47e3f0e6312c8b93dbc32e1c25445dd3 
>   src/main/java/org/apache/aurora/scheduler/discovery/ServiceDiscoveryModule.java 917a5679699581fb345d51d2837f372411d4beda 
>   src/main/java/org/apache/aurora/scheduler/discovery/ZooKeeperConfig.java 4014a91299d863337c5f58a0495775c5f4aa6c09 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java a7c9c83eebbbea7ae8a6c807f98d3ce8bd050137 
>   src/test/java/org/apache/aurora/scheduler/discovery/AbstractDiscoveryModuleTest.java 0f2121ef1453b41286f9713d5ce89a26d1cb1b4f 
>   src/test/java/org/apache/aurora/scheduler/discovery/BaseCuratorDiscoveryTest.java 226b0686f91539decd7d421a28d431209bcec790 
>   src/test/java/org/apache/aurora/scheduler/discovery/CommonsDiscoveryModuleTest.java 7a4c4dd932fa0a69f22bcbb5cba7750bba52e16d 
>   src/test/java/org/apache/aurora/scheduler/discovery/CommonsServiceGroupMonitorTest.java 42a2224839f00e6938100fc315874c531cb37096 
>   src/test/java/org/apache/aurora/scheduler/discovery/CuratorDiscoveryModuleTest.java f1a02e4becc5865e72aaf96334c53cb9c395d09a 
>   src/test/java/org/apache/aurora/scheduler/discovery/ZooKeeperConfigTest.java a065505134c1ad30ba9f4d4cd829355b2acc9a13 
>   src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java d21a38ed66253eb7a2ceefaf6c0ec1b788036552 
> 
> 
> Diff: https://reviews.apache.org/r/62652/diff/1/
> 
> 
> Testing
> -------
> 
> `./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh`
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 62652: Remove legacy commons ZK code

Posted by David McLaughlin <da...@dmclaughlin.com>.


> On Sept. 28, 2017, 3:22 a.m., David McLaughlin wrote:
> > -1. 
> > 
> > Please see here for details on the current status of curator in production: https://issues.apache.org/jira/browse/AURORA-1840
> 
> Bill Farner wrote:
>     Aha, there was no trace of this in the code, thanks for the pointer.  I'll take a look.
> 
> Renan DelValle wrote:
>     Just wanted to chime in and point out we also have this other curator related issue as well that we should resolve: https://issues.apache.org/jira/browse/AURORA-1944
>     
>     We have not run into this issuse on our cluster where we reverted to the commons code.
> 
> Stephan Erb wrote:
>     With these two patches merged I belive Curator should be working fine now:
>     
>     * Only failover on ZK session timeout (rather than on connection loss) https://reviews.apache.org/r/54288/
>     * Allow transitions from CONSTRUCTED to STOPPED: https://reviews.apache.org/r/62621
>     
>     Is there anything else I am missing that might need to be fixed?
> 
> Stephan Erb wrote:
>     How do we want to proceed here? David do you see a chance to re-test with current `master`? Or do you still expect it to be broken?
> 
> Bill Farner wrote:
>     Thanks for the ping.  This is blocked on me - i need to upgrade the curator rev to 4.0+.  The 4.0 line is [compatible](https://curator.apache.org/zk-compatibility.html) with our current ZK client version, and includes the error handling [control](https://issues.apache.org/jira/browse/CURATOR-248) we need.  Thanks for John Sirois for doing the wrangling on those details!
>     
>     I plan to update this review with the above changes, but it will likely take me another week to get around to it.
> 
> Bill Farner wrote:
>     After looking more closely, i believe Stephan is right.  The two patches above address the issues David and Renan observed.
>     
>     I was able to reproduce [AURORA-1840](https://issues.apache.org/jira/browse/AURORA-1840) in vagrant by building the scheduler from the commit prior to [1e2a9e1](https://github.com/apache/aurora/commit/1e2a9e1).  The scheduler would predictably fail over after taking down the ZK server for ~15 seconds.  On master, i can repeat the procedure and take ZK down for 5 minutes.  The scheduler happily picks back up once ZK is reachable again.
>     
>     The curator 4.0 and error-handling support is nice-to-have at this point, Zameer's patch above seems to have worked around it just fine.  We're just able to remove some code once we are at 4.0.
>     
>     David - i'd like you to reconsider your -1 based on this information, or provide a procedure to reproduce other problems you have observed.

That sounds good. I'll withdraw my -1 based on your testing.


- David


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


On Oct. 8, 2017, 5:29 p.m., Bill Farner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62652/
> -----------------------------------------------------------
> 
> (Updated Oct. 8, 2017, 5:29 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and John Sirois.
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> This is the result removing the deprecated `-zk_use_curator` arg in `FlaggedZooKeeperConfig.java`, and following the trail of unused code.  Thanks for the clean separation, John!
> 
> 
> Diffs
> -----
> 
>   RELEASE-NOTES.md c58e68080beb1740d92639601ef7cc29c63be37e 
>   commons/src/main/java/org/apache/aurora/common/net/pool/DynamicHostSet.java df469efac2d994517d9b931cb2c1582e09d0cab7 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/Candidate.java 75c1b1493eb178ea035a0e60f20df694626c7f50 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/CandidateImpl.java 98b5ee4690482683526f2e8ce4c965ed052b0232 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/Group.java 2720dd13a3cc5e39f7242664fbe98f00fb75eb08 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ServerSet.java aeea02d81faa3c8841edb9002498515dd0b52242 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ServerSetImpl.java ace4980d3e15dc830fea48b83f0688347c71d900 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ServerSets.java 01a54a5718388df7f02992388653d7b049422787 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/SingletonServiceImpl.java d9978a926e05e4303a168c7c51ee226a7dc94336 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ZooKeeperClient.java ce243fb44266b741ff3f14eb5438ff38cf46133f 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ZooKeeperUtils.java 2ada2646da7bdc3276803336cc638e23b9e61d26 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/testing/BaseZooKeeperClientTest.java ba09279be9d8bfde79b7610a184bb472ab0396b5 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/testing/ZooKeeperTestServer.java 29204cda66343c5b3c80d167eccd36abfb6ee641 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/CandidateImplTest.java 9c0cebe888d966fd64d17778c354136df0e13c7e 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/GroupTest.java 97a42d1bd6c60598d7936a2c065e163c1505ef3d 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/JsonCodecTest.java 2166123e9acd701e55e10b450a9eb89054c7afdd 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/ServerSetImplTest.java f0c0cb4491c7b5c34103864ee244a27db3972a94 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/ServerSetsTest.java 0e6719154ee5195111db51d323fa35c3bc56ec2b 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/SingletonServiceImplTest.java 5f6cdd83ca6c6338f8c7a0a3c93b960c036cb05e 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/ZooKeeperClientTest.java 5eee235162e938ecdfdb7d3c4d31d848a34a648b 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/ZooKeeperUtilsTest.java 9e482a6162abe5f9864d0ca0094cd97dc8036496 
>   src/main/java/org/apache/aurora/scheduler/discovery/CommonsServiceDiscoveryModule.java 339f63b94e996e13514e6a6dcd7bbeef3a0f09f8 
>   src/main/java/org/apache/aurora/scheduler/discovery/CommonsServiceGroupMonitor.java 9161455c3101fb0d83633883a67ce0ffe22fcdf8 
>   src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceDiscoveryModule.java ea167a89a95f0f35d53a34631ae44e0a911a6817 
>   src/main/java/org/apache/aurora/scheduler/discovery/FlaggedZooKeeperConfig.java e8aafe4a47e3f0e6312c8b93dbc32e1c25445dd3 
>   src/main/java/org/apache/aurora/scheduler/discovery/ServiceDiscoveryModule.java 917a5679699581fb345d51d2837f372411d4beda 
>   src/main/java/org/apache/aurora/scheduler/discovery/ZooKeeperConfig.java 4014a91299d863337c5f58a0495775c5f4aa6c09 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java a7c9c83eebbbea7ae8a6c807f98d3ce8bd050137 
>   src/test/java/org/apache/aurora/scheduler/discovery/AbstractDiscoveryModuleTest.java 0f2121ef1453b41286f9713d5ce89a26d1cb1b4f 
>   src/test/java/org/apache/aurora/scheduler/discovery/BaseCuratorDiscoveryTest.java 226b0686f91539decd7d421a28d431209bcec790 
>   src/test/java/org/apache/aurora/scheduler/discovery/CommonsDiscoveryModuleTest.java 7a4c4dd932fa0a69f22bcbb5cba7750bba52e16d 
>   src/test/java/org/apache/aurora/scheduler/discovery/CommonsServiceGroupMonitorTest.java 42a2224839f00e6938100fc315874c531cb37096 
>   src/test/java/org/apache/aurora/scheduler/discovery/CuratorDiscoveryModuleTest.java f1a02e4becc5865e72aaf96334c53cb9c395d09a 
>   src/test/java/org/apache/aurora/scheduler/discovery/ZooKeeperConfigTest.java a065505134c1ad30ba9f4d4cd829355b2acc9a13 
>   src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java d21a38ed66253eb7a2ceefaf6c0ec1b788036552 
> 
> 
> Diff: https://reviews.apache.org/r/62652/diff/2/
> 
> 
> Testing
> -------
> 
> `./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh`
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 62652: Remove legacy commons ZK code

Posted by Bill Farner <wf...@apache.org>.


> On Sept. 27, 2017, 8:22 p.m., David McLaughlin wrote:
> > -1. 
> > 
> > Please see here for details on the current status of curator in production: https://issues.apache.org/jira/browse/AURORA-1840
> 
> Bill Farner wrote:
>     Aha, there was no trace of this in the code, thanks for the pointer.  I'll take a look.
> 
> Renan DelValle wrote:
>     Just wanted to chime in and point out we also have this other curator related issue as well that we should resolve: https://issues.apache.org/jira/browse/AURORA-1944
>     
>     We have not run into this issuse on our cluster where we reverted to the commons code.
> 
> Stephan Erb wrote:
>     With these two patches merged I belive Curator should be working fine now:
>     
>     * Only failover on ZK session timeout (rather than on connection loss) https://reviews.apache.org/r/54288/
>     * Allow transitions from CONSTRUCTED to STOPPED: https://reviews.apache.org/r/62621
>     
>     Is there anything else I am missing that might need to be fixed?
> 
> Stephan Erb wrote:
>     How do we want to proceed here? David do you see a chance to re-test with current `master`? Or do you still expect it to be broken?
> 
> Bill Farner wrote:
>     Thanks for the ping.  This is blocked on me - i need to upgrade the curator rev to 4.0+.  The 4.0 line is [compatible](https://curator.apache.org/zk-compatibility.html) with our current ZK client version, and includes the error handling [control](https://issues.apache.org/jira/browse/CURATOR-248) we need.  Thanks for John Sirois for doing the wrangling on those details!
>     
>     I plan to update this review with the above changes, but it will likely take me another week to get around to it.

After looking more closely, i believe Stephan is right.  The two patches above address the issues David and Renan observed.

I was able to reproduce [AURORA-1840](https://issues.apache.org/jira/browse/AURORA-1840) in vagrant by building the scheduler from the commit prior to [1e2a9e1](https://github.com/apache/aurora/commit/1e2a9e1).  The scheduler would predictably fail over after taking down the ZK server for ~15 seconds.  On master, i can repeat the procedure and take ZK down for 5 minutes.  The scheduler happily picks back up once ZK is reachable again.

The curator 4.0 and error-handling support is nice-to-have at this point, Zameer's patch above seems to have worked around it just fine.  We're just able to remove some code once we are at 4.0.

David - i'd like you to reconsider your -1 based on this information, or provide a procedure to reproduce other problems you have observed.


- Bill


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


On Sept. 27, 2017, 8:14 p.m., Bill Farner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62652/
> -----------------------------------------------------------
> 
> (Updated Sept. 27, 2017, 8:14 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and John Sirois.
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> This is the result removing the deprecated `-zk_use_curator` arg in `FlaggedZooKeeperConfig.java`, and following the trail of unused code.  Thanks for the clean separation, John!
> 
> 
> Diffs
> -----
> 
>   RELEASE-NOTES.md fd2618fee8ef05091849e177bd99fc321548be90 
>   commons/src/main/java/org/apache/aurora/common/net/pool/DynamicHostSet.java df469efac2d994517d9b931cb2c1582e09d0cab7 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/Candidate.java 75c1b1493eb178ea035a0e60f20df694626c7f50 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/CandidateImpl.java 98b5ee4690482683526f2e8ce4c965ed052b0232 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/Group.java 2720dd13a3cc5e39f7242664fbe98f00fb75eb08 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ServerSet.java aeea02d81faa3c8841edb9002498515dd0b52242 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ServerSetImpl.java ace4980d3e15dc830fea48b83f0688347c71d900 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ServerSets.java 01a54a5718388df7f02992388653d7b049422787 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/SingletonServiceImpl.java d9978a926e05e4303a168c7c51ee226a7dc94336 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ZooKeeperClient.java ce243fb44266b741ff3f14eb5438ff38cf46133f 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ZooKeeperUtils.java 2ada2646da7bdc3276803336cc638e23b9e61d26 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/testing/BaseZooKeeperClientTest.java ba09279be9d8bfde79b7610a184bb472ab0396b5 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/testing/ZooKeeperTestServer.java 29204cda66343c5b3c80d167eccd36abfb6ee641 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/CandidateImplTest.java 9c0cebe888d966fd64d17778c354136df0e13c7e 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/GroupTest.java 97a42d1bd6c60598d7936a2c065e163c1505ef3d 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/JsonCodecTest.java 2166123e9acd701e55e10b450a9eb89054c7afdd 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/ServerSetImplTest.java f0c0cb4491c7b5c34103864ee244a27db3972a94 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/ServerSetsTest.java 0e6719154ee5195111db51d323fa35c3bc56ec2b 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/SingletonServiceImplTest.java 5f6cdd83ca6c6338f8c7a0a3c93b960c036cb05e 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/ZooKeeperClientTest.java 5eee235162e938ecdfdb7d3c4d31d848a34a648b 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/ZooKeeperUtilsTest.java 9e482a6162abe5f9864d0ca0094cd97dc8036496 
>   src/main/java/org/apache/aurora/scheduler/discovery/CommonsServiceDiscoveryModule.java 339f63b94e996e13514e6a6dcd7bbeef3a0f09f8 
>   src/main/java/org/apache/aurora/scheduler/discovery/CommonsServiceGroupMonitor.java 9161455c3101fb0d83633883a67ce0ffe22fcdf8 
>   src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceDiscoveryModule.java ea167a89a95f0f35d53a34631ae44e0a911a6817 
>   src/main/java/org/apache/aurora/scheduler/discovery/FlaggedZooKeeperConfig.java e8aafe4a47e3f0e6312c8b93dbc32e1c25445dd3 
>   src/main/java/org/apache/aurora/scheduler/discovery/ServiceDiscoveryModule.java 917a5679699581fb345d51d2837f372411d4beda 
>   src/main/java/org/apache/aurora/scheduler/discovery/ZooKeeperConfig.java 4014a91299d863337c5f58a0495775c5f4aa6c09 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java a7c9c83eebbbea7ae8a6c807f98d3ce8bd050137 
>   src/test/java/org/apache/aurora/scheduler/discovery/AbstractDiscoveryModuleTest.java 0f2121ef1453b41286f9713d5ce89a26d1cb1b4f 
>   src/test/java/org/apache/aurora/scheduler/discovery/BaseCuratorDiscoveryTest.java 226b0686f91539decd7d421a28d431209bcec790 
>   src/test/java/org/apache/aurora/scheduler/discovery/CommonsDiscoveryModuleTest.java 7a4c4dd932fa0a69f22bcbb5cba7750bba52e16d 
>   src/test/java/org/apache/aurora/scheduler/discovery/CommonsServiceGroupMonitorTest.java 42a2224839f00e6938100fc315874c531cb37096 
>   src/test/java/org/apache/aurora/scheduler/discovery/CuratorDiscoveryModuleTest.java f1a02e4becc5865e72aaf96334c53cb9c395d09a 
>   src/test/java/org/apache/aurora/scheduler/discovery/ZooKeeperConfigTest.java a065505134c1ad30ba9f4d4cd829355b2acc9a13 
>   src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java d21a38ed66253eb7a2ceefaf6c0ec1b788036552 
> 
> 
> Diff: https://reviews.apache.org/r/62652/diff/1/
> 
> 
> Testing
> -------
> 
> `./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh`
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 62652: Remove legacy commons ZK code

Posted by Stephan Erb <se...@apache.org>.


> On Sept. 28, 2017, 5:22 a.m., David McLaughlin wrote:
> > -1. 
> > 
> > Please see here for details on the current status of curator in production: https://issues.apache.org/jira/browse/AURORA-1840
> 
> Bill Farner wrote:
>     Aha, there was no trace of this in the code, thanks for the pointer.  I'll take a look.
> 
> Renan DelValle wrote:
>     Just wanted to chime in and point out we also have this other curator related issue as well that we should resolve: https://issues.apache.org/jira/browse/AURORA-1944
>     
>     We have not run into this issuse on our cluster where we reverted to the commons code.
> 
> Stephan Erb wrote:
>     With these two patches merged I belive Curator should be working fine now:
>     
>     * Only failover on ZK session timeout (rather than on connection loss) https://reviews.apache.org/r/54288/
>     * Allow transitions from CONSTRUCTED to STOPPED: https://reviews.apache.org/r/62621
>     
>     Is there anything else I am missing that might need to be fixed?

How do we want to proceed here? David do you see a chance to re-test with current `master`? Or do you still expect it to be broken?


- Stephan


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


On Sept. 28, 2017, 5:14 a.m., Bill Farner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62652/
> -----------------------------------------------------------
> 
> (Updated Sept. 28, 2017, 5:14 a.m.)
> 
> 
> Review request for Aurora, David McLaughlin and John Sirois.
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> This is the result removing the deprecated `-zk_use_curator` arg in `FlaggedZooKeeperConfig.java`, and following the trail of unused code.  Thanks for the clean separation, John!
> 
> 
> Diffs
> -----
> 
>   RELEASE-NOTES.md fd2618fee8ef05091849e177bd99fc321548be90 
>   commons/src/main/java/org/apache/aurora/common/net/pool/DynamicHostSet.java df469efac2d994517d9b931cb2c1582e09d0cab7 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/Candidate.java 75c1b1493eb178ea035a0e60f20df694626c7f50 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/CandidateImpl.java 98b5ee4690482683526f2e8ce4c965ed052b0232 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/Group.java 2720dd13a3cc5e39f7242664fbe98f00fb75eb08 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ServerSet.java aeea02d81faa3c8841edb9002498515dd0b52242 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ServerSetImpl.java ace4980d3e15dc830fea48b83f0688347c71d900 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ServerSets.java 01a54a5718388df7f02992388653d7b049422787 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/SingletonServiceImpl.java d9978a926e05e4303a168c7c51ee226a7dc94336 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ZooKeeperClient.java ce243fb44266b741ff3f14eb5438ff38cf46133f 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ZooKeeperUtils.java 2ada2646da7bdc3276803336cc638e23b9e61d26 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/testing/BaseZooKeeperClientTest.java ba09279be9d8bfde79b7610a184bb472ab0396b5 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/testing/ZooKeeperTestServer.java 29204cda66343c5b3c80d167eccd36abfb6ee641 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/CandidateImplTest.java 9c0cebe888d966fd64d17778c354136df0e13c7e 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/GroupTest.java 97a42d1bd6c60598d7936a2c065e163c1505ef3d 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/JsonCodecTest.java 2166123e9acd701e55e10b450a9eb89054c7afdd 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/ServerSetImplTest.java f0c0cb4491c7b5c34103864ee244a27db3972a94 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/ServerSetsTest.java 0e6719154ee5195111db51d323fa35c3bc56ec2b 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/SingletonServiceImplTest.java 5f6cdd83ca6c6338f8c7a0a3c93b960c036cb05e 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/ZooKeeperClientTest.java 5eee235162e938ecdfdb7d3c4d31d848a34a648b 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/ZooKeeperUtilsTest.java 9e482a6162abe5f9864d0ca0094cd97dc8036496 
>   src/main/java/org/apache/aurora/scheduler/discovery/CommonsServiceDiscoveryModule.java 339f63b94e996e13514e6a6dcd7bbeef3a0f09f8 
>   src/main/java/org/apache/aurora/scheduler/discovery/CommonsServiceGroupMonitor.java 9161455c3101fb0d83633883a67ce0ffe22fcdf8 
>   src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceDiscoveryModule.java ea167a89a95f0f35d53a34631ae44e0a911a6817 
>   src/main/java/org/apache/aurora/scheduler/discovery/FlaggedZooKeeperConfig.java e8aafe4a47e3f0e6312c8b93dbc32e1c25445dd3 
>   src/main/java/org/apache/aurora/scheduler/discovery/ServiceDiscoveryModule.java 917a5679699581fb345d51d2837f372411d4beda 
>   src/main/java/org/apache/aurora/scheduler/discovery/ZooKeeperConfig.java 4014a91299d863337c5f58a0495775c5f4aa6c09 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java a7c9c83eebbbea7ae8a6c807f98d3ce8bd050137 
>   src/test/java/org/apache/aurora/scheduler/discovery/AbstractDiscoveryModuleTest.java 0f2121ef1453b41286f9713d5ce89a26d1cb1b4f 
>   src/test/java/org/apache/aurora/scheduler/discovery/BaseCuratorDiscoveryTest.java 226b0686f91539decd7d421a28d431209bcec790 
>   src/test/java/org/apache/aurora/scheduler/discovery/CommonsDiscoveryModuleTest.java 7a4c4dd932fa0a69f22bcbb5cba7750bba52e16d 
>   src/test/java/org/apache/aurora/scheduler/discovery/CommonsServiceGroupMonitorTest.java 42a2224839f00e6938100fc315874c531cb37096 
>   src/test/java/org/apache/aurora/scheduler/discovery/CuratorDiscoveryModuleTest.java f1a02e4becc5865e72aaf96334c53cb9c395d09a 
>   src/test/java/org/apache/aurora/scheduler/discovery/ZooKeeperConfigTest.java a065505134c1ad30ba9f4d4cd829355b2acc9a13 
>   src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java d21a38ed66253eb7a2ceefaf6c0ec1b788036552 
> 
> 
> Diff: https://reviews.apache.org/r/62652/diff/1/
> 
> 
> Testing
> -------
> 
> `./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh`
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 62652: Remove legacy commons ZK code

Posted by Renan DelValle <re...@gmail.com>.


> On Sept. 27, 2017, 8:22 p.m., David McLaughlin wrote:
> > -1. 
> > 
> > Please see here for details on the current status of curator in production: https://issues.apache.org/jira/browse/AURORA-1840
> 
> Bill Farner wrote:
>     Aha, there was no trace of this in the code, thanks for the pointer.  I'll take a look.

Just wanted to chime in and point out we also have this other curator related issue as well that we should resolve: https://issues.apache.org/jira/browse/AURORA-1944

We have not run into this issuse on our cluster where we reverted to the commons code.


- Renan


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


On Sept. 27, 2017, 8:14 p.m., Bill Farner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62652/
> -----------------------------------------------------------
> 
> (Updated Sept. 27, 2017, 8:14 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and John Sirois.
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> This is the result removing the deprecated `-zk_use_curator` arg in `FlaggedZooKeeperConfig.java`, and following the trail of unused code.  Thanks for the clean separation, John!
> 
> 
> Diffs
> -----
> 
>   RELEASE-NOTES.md fd2618fee8ef05091849e177bd99fc321548be90 
>   commons/src/main/java/org/apache/aurora/common/net/pool/DynamicHostSet.java df469efac2d994517d9b931cb2c1582e09d0cab7 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/Candidate.java 75c1b1493eb178ea035a0e60f20df694626c7f50 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/CandidateImpl.java 98b5ee4690482683526f2e8ce4c965ed052b0232 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/Group.java 2720dd13a3cc5e39f7242664fbe98f00fb75eb08 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ServerSet.java aeea02d81faa3c8841edb9002498515dd0b52242 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ServerSetImpl.java ace4980d3e15dc830fea48b83f0688347c71d900 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ServerSets.java 01a54a5718388df7f02992388653d7b049422787 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/SingletonServiceImpl.java d9978a926e05e4303a168c7c51ee226a7dc94336 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ZooKeeperClient.java ce243fb44266b741ff3f14eb5438ff38cf46133f 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ZooKeeperUtils.java 2ada2646da7bdc3276803336cc638e23b9e61d26 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/testing/BaseZooKeeperClientTest.java ba09279be9d8bfde79b7610a184bb472ab0396b5 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/testing/ZooKeeperTestServer.java 29204cda66343c5b3c80d167eccd36abfb6ee641 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/CandidateImplTest.java 9c0cebe888d966fd64d17778c354136df0e13c7e 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/GroupTest.java 97a42d1bd6c60598d7936a2c065e163c1505ef3d 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/JsonCodecTest.java 2166123e9acd701e55e10b450a9eb89054c7afdd 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/ServerSetImplTest.java f0c0cb4491c7b5c34103864ee244a27db3972a94 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/ServerSetsTest.java 0e6719154ee5195111db51d323fa35c3bc56ec2b 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/SingletonServiceImplTest.java 5f6cdd83ca6c6338f8c7a0a3c93b960c036cb05e 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/ZooKeeperClientTest.java 5eee235162e938ecdfdb7d3c4d31d848a34a648b 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/ZooKeeperUtilsTest.java 9e482a6162abe5f9864d0ca0094cd97dc8036496 
>   src/main/java/org/apache/aurora/scheduler/discovery/CommonsServiceDiscoveryModule.java 339f63b94e996e13514e6a6dcd7bbeef3a0f09f8 
>   src/main/java/org/apache/aurora/scheduler/discovery/CommonsServiceGroupMonitor.java 9161455c3101fb0d83633883a67ce0ffe22fcdf8 
>   src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceDiscoveryModule.java ea167a89a95f0f35d53a34631ae44e0a911a6817 
>   src/main/java/org/apache/aurora/scheduler/discovery/FlaggedZooKeeperConfig.java e8aafe4a47e3f0e6312c8b93dbc32e1c25445dd3 
>   src/main/java/org/apache/aurora/scheduler/discovery/ServiceDiscoveryModule.java 917a5679699581fb345d51d2837f372411d4beda 
>   src/main/java/org/apache/aurora/scheduler/discovery/ZooKeeperConfig.java 4014a91299d863337c5f58a0495775c5f4aa6c09 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java a7c9c83eebbbea7ae8a6c807f98d3ce8bd050137 
>   src/test/java/org/apache/aurora/scheduler/discovery/AbstractDiscoveryModuleTest.java 0f2121ef1453b41286f9713d5ce89a26d1cb1b4f 
>   src/test/java/org/apache/aurora/scheduler/discovery/BaseCuratorDiscoveryTest.java 226b0686f91539decd7d421a28d431209bcec790 
>   src/test/java/org/apache/aurora/scheduler/discovery/CommonsDiscoveryModuleTest.java 7a4c4dd932fa0a69f22bcbb5cba7750bba52e16d 
>   src/test/java/org/apache/aurora/scheduler/discovery/CommonsServiceGroupMonitorTest.java 42a2224839f00e6938100fc315874c531cb37096 
>   src/test/java/org/apache/aurora/scheduler/discovery/CuratorDiscoveryModuleTest.java f1a02e4becc5865e72aaf96334c53cb9c395d09a 
>   src/test/java/org/apache/aurora/scheduler/discovery/ZooKeeperConfigTest.java a065505134c1ad30ba9f4d4cd829355b2acc9a13 
>   src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java d21a38ed66253eb7a2ceefaf6c0ec1b788036552 
> 
> 
> Diff: https://reviews.apache.org/r/62652/diff/1/
> 
> 
> Testing
> -------
> 
> `./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh`
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 62652: Remove legacy commons ZK code

Posted by Bill Farner <wf...@apache.org>.


> On Sept. 27, 2017, 8:22 p.m., David McLaughlin wrote:
> > -1. 
> > 
> > Please see here for details on the current status of curator in production: https://issues.apache.org/jira/browse/AURORA-1840
> 
> Bill Farner wrote:
>     Aha, there was no trace of this in the code, thanks for the pointer.  I'll take a look.
> 
> Renan DelValle wrote:
>     Just wanted to chime in and point out we also have this other curator related issue as well that we should resolve: https://issues.apache.org/jira/browse/AURORA-1944
>     
>     We have not run into this issuse on our cluster where we reverted to the commons code.
> 
> Stephan Erb wrote:
>     With these two patches merged I belive Curator should be working fine now:
>     
>     * Only failover on ZK session timeout (rather than on connection loss) https://reviews.apache.org/r/54288/
>     * Allow transitions from CONSTRUCTED to STOPPED: https://reviews.apache.org/r/62621
>     
>     Is there anything else I am missing that might need to be fixed?
> 
> Stephan Erb wrote:
>     How do we want to proceed here? David do you see a chance to re-test with current `master`? Or do you still expect it to be broken?

Thanks for the ping.  This is blocked on me - i need to upgrade the curator rev to 4.0+.  The 4.0 line is [compatible](https://curator.apache.org/zk-compatibility.html) with our current ZK client version, and includes the error handling [control](https://issues.apache.org/jira/browse/CURATOR-248) we need.  Thanks for John Sirois for doing the wrangling on those details!

I plan to update this review with the above changes, but it will likely take me another week to get around to it.


- Bill


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


On Sept. 27, 2017, 8:14 p.m., Bill Farner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62652/
> -----------------------------------------------------------
> 
> (Updated Sept. 27, 2017, 8:14 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and John Sirois.
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> This is the result removing the deprecated `-zk_use_curator` arg in `FlaggedZooKeeperConfig.java`, and following the trail of unused code.  Thanks for the clean separation, John!
> 
> 
> Diffs
> -----
> 
>   RELEASE-NOTES.md fd2618fee8ef05091849e177bd99fc321548be90 
>   commons/src/main/java/org/apache/aurora/common/net/pool/DynamicHostSet.java df469efac2d994517d9b931cb2c1582e09d0cab7 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/Candidate.java 75c1b1493eb178ea035a0e60f20df694626c7f50 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/CandidateImpl.java 98b5ee4690482683526f2e8ce4c965ed052b0232 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/Group.java 2720dd13a3cc5e39f7242664fbe98f00fb75eb08 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ServerSet.java aeea02d81faa3c8841edb9002498515dd0b52242 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ServerSetImpl.java ace4980d3e15dc830fea48b83f0688347c71d900 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ServerSets.java 01a54a5718388df7f02992388653d7b049422787 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/SingletonServiceImpl.java d9978a926e05e4303a168c7c51ee226a7dc94336 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ZooKeeperClient.java ce243fb44266b741ff3f14eb5438ff38cf46133f 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ZooKeeperUtils.java 2ada2646da7bdc3276803336cc638e23b9e61d26 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/testing/BaseZooKeeperClientTest.java ba09279be9d8bfde79b7610a184bb472ab0396b5 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/testing/ZooKeeperTestServer.java 29204cda66343c5b3c80d167eccd36abfb6ee641 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/CandidateImplTest.java 9c0cebe888d966fd64d17778c354136df0e13c7e 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/GroupTest.java 97a42d1bd6c60598d7936a2c065e163c1505ef3d 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/JsonCodecTest.java 2166123e9acd701e55e10b450a9eb89054c7afdd 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/ServerSetImplTest.java f0c0cb4491c7b5c34103864ee244a27db3972a94 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/ServerSetsTest.java 0e6719154ee5195111db51d323fa35c3bc56ec2b 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/SingletonServiceImplTest.java 5f6cdd83ca6c6338f8c7a0a3c93b960c036cb05e 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/ZooKeeperClientTest.java 5eee235162e938ecdfdb7d3c4d31d848a34a648b 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/ZooKeeperUtilsTest.java 9e482a6162abe5f9864d0ca0094cd97dc8036496 
>   src/main/java/org/apache/aurora/scheduler/discovery/CommonsServiceDiscoveryModule.java 339f63b94e996e13514e6a6dcd7bbeef3a0f09f8 
>   src/main/java/org/apache/aurora/scheduler/discovery/CommonsServiceGroupMonitor.java 9161455c3101fb0d83633883a67ce0ffe22fcdf8 
>   src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceDiscoveryModule.java ea167a89a95f0f35d53a34631ae44e0a911a6817 
>   src/main/java/org/apache/aurora/scheduler/discovery/FlaggedZooKeeperConfig.java e8aafe4a47e3f0e6312c8b93dbc32e1c25445dd3 
>   src/main/java/org/apache/aurora/scheduler/discovery/ServiceDiscoveryModule.java 917a5679699581fb345d51d2837f372411d4beda 
>   src/main/java/org/apache/aurora/scheduler/discovery/ZooKeeperConfig.java 4014a91299d863337c5f58a0495775c5f4aa6c09 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java a7c9c83eebbbea7ae8a6c807f98d3ce8bd050137 
>   src/test/java/org/apache/aurora/scheduler/discovery/AbstractDiscoveryModuleTest.java 0f2121ef1453b41286f9713d5ce89a26d1cb1b4f 
>   src/test/java/org/apache/aurora/scheduler/discovery/BaseCuratorDiscoveryTest.java 226b0686f91539decd7d421a28d431209bcec790 
>   src/test/java/org/apache/aurora/scheduler/discovery/CommonsDiscoveryModuleTest.java 7a4c4dd932fa0a69f22bcbb5cba7750bba52e16d 
>   src/test/java/org/apache/aurora/scheduler/discovery/CommonsServiceGroupMonitorTest.java 42a2224839f00e6938100fc315874c531cb37096 
>   src/test/java/org/apache/aurora/scheduler/discovery/CuratorDiscoveryModuleTest.java f1a02e4becc5865e72aaf96334c53cb9c395d09a 
>   src/test/java/org/apache/aurora/scheduler/discovery/ZooKeeperConfigTest.java a065505134c1ad30ba9f4d4cd829355b2acc9a13 
>   src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java d21a38ed66253eb7a2ceefaf6c0ec1b788036552 
> 
> 
> Diff: https://reviews.apache.org/r/62652/diff/1/
> 
> 
> Testing
> -------
> 
> `./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh`
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 62652: Remove legacy commons ZK code

Posted by Stephan Erb <se...@apache.org>.


> On Sept. 28, 2017, 5:22 a.m., David McLaughlin wrote:
> > -1. 
> > 
> > Please see here for details on the current status of curator in production: https://issues.apache.org/jira/browse/AURORA-1840
> 
> Bill Farner wrote:
>     Aha, there was no trace of this in the code, thanks for the pointer.  I'll take a look.
> 
> Renan DelValle wrote:
>     Just wanted to chime in and point out we also have this other curator related issue as well that we should resolve: https://issues.apache.org/jira/browse/AURORA-1944
>     
>     We have not run into this issuse on our cluster where we reverted to the commons code.

With these two patches merged I belive Curator should be working fine now:

* Only failover on ZK session timeout (rather than on connection loss) https://reviews.apache.org/r/54288/
* Allow transitions from CONSTRUCTED to STOPPED: https://reviews.apache.org/r/62621

Is there anything else I am missing that might need to be fixed?


- Stephan


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


On Sept. 28, 2017, 5:14 a.m., Bill Farner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62652/
> -----------------------------------------------------------
> 
> (Updated Sept. 28, 2017, 5:14 a.m.)
> 
> 
> Review request for Aurora, David McLaughlin and John Sirois.
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> This is the result removing the deprecated `-zk_use_curator` arg in `FlaggedZooKeeperConfig.java`, and following the trail of unused code.  Thanks for the clean separation, John!
> 
> 
> Diffs
> -----
> 
>   RELEASE-NOTES.md fd2618fee8ef05091849e177bd99fc321548be90 
>   commons/src/main/java/org/apache/aurora/common/net/pool/DynamicHostSet.java df469efac2d994517d9b931cb2c1582e09d0cab7 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/Candidate.java 75c1b1493eb178ea035a0e60f20df694626c7f50 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/CandidateImpl.java 98b5ee4690482683526f2e8ce4c965ed052b0232 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/Group.java 2720dd13a3cc5e39f7242664fbe98f00fb75eb08 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ServerSet.java aeea02d81faa3c8841edb9002498515dd0b52242 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ServerSetImpl.java ace4980d3e15dc830fea48b83f0688347c71d900 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ServerSets.java 01a54a5718388df7f02992388653d7b049422787 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/SingletonServiceImpl.java d9978a926e05e4303a168c7c51ee226a7dc94336 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ZooKeeperClient.java ce243fb44266b741ff3f14eb5438ff38cf46133f 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ZooKeeperUtils.java 2ada2646da7bdc3276803336cc638e23b9e61d26 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/testing/BaseZooKeeperClientTest.java ba09279be9d8bfde79b7610a184bb472ab0396b5 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/testing/ZooKeeperTestServer.java 29204cda66343c5b3c80d167eccd36abfb6ee641 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/CandidateImplTest.java 9c0cebe888d966fd64d17778c354136df0e13c7e 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/GroupTest.java 97a42d1bd6c60598d7936a2c065e163c1505ef3d 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/JsonCodecTest.java 2166123e9acd701e55e10b450a9eb89054c7afdd 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/ServerSetImplTest.java f0c0cb4491c7b5c34103864ee244a27db3972a94 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/ServerSetsTest.java 0e6719154ee5195111db51d323fa35c3bc56ec2b 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/SingletonServiceImplTest.java 5f6cdd83ca6c6338f8c7a0a3c93b960c036cb05e 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/ZooKeeperClientTest.java 5eee235162e938ecdfdb7d3c4d31d848a34a648b 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/ZooKeeperUtilsTest.java 9e482a6162abe5f9864d0ca0094cd97dc8036496 
>   src/main/java/org/apache/aurora/scheduler/discovery/CommonsServiceDiscoveryModule.java 339f63b94e996e13514e6a6dcd7bbeef3a0f09f8 
>   src/main/java/org/apache/aurora/scheduler/discovery/CommonsServiceGroupMonitor.java 9161455c3101fb0d83633883a67ce0ffe22fcdf8 
>   src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceDiscoveryModule.java ea167a89a95f0f35d53a34631ae44e0a911a6817 
>   src/main/java/org/apache/aurora/scheduler/discovery/FlaggedZooKeeperConfig.java e8aafe4a47e3f0e6312c8b93dbc32e1c25445dd3 
>   src/main/java/org/apache/aurora/scheduler/discovery/ServiceDiscoveryModule.java 917a5679699581fb345d51d2837f372411d4beda 
>   src/main/java/org/apache/aurora/scheduler/discovery/ZooKeeperConfig.java 4014a91299d863337c5f58a0495775c5f4aa6c09 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java a7c9c83eebbbea7ae8a6c807f98d3ce8bd050137 
>   src/test/java/org/apache/aurora/scheduler/discovery/AbstractDiscoveryModuleTest.java 0f2121ef1453b41286f9713d5ce89a26d1cb1b4f 
>   src/test/java/org/apache/aurora/scheduler/discovery/BaseCuratorDiscoveryTest.java 226b0686f91539decd7d421a28d431209bcec790 
>   src/test/java/org/apache/aurora/scheduler/discovery/CommonsDiscoveryModuleTest.java 7a4c4dd932fa0a69f22bcbb5cba7750bba52e16d 
>   src/test/java/org/apache/aurora/scheduler/discovery/CommonsServiceGroupMonitorTest.java 42a2224839f00e6938100fc315874c531cb37096 
>   src/test/java/org/apache/aurora/scheduler/discovery/CuratorDiscoveryModuleTest.java f1a02e4becc5865e72aaf96334c53cb9c395d09a 
>   src/test/java/org/apache/aurora/scheduler/discovery/ZooKeeperConfigTest.java a065505134c1ad30ba9f4d4cd829355b2acc9a13 
>   src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java d21a38ed66253eb7a2ceefaf6c0ec1b788036552 
> 
> 
> Diff: https://reviews.apache.org/r/62652/diff/1/
> 
> 
> Testing
> -------
> 
> `./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh`
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 62652: Remove legacy commons ZK code

Posted by David McLaughlin <da...@dmclaughlin.com>.

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



-1. 

Please see here for details on the current status of curator in production: https://issues.apache.org/jira/browse/AURORA-1840

- David McLaughlin


On Sept. 28, 2017, 3:14 a.m., Bill Farner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62652/
> -----------------------------------------------------------
> 
> (Updated Sept. 28, 2017, 3:14 a.m.)
> 
> 
> Review request for Aurora, David McLaughlin and John Sirois.
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> This is the result removing the deprecated `-zk_use_curator` arg in `FlaggedZooKeeperConfig.java`, and following the trail of unused code.  Thanks for the clean separation, John!
> 
> 
> Diffs
> -----
> 
>   RELEASE-NOTES.md fd2618fee8ef05091849e177bd99fc321548be90 
>   commons/src/main/java/org/apache/aurora/common/net/pool/DynamicHostSet.java df469efac2d994517d9b931cb2c1582e09d0cab7 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/Candidate.java 75c1b1493eb178ea035a0e60f20df694626c7f50 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/CandidateImpl.java 98b5ee4690482683526f2e8ce4c965ed052b0232 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/Group.java 2720dd13a3cc5e39f7242664fbe98f00fb75eb08 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ServerSet.java aeea02d81faa3c8841edb9002498515dd0b52242 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ServerSetImpl.java ace4980d3e15dc830fea48b83f0688347c71d900 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ServerSets.java 01a54a5718388df7f02992388653d7b049422787 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/SingletonServiceImpl.java d9978a926e05e4303a168c7c51ee226a7dc94336 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ZooKeeperClient.java ce243fb44266b741ff3f14eb5438ff38cf46133f 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/ZooKeeperUtils.java 2ada2646da7bdc3276803336cc638e23b9e61d26 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/testing/BaseZooKeeperClientTest.java ba09279be9d8bfde79b7610a184bb472ab0396b5 
>   commons/src/main/java/org/apache/aurora/common/zookeeper/testing/ZooKeeperTestServer.java 29204cda66343c5b3c80d167eccd36abfb6ee641 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/CandidateImplTest.java 9c0cebe888d966fd64d17778c354136df0e13c7e 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/GroupTest.java 97a42d1bd6c60598d7936a2c065e163c1505ef3d 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/JsonCodecTest.java 2166123e9acd701e55e10b450a9eb89054c7afdd 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/ServerSetImplTest.java f0c0cb4491c7b5c34103864ee244a27db3972a94 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/ServerSetsTest.java 0e6719154ee5195111db51d323fa35c3bc56ec2b 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/SingletonServiceImplTest.java 5f6cdd83ca6c6338f8c7a0a3c93b960c036cb05e 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/ZooKeeperClientTest.java 5eee235162e938ecdfdb7d3c4d31d848a34a648b 
>   commons/src/test/java/org/apache/aurora/common/zookeeper/ZooKeeperUtilsTest.java 9e482a6162abe5f9864d0ca0094cd97dc8036496 
>   src/main/java/org/apache/aurora/scheduler/discovery/CommonsServiceDiscoveryModule.java 339f63b94e996e13514e6a6dcd7bbeef3a0f09f8 
>   src/main/java/org/apache/aurora/scheduler/discovery/CommonsServiceGroupMonitor.java 9161455c3101fb0d83633883a67ce0ffe22fcdf8 
>   src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceDiscoveryModule.java ea167a89a95f0f35d53a34631ae44e0a911a6817 
>   src/main/java/org/apache/aurora/scheduler/discovery/FlaggedZooKeeperConfig.java e8aafe4a47e3f0e6312c8b93dbc32e1c25445dd3 
>   src/main/java/org/apache/aurora/scheduler/discovery/ServiceDiscoveryModule.java 917a5679699581fb345d51d2837f372411d4beda 
>   src/main/java/org/apache/aurora/scheduler/discovery/ZooKeeperConfig.java 4014a91299d863337c5f58a0495775c5f4aa6c09 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java a7c9c83eebbbea7ae8a6c807f98d3ce8bd050137 
>   src/test/java/org/apache/aurora/scheduler/discovery/AbstractDiscoveryModuleTest.java 0f2121ef1453b41286f9713d5ce89a26d1cb1b4f 
>   src/test/java/org/apache/aurora/scheduler/discovery/BaseCuratorDiscoveryTest.java 226b0686f91539decd7d421a28d431209bcec790 
>   src/test/java/org/apache/aurora/scheduler/discovery/CommonsDiscoveryModuleTest.java 7a4c4dd932fa0a69f22bcbb5cba7750bba52e16d 
>   src/test/java/org/apache/aurora/scheduler/discovery/CommonsServiceGroupMonitorTest.java 42a2224839f00e6938100fc315874c531cb37096 
>   src/test/java/org/apache/aurora/scheduler/discovery/CuratorDiscoveryModuleTest.java f1a02e4becc5865e72aaf96334c53cb9c395d09a 
>   src/test/java/org/apache/aurora/scheduler/discovery/ZooKeeperConfigTest.java a065505134c1ad30ba9f4d4cd829355b2acc9a13 
>   src/test/java/org/apache/aurora/scheduler/http/LeaderHealthTest.java d21a38ed66253eb7a2ceefaf6c0ec1b788036552 
> 
> 
> Diff: https://reviews.apache.org/r/62652/diff/1/
> 
> 
> Testing
> -------
> 
> `./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh`
> 
> 
> Thanks,
> 
> Bill Farner
> 
>