You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by Jinmei Liao <ji...@pivotal.io> on 2016/12/21 18:16:45 UTC

Review Request 54943: GEODE-2197: refactor cluster config

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

Review request for geode, Bruce Schuchardt, Jared Stewart, John Blum, Kevin Duling, and Kirk Lund.


Repository: geode


Description
-------

* not to save the xml, properties in the file system, only in the internal region.
* the cc region's change listener is to download the jar from other locators

GEODE-2197: refactor cluster config and fix the test failures


Diffs
-----

  geode-core/src/main/java/org/apache/geode/distributed/internal/SharedConfiguration.java 3119823d6b8886cf893fd38ed58ca90b0c28eeb5 
  geode-core/src/main/java/org/apache/geode/internal/cache/ClusterConfigurationLoader.java 5e48d7ca0d9333b499fc5a9a9b0d4d67eff34856 
  geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DeployCommands.java a5302eac2389c88d4f2af7b6b0681ae149175287 
  geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportImportSharedConfigurationCommands.java 8d4677357450b30eb460344571174b044251c5d6 
  geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ExportSharedConfigurationFunction.java 821e04b2c590bddd28b0727e504c6460d77cf709 
  geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ImportSharedConfigurationArtifactsFunction.java 2ddeda69177b2aea41ee1d1318d8fdb19324a20b 
  geode-core/src/main/java/org/apache/geode/management/internal/configuration/SharedConfigurationWriter.java 3af55874282d380e340874a24860fafad7ace126 
  geode-core/src/main/java/org/apache/geode/management/internal/configuration/callbacks/ConfigurationChangeListener.java c08ba924b652c05436f921c19d65392fd7f6859b 
  geode-core/src/main/java/org/apache/geode/management/internal/configuration/domain/Configuration.java 4adf2110c4a270127adda1a5090d537a5e2de8c1 
  geode-core/src/main/java/org/apache/geode/management/internal/configuration/functions/AddJarFunction.java 605233fe69f68c04459dcd4b83cb71a2dc1b20c0 
  geode-core/src/main/java/org/apache/geode/management/internal/configuration/functions/GetAllJarsFunction.java 0e50524bee26aaf78c8b11977b5d884b869548da 
  geode-core/src/test/java/org/apache/geode/management/ConnectToLocatorSSLDUnitTest.java 2777b152134c66d9dcc8e047d9d78c1253cf878a 
  geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfig.java f406d51ca467cd21321a0dfa19cffa832770171d 
  geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigBaseTest.java 63c88dd1aa521a6781a8e07567df661cd42fb6f7 
  geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigImportDUnitTest.java c135f3d9b3370b4a756a2c7688530c63a4e06d77 
  geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigStartMemberDUnitTest.java cfcbeed86ade004eb305d9a01d6fb128132a21ac 
  geode-core/src/test/java/org/apache/geode/management/internal/configuration/ConfigGroup.java 971f66751505b57a98d07b8680873a249d92a30d 
  geode-core/src/test/java/org/apache/geode/management/internal/configuration/SharedConfigurationDUnitTest.java 675f5fe25c560fdcccfeb493c759320d12b64caf 
  geode-core/src/test/resources/org/apache/geode/codeAnalysis/sanctionedSerializables.txt d6b770f6c9b18e8c090045a66864a027f0ad8062 

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


Testing
-------


Thanks,

Jinmei Liao


Re: Review Request 54943: GEODE-2197: refactor cluster config

Posted by Kirk Lund <ki...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54943/#review160103
-----------------------------------------------------------


Ship it!




Ship It!

- Kirk Lund


On Dec. 22, 2016, 6:48 a.m., Jinmei Liao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54943/
> -----------------------------------------------------------
> 
> (Updated Dec. 22, 2016, 6:48 a.m.)
> 
> 
> Review request for geode, Bruce Schuchardt, Jared Stewart, John Blum, Kevin Duling, and Kirk Lund.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> * not to save the xml, properties in the file system, only in the internal region.
> * the cc region's change listener is to download the jar from other locators
> 
> 
> GEODE-2197: refactor cluster config and fix the test failures
> 
> 
> Diffs
> -----
> 
>   geode-core/src/main/java/org/apache/geode/distributed/internal/SharedConfiguration.java 3119823d6b8886cf893fd38ed58ca90b0c28eeb5 
>   geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ExportSharedConfigurationFunction.java 821e04b2c590bddd28b0727e504c6460d77cf709 
>   geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ImportSharedConfigurationArtifactsFunction.java 2ddeda69177b2aea41ee1d1318d8fdb19324a20b 
>   geode-core/src/main/java/org/apache/geode/management/internal/configuration/SharedConfigurationWriter.java 3af55874282d380e340874a24860fafad7ace126 
>   geode-core/src/main/java/org/apache/geode/management/internal/configuration/callbacks/ConfigurationChangeListener.java c08ba924b652c05436f921c19d65392fd7f6859b 
>   geode-core/src/main/java/org/apache/geode/management/internal/configuration/domain/Configuration.java 4adf2110c4a270127adda1a5090d537a5e2de8c1 
>   geode-core/src/main/java/org/apache/geode/management/internal/configuration/functions/ModifyPropertiesFunction.java 075993716c1afe47ca116727994a3a3661d35a61 
>   geode-core/src/main/java/org/apache/geode/management/internal/configuration/utils/XmlUtils.java 0b4e0f617cf4b5b91f49dc185d7c02b6b7fe0472 
>   geode-core/src/test/java/org/apache/geode/distributed/internal/SharedConfigurationJUnitTest.java 4787c6665cb62eb3dbfe18f3e0128f572d5a36b4 
>   geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigBaseTest.java 63c88dd1aa521a6781a8e07567df661cd42fb6f7 
>   geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigDeployJarDUnitTest.java da4fcf9be6637ddd79403f0a01cb2ba1823f7bd4 
>   geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigImportDUnitTest.java c135f3d9b3370b4a756a2c7688530c63a4e06d77 
>   geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigStartMemberDUnitTest.java cfcbeed86ade004eb305d9a01d6fb128132a21ac 
>   geode-core/src/test/resources/org/apache/geode/codeAnalysis/sanctionedSerializables.txt d6b770f6c9b18e8c090045a66864a027f0ad8062 
> 
> Diff: https://reviews.apache.org/r/54943/diff/
> 
> 
> Testing
> -------
> 
> precheckin successful
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>


Re: Review Request 54943: GEODE-2197: refactor cluster config

Posted by Jinmei Liao <ji...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54943/
-----------------------------------------------------------

(Updated Dec. 22, 2016, 6:48 a.m.)


Review request for geode, Bruce Schuchardt, Jared Stewart, John Blum, Kevin Duling, and Kirk Lund.


Changes
-------

more work to refactor SharedConfiguration class and fix tests


Repository: geode


Description (updated)
-------

* not to save the xml, properties in the file system, only in the internal region.
* the cc region's change listener is to download the jar from other locators


GEODE-2197: refactor cluster config and fix the test failures


Diffs (updated)
-----

  geode-core/src/main/java/org/apache/geode/distributed/internal/SharedConfiguration.java 3119823d6b8886cf893fd38ed58ca90b0c28eeb5 
  geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ExportSharedConfigurationFunction.java 821e04b2c590bddd28b0727e504c6460d77cf709 
  geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ImportSharedConfigurationArtifactsFunction.java 2ddeda69177b2aea41ee1d1318d8fdb19324a20b 
  geode-core/src/main/java/org/apache/geode/management/internal/configuration/SharedConfigurationWriter.java 3af55874282d380e340874a24860fafad7ace126 
  geode-core/src/main/java/org/apache/geode/management/internal/configuration/callbacks/ConfigurationChangeListener.java c08ba924b652c05436f921c19d65392fd7f6859b 
  geode-core/src/main/java/org/apache/geode/management/internal/configuration/domain/Configuration.java 4adf2110c4a270127adda1a5090d537a5e2de8c1 
  geode-core/src/main/java/org/apache/geode/management/internal/configuration/functions/ModifyPropertiesFunction.java 075993716c1afe47ca116727994a3a3661d35a61 
  geode-core/src/main/java/org/apache/geode/management/internal/configuration/utils/XmlUtils.java 0b4e0f617cf4b5b91f49dc185d7c02b6b7fe0472 
  geode-core/src/test/java/org/apache/geode/distributed/internal/SharedConfigurationJUnitTest.java 4787c6665cb62eb3dbfe18f3e0128f572d5a36b4 
  geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigBaseTest.java 63c88dd1aa521a6781a8e07567df661cd42fb6f7 
  geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigDeployJarDUnitTest.java da4fcf9be6637ddd79403f0a01cb2ba1823f7bd4 
  geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigImportDUnitTest.java c135f3d9b3370b4a756a2c7688530c63a4e06d77 
  geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigStartMemberDUnitTest.java cfcbeed86ade004eb305d9a01d6fb128132a21ac 
  geode-core/src/test/resources/org/apache/geode/codeAnalysis/sanctionedSerializables.txt d6b770f6c9b18e8c090045a66864a027f0ad8062 

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


Testing
-------

precheckin successful


Thanks,

Jinmei Liao


Re: Review Request 54943: GEODE-2197: refactor cluster config

Posted by Kirk Lund <ki...@gmail.com>.

> On Dec. 21, 2016, 8:52 p.m., Bruce Schuchardt wrote:
> > geode-core/src/main/java/org/apache/geode/distributed/internal/SharedConfiguration.java, line 743
> > <https://reviews.apache.org/r/54943/diff/1/?file=1590422#file1590422line743>
> >
> >     I can't tell from the diff if this method is rolling-upgrade safe.  If it's just used during jar deployment then there's no problem.  If it's used during locator start-up then it will not work during rolling upgrade and backward-compatibility needs to be addressed.
> >     
> >     If it's just used during jar deployment then you can ignore this comment.
> 
> Jinmei Liao wrote:
>     This is used in locator start-up. So in rolling-upgrade scenario, we would have a locator in 9.1 trying to start up and contacting locator in 9.0 which doesn't have this function?
> 
> Jinmei Liao wrote:
>     In this case, we should be fine. Locator A before shutting down, should have all the deployed jars in its cluster_config directory, when it starts up with newer version, it shouldn't be trying to contact other locators to get the jars because all the deployed jars it knows of is already in it's working dir (it only goes to download jars from other locator when the jar is not in it's own file system).
>     
>     This code is run when a new locator gets started up. we won't run into problems as long as we finish rolling upgrade all the existing locators before starting up new locator.

It would be safest to write a rolling-upgrade test that involves cluster config and jar deployment (even if we write that test as a follow-up to this change).


- Kirk


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


On Dec. 22, 2016, 6:48 a.m., Jinmei Liao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54943/
> -----------------------------------------------------------
> 
> (Updated Dec. 22, 2016, 6:48 a.m.)
> 
> 
> Review request for geode, Bruce Schuchardt, Jared Stewart, John Blum, Kevin Duling, and Kirk Lund.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> * not to save the xml, properties in the file system, only in the internal region.
> * the cc region's change listener is to download the jar from other locators
> 
> 
> GEODE-2197: refactor cluster config and fix the test failures
> 
> 
> Diffs
> -----
> 
>   geode-core/src/main/java/org/apache/geode/distributed/internal/SharedConfiguration.java 3119823d6b8886cf893fd38ed58ca90b0c28eeb5 
>   geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ExportSharedConfigurationFunction.java 821e04b2c590bddd28b0727e504c6460d77cf709 
>   geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ImportSharedConfigurationArtifactsFunction.java 2ddeda69177b2aea41ee1d1318d8fdb19324a20b 
>   geode-core/src/main/java/org/apache/geode/management/internal/configuration/SharedConfigurationWriter.java 3af55874282d380e340874a24860fafad7ace126 
>   geode-core/src/main/java/org/apache/geode/management/internal/configuration/callbacks/ConfigurationChangeListener.java c08ba924b652c05436f921c19d65392fd7f6859b 
>   geode-core/src/main/java/org/apache/geode/management/internal/configuration/domain/Configuration.java 4adf2110c4a270127adda1a5090d537a5e2de8c1 
>   geode-core/src/main/java/org/apache/geode/management/internal/configuration/functions/ModifyPropertiesFunction.java 075993716c1afe47ca116727994a3a3661d35a61 
>   geode-core/src/main/java/org/apache/geode/management/internal/configuration/utils/XmlUtils.java 0b4e0f617cf4b5b91f49dc185d7c02b6b7fe0472 
>   geode-core/src/test/java/org/apache/geode/distributed/internal/SharedConfigurationJUnitTest.java 4787c6665cb62eb3dbfe18f3e0128f572d5a36b4 
>   geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigBaseTest.java 63c88dd1aa521a6781a8e07567df661cd42fb6f7 
>   geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigDeployJarDUnitTest.java da4fcf9be6637ddd79403f0a01cb2ba1823f7bd4 
>   geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigImportDUnitTest.java c135f3d9b3370b4a756a2c7688530c63a4e06d77 
>   geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigStartMemberDUnitTest.java cfcbeed86ade004eb305d9a01d6fb128132a21ac 
>   geode-core/src/test/resources/org/apache/geode/codeAnalysis/sanctionedSerializables.txt d6b770f6c9b18e8c090045a66864a027f0ad8062 
> 
> Diff: https://reviews.apache.org/r/54943/diff/
> 
> 
> Testing
> -------
> 
> precheckin successful
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>


Re: Review Request 54943: GEODE-2197: refactor cluster config

Posted by Jinmei Liao <ji...@pivotal.io>.

> On Dec. 21, 2016, 8:52 p.m., Bruce Schuchardt wrote:
> > geode-core/src/main/java/org/apache/geode/distributed/internal/SharedConfiguration.java, line 743
> > <https://reviews.apache.org/r/54943/diff/1/?file=1590422#file1590422line743>
> >
> >     I can't tell from the diff if this method is rolling-upgrade safe.  If it's just used during jar deployment then there's no problem.  If it's used during locator start-up then it will not work during rolling upgrade and backward-compatibility needs to be addressed.
> >     
> >     If it's just used during jar deployment then you can ignore this comment.
> 
> Jinmei Liao wrote:
>     This is used in locator start-up. So in rolling-upgrade scenario, we would have a locator in 9.1 trying to start up and contacting locator in 9.0 which doesn't have this function?

In this case, we should be fine. Locator A before shutting down, should have all the deployed jars in its cluster_config directory, when it starts up with newer version, it shouldn't be trying to contact other locators to get the jars because all the deployed jars it knows of is already in it's working dir (it only goes to download jars from other locator when the jar is not in it's own file system).

This code is run when a new locator gets started up. we won't run into problems as long as we finish rolling upgrade all the existing locators before starting up new locator.


- Jinmei


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


On Dec. 21, 2016, 6:16 p.m., Jinmei Liao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54943/
> -----------------------------------------------------------
> 
> (Updated Dec. 21, 2016, 6:16 p.m.)
> 
> 
> Review request for geode, Bruce Schuchardt, Jared Stewart, John Blum, Kevin Duling, and Kirk Lund.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> * not to save the xml, properties in the file system, only in the internal region.
> * the cc region's change listener is to download the jar from other locators
> 
> GEODE-2197: refactor cluster config and fix the test failures
> 
> 
> Diffs
> -----
> 
>   geode-core/src/main/java/org/apache/geode/distributed/internal/SharedConfiguration.java 3119823d6b8886cf893fd38ed58ca90b0c28eeb5 
>   geode-core/src/main/java/org/apache/geode/internal/cache/ClusterConfigurationLoader.java 5e48d7ca0d9333b499fc5a9a9b0d4d67eff34856 
>   geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DeployCommands.java a5302eac2389c88d4f2af7b6b0681ae149175287 
>   geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportImportSharedConfigurationCommands.java 8d4677357450b30eb460344571174b044251c5d6 
>   geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ExportSharedConfigurationFunction.java 821e04b2c590bddd28b0727e504c6460d77cf709 
>   geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ImportSharedConfigurationArtifactsFunction.java 2ddeda69177b2aea41ee1d1318d8fdb19324a20b 
>   geode-core/src/main/java/org/apache/geode/management/internal/configuration/SharedConfigurationWriter.java 3af55874282d380e340874a24860fafad7ace126 
>   geode-core/src/main/java/org/apache/geode/management/internal/configuration/callbacks/ConfigurationChangeListener.java c08ba924b652c05436f921c19d65392fd7f6859b 
>   geode-core/src/main/java/org/apache/geode/management/internal/configuration/domain/Configuration.java 4adf2110c4a270127adda1a5090d537a5e2de8c1 
>   geode-core/src/main/java/org/apache/geode/management/internal/configuration/functions/AddJarFunction.java 605233fe69f68c04459dcd4b83cb71a2dc1b20c0 
>   geode-core/src/main/java/org/apache/geode/management/internal/configuration/functions/GetAllJarsFunction.java 0e50524bee26aaf78c8b11977b5d884b869548da 
>   geode-core/src/test/java/org/apache/geode/management/ConnectToLocatorSSLDUnitTest.java 2777b152134c66d9dcc8e047d9d78c1253cf878a 
>   geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfig.java f406d51ca467cd21321a0dfa19cffa832770171d 
>   geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigBaseTest.java 63c88dd1aa521a6781a8e07567df661cd42fb6f7 
>   geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigImportDUnitTest.java c135f3d9b3370b4a756a2c7688530c63a4e06d77 
>   geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigStartMemberDUnitTest.java cfcbeed86ade004eb305d9a01d6fb128132a21ac 
>   geode-core/src/test/java/org/apache/geode/management/internal/configuration/ConfigGroup.java 971f66751505b57a98d07b8680873a249d92a30d 
>   geode-core/src/test/java/org/apache/geode/management/internal/configuration/SharedConfigurationDUnitTest.java 675f5fe25c560fdcccfeb493c759320d12b64caf 
>   geode-core/src/test/resources/org/apache/geode/codeAnalysis/sanctionedSerializables.txt d6b770f6c9b18e8c090045a66864a027f0ad8062 
> 
> Diff: https://reviews.apache.org/r/54943/diff/
> 
> 
> Testing
> -------
> 
> precheckin successful
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>


Re: Review Request 54943: GEODE-2197: refactor cluster config

Posted by Bruce Schuchardt <bs...@pivotal.io>.
If it's used at startup you could have this happen:

    Locator A running 1.0.0
    Locator B running 1.0.0
    Server R running 1.0.0
    Server S running 1.0.0


In a rolling upgrade Locators are rolled to the new version first, one 
by one.

    Locator B running 1.0.0
    Server R running 1.0.0
    Server S running 1.0.0
    Locator A (restarted) running 1.1.0

Here Locator A would bootstrap from old-version Locator B.

In order for your change to work all locators would have to be stopped 
at the same time and then restarted.


Le 12/21/2016 � 1:03 PM, Jinmei Liao a �crit :
>
>> On Dec. 21, 2016, 8:52 p.m., Bruce Schuchardt wrote:
>>> geode-core/src/main/java/org/apache/geode/distributed/internal/SharedConfiguration.java, line 743
>>> <https://reviews.apache.org/r/54943/diff/1/?file=1590422#file1590422line743>
>>>
>>>      I can't tell from the diff if this method is rolling-upgrade safe.  If it's just used during jar deployment then there's no problem.  If it's used during locator start-up then it will not work during rolling upgrade and backward-compatibility needs to be addressed.
>>>      
>>>      If it's just used during jar deployment then you can ignore this comment.
> This is used in locator start-up. So in rolling-upgrade scenario, we would have a locator in 9.1 trying to start up and contacting locator in 9.0 which doesn't have this function?
>
>
> - Jinmei
>
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54943/#review159856
> -----------------------------------------------------------
>
>
> On Dec. 21, 2016, 6:16 p.m., Jinmei Liao wrote:
>> -----------------------------------------------------------
>> This is an automatically generated e-mail. To reply, visit:
>> https://reviews.apache.org/r/54943/
>> -----------------------------------------------------------
>>
>> (Updated Dec. 21, 2016, 6:16 p.m.)
>>
>>
>> Review request for geode, Bruce Schuchardt, Jared Stewart, John Blum, Kevin Duling, and Kirk Lund.
>>
>>
>> Repository: geode
>>
>>
>> Description
>> -------
>>
>> * not to save the xml, properties in the file system, only in the internal region.
>> * the cc region's change listener is to download the jar from other locators
>>
>> GEODE-2197: refactor cluster config and fix the test failures
>>
>>
>> Diffs
>> -----
>>
>>    geode-core/src/main/java/org/apache/geode/distributed/internal/SharedConfiguration.java 3119823d6b8886cf893fd38ed58ca90b0c28eeb5
>>    geode-core/src/main/java/org/apache/geode/internal/cache/ClusterConfigurationLoader.java 5e48d7ca0d9333b499fc5a9a9b0d4d67eff34856
>>    geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DeployCommands.java a5302eac2389c88d4f2af7b6b0681ae149175287
>>    geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportImportSharedConfigurationCommands.java 8d4677357450b30eb460344571174b044251c5d6
>>    geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ExportSharedConfigurationFunction.java 821e04b2c590bddd28b0727e504c6460d77cf709
>>    geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ImportSharedConfigurationArtifactsFunction.java 2ddeda69177b2aea41ee1d1318d8fdb19324a20b
>>    geode-core/src/main/java/org/apache/geode/management/internal/configuration/SharedConfigurationWriter.java 3af55874282d380e340874a24860fafad7ace126
>>    geode-core/src/main/java/org/apache/geode/management/internal/configuration/callbacks/ConfigurationChangeListener.java c08ba924b652c05436f921c19d65392fd7f6859b
>>    geode-core/src/main/java/org/apache/geode/management/internal/configuration/domain/Configuration.java 4adf2110c4a270127adda1a5090d537a5e2de8c1
>>    geode-core/src/main/java/org/apache/geode/management/internal/configuration/functions/AddJarFunction.java 605233fe69f68c04459dcd4b83cb71a2dc1b20c0
>>    geode-core/src/main/java/org/apache/geode/management/internal/configuration/functions/GetAllJarsFunction.java 0e50524bee26aaf78c8b11977b5d884b869548da
>>    geode-core/src/test/java/org/apache/geode/management/ConnectToLocatorSSLDUnitTest.java 2777b152134c66d9dcc8e047d9d78c1253cf878a
>>    geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfig.java f406d51ca467cd21321a0dfa19cffa832770171d
>>    geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigBaseTest.java 63c88dd1aa521a6781a8e07567df661cd42fb6f7
>>    geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigImportDUnitTest.java c135f3d9b3370b4a756a2c7688530c63a4e06d77
>>    geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigStartMemberDUnitTest.java cfcbeed86ade004eb305d9a01d6fb128132a21ac
>>    geode-core/src/test/java/org/apache/geode/management/internal/configuration/ConfigGroup.java 971f66751505b57a98d07b8680873a249d92a30d
>>    geode-core/src/test/java/org/apache/geode/management/internal/configuration/SharedConfigurationDUnitTest.java 675f5fe25c560fdcccfeb493c759320d12b64caf
>>    geode-core/src/test/resources/org/apache/geode/codeAnalysis/sanctionedSerializables.txt d6b770f6c9b18e8c090045a66864a027f0ad8062
>>
>> Diff: https://reviews.apache.org/r/54943/diff/
>>
>>
>> Testing
>> -------
>>
>> precheckin successful
>>
>>
>> Thanks,
>>
>> Jinmei Liao
>>
>>
>


Re: Review Request 54943: GEODE-2197: refactor cluster config

Posted by Jinmei Liao <ji...@pivotal.io>.

> On Dec. 21, 2016, 8:52 p.m., Bruce Schuchardt wrote:
> > geode-core/src/main/java/org/apache/geode/distributed/internal/SharedConfiguration.java, line 743
> > <https://reviews.apache.org/r/54943/diff/1/?file=1590422#file1590422line743>
> >
> >     I can't tell from the diff if this method is rolling-upgrade safe.  If it's just used during jar deployment then there's no problem.  If it's used during locator start-up then it will not work during rolling upgrade and backward-compatibility needs to be addressed.
> >     
> >     If it's just used during jar deployment then you can ignore this comment.

This is used in locator start-up. So in rolling-upgrade scenario, we would have a locator in 9.1 trying to start up and contacting locator in 9.0 which doesn't have this function?


- Jinmei


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


On Dec. 21, 2016, 6:16 p.m., Jinmei Liao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54943/
> -----------------------------------------------------------
> 
> (Updated Dec. 21, 2016, 6:16 p.m.)
> 
> 
> Review request for geode, Bruce Schuchardt, Jared Stewart, John Blum, Kevin Duling, and Kirk Lund.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> * not to save the xml, properties in the file system, only in the internal region.
> * the cc region's change listener is to download the jar from other locators
> 
> GEODE-2197: refactor cluster config and fix the test failures
> 
> 
> Diffs
> -----
> 
>   geode-core/src/main/java/org/apache/geode/distributed/internal/SharedConfiguration.java 3119823d6b8886cf893fd38ed58ca90b0c28eeb5 
>   geode-core/src/main/java/org/apache/geode/internal/cache/ClusterConfigurationLoader.java 5e48d7ca0d9333b499fc5a9a9b0d4d67eff34856 
>   geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DeployCommands.java a5302eac2389c88d4f2af7b6b0681ae149175287 
>   geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportImportSharedConfigurationCommands.java 8d4677357450b30eb460344571174b044251c5d6 
>   geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ExportSharedConfigurationFunction.java 821e04b2c590bddd28b0727e504c6460d77cf709 
>   geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ImportSharedConfigurationArtifactsFunction.java 2ddeda69177b2aea41ee1d1318d8fdb19324a20b 
>   geode-core/src/main/java/org/apache/geode/management/internal/configuration/SharedConfigurationWriter.java 3af55874282d380e340874a24860fafad7ace126 
>   geode-core/src/main/java/org/apache/geode/management/internal/configuration/callbacks/ConfigurationChangeListener.java c08ba924b652c05436f921c19d65392fd7f6859b 
>   geode-core/src/main/java/org/apache/geode/management/internal/configuration/domain/Configuration.java 4adf2110c4a270127adda1a5090d537a5e2de8c1 
>   geode-core/src/main/java/org/apache/geode/management/internal/configuration/functions/AddJarFunction.java 605233fe69f68c04459dcd4b83cb71a2dc1b20c0 
>   geode-core/src/main/java/org/apache/geode/management/internal/configuration/functions/GetAllJarsFunction.java 0e50524bee26aaf78c8b11977b5d884b869548da 
>   geode-core/src/test/java/org/apache/geode/management/ConnectToLocatorSSLDUnitTest.java 2777b152134c66d9dcc8e047d9d78c1253cf878a 
>   geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfig.java f406d51ca467cd21321a0dfa19cffa832770171d 
>   geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigBaseTest.java 63c88dd1aa521a6781a8e07567df661cd42fb6f7 
>   geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigImportDUnitTest.java c135f3d9b3370b4a756a2c7688530c63a4e06d77 
>   geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigStartMemberDUnitTest.java cfcbeed86ade004eb305d9a01d6fb128132a21ac 
>   geode-core/src/test/java/org/apache/geode/management/internal/configuration/ConfigGroup.java 971f66751505b57a98d07b8680873a249d92a30d 
>   geode-core/src/test/java/org/apache/geode/management/internal/configuration/SharedConfigurationDUnitTest.java 675f5fe25c560fdcccfeb493c759320d12b64caf 
>   geode-core/src/test/resources/org/apache/geode/codeAnalysis/sanctionedSerializables.txt d6b770f6c9b18e8c090045a66864a027f0ad8062 
> 
> Diff: https://reviews.apache.org/r/54943/diff/
> 
> 
> Testing
> -------
> 
> precheckin successful
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>


Re: Review Request 54943: GEODE-2197: refactor cluster config

Posted by Bruce Schuchardt <bs...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54943/#review159856
-----------------------------------------------------------



I don't know much about shared-configuration code but I looked through the diff and have one comment.


geode-core/src/main/java/org/apache/geode/distributed/internal/SharedConfiguration.java (line 635)
<https://reviews.apache.org/r/54943/#comment230890>

    I can't tell from the diff if this method is rolling-upgrade safe.  If it's just used during jar deployment then there's no problem.  If it's used during locator start-up then it will not work during rolling upgrade and backward-compatibility needs to be addressed.
    
    If it's just used during jar deployment then you can ignore this comment.


- Bruce Schuchardt


On Dec. 21, 2016, 6:16 p.m., Jinmei Liao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54943/
> -----------------------------------------------------------
> 
> (Updated Dec. 21, 2016, 6:16 p.m.)
> 
> 
> Review request for geode, Bruce Schuchardt, Jared Stewart, John Blum, Kevin Duling, and Kirk Lund.
> 
> 
> Repository: geode
> 
> 
> Description
> -------
> 
> * not to save the xml, properties in the file system, only in the internal region.
> * the cc region's change listener is to download the jar from other locators
> 
> GEODE-2197: refactor cluster config and fix the test failures
> 
> 
> Diffs
> -----
> 
>   geode-core/src/main/java/org/apache/geode/distributed/internal/SharedConfiguration.java 3119823d6b8886cf893fd38ed58ca90b0c28eeb5 
>   geode-core/src/main/java/org/apache/geode/internal/cache/ClusterConfigurationLoader.java 5e48d7ca0d9333b499fc5a9a9b0d4d67eff34856 
>   geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DeployCommands.java a5302eac2389c88d4f2af7b6b0681ae149175287 
>   geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportImportSharedConfigurationCommands.java 8d4677357450b30eb460344571174b044251c5d6 
>   geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ExportSharedConfigurationFunction.java 821e04b2c590bddd28b0727e504c6460d77cf709 
>   geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ImportSharedConfigurationArtifactsFunction.java 2ddeda69177b2aea41ee1d1318d8fdb19324a20b 
>   geode-core/src/main/java/org/apache/geode/management/internal/configuration/SharedConfigurationWriter.java 3af55874282d380e340874a24860fafad7ace126 
>   geode-core/src/main/java/org/apache/geode/management/internal/configuration/callbacks/ConfigurationChangeListener.java c08ba924b652c05436f921c19d65392fd7f6859b 
>   geode-core/src/main/java/org/apache/geode/management/internal/configuration/domain/Configuration.java 4adf2110c4a270127adda1a5090d537a5e2de8c1 
>   geode-core/src/main/java/org/apache/geode/management/internal/configuration/functions/AddJarFunction.java 605233fe69f68c04459dcd4b83cb71a2dc1b20c0 
>   geode-core/src/main/java/org/apache/geode/management/internal/configuration/functions/GetAllJarsFunction.java 0e50524bee26aaf78c8b11977b5d884b869548da 
>   geode-core/src/test/java/org/apache/geode/management/ConnectToLocatorSSLDUnitTest.java 2777b152134c66d9dcc8e047d9d78c1253cf878a 
>   geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfig.java f406d51ca467cd21321a0dfa19cffa832770171d 
>   geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigBaseTest.java 63c88dd1aa521a6781a8e07567df661cd42fb6f7 
>   geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigImportDUnitTest.java c135f3d9b3370b4a756a2c7688530c63a4e06d77 
>   geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigStartMemberDUnitTest.java cfcbeed86ade004eb305d9a01d6fb128132a21ac 
>   geode-core/src/test/java/org/apache/geode/management/internal/configuration/ConfigGroup.java 971f66751505b57a98d07b8680873a249d92a30d 
>   geode-core/src/test/java/org/apache/geode/management/internal/configuration/SharedConfigurationDUnitTest.java 675f5fe25c560fdcccfeb493c759320d12b64caf 
>   geode-core/src/test/resources/org/apache/geode/codeAnalysis/sanctionedSerializables.txt d6b770f6c9b18e8c090045a66864a027f0ad8062 
> 
> Diff: https://reviews.apache.org/r/54943/diff/
> 
> 
> Testing
> -------
> 
> precheckin successful
> 
> 
> Thanks,
> 
> Jinmei Liao
> 
>


Re: Review Request 54943: GEODE-2197: refactor cluster config

Posted by Jinmei Liao <ji...@pivotal.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54943/
-----------------------------------------------------------

(Updated Dec. 21, 2016, 6:16 p.m.)


Review request for geode, Bruce Schuchardt, Jared Stewart, John Blum, Kevin Duling, and Kirk Lund.


Repository: geode


Description
-------

* not to save the xml, properties in the file system, only in the internal region.
* the cc region's change listener is to download the jar from other locators

GEODE-2197: refactor cluster config and fix the test failures


Diffs
-----

  geode-core/src/main/java/org/apache/geode/distributed/internal/SharedConfiguration.java 3119823d6b8886cf893fd38ed58ca90b0c28eeb5 
  geode-core/src/main/java/org/apache/geode/internal/cache/ClusterConfigurationLoader.java 5e48d7ca0d9333b499fc5a9a9b0d4d67eff34856 
  geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/DeployCommands.java a5302eac2389c88d4f2af7b6b0681ae149175287 
  geode-core/src/main/java/org/apache/geode/management/internal/cli/commands/ExportImportSharedConfigurationCommands.java 8d4677357450b30eb460344571174b044251c5d6 
  geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ExportSharedConfigurationFunction.java 821e04b2c590bddd28b0727e504c6460d77cf709 
  geode-core/src/main/java/org/apache/geode/management/internal/cli/functions/ImportSharedConfigurationArtifactsFunction.java 2ddeda69177b2aea41ee1d1318d8fdb19324a20b 
  geode-core/src/main/java/org/apache/geode/management/internal/configuration/SharedConfigurationWriter.java 3af55874282d380e340874a24860fafad7ace126 
  geode-core/src/main/java/org/apache/geode/management/internal/configuration/callbacks/ConfigurationChangeListener.java c08ba924b652c05436f921c19d65392fd7f6859b 
  geode-core/src/main/java/org/apache/geode/management/internal/configuration/domain/Configuration.java 4adf2110c4a270127adda1a5090d537a5e2de8c1 
  geode-core/src/main/java/org/apache/geode/management/internal/configuration/functions/AddJarFunction.java 605233fe69f68c04459dcd4b83cb71a2dc1b20c0 
  geode-core/src/main/java/org/apache/geode/management/internal/configuration/functions/GetAllJarsFunction.java 0e50524bee26aaf78c8b11977b5d884b869548da 
  geode-core/src/test/java/org/apache/geode/management/ConnectToLocatorSSLDUnitTest.java 2777b152134c66d9dcc8e047d9d78c1253cf878a 
  geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfig.java f406d51ca467cd21321a0dfa19cffa832770171d 
  geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigBaseTest.java 63c88dd1aa521a6781a8e07567df661cd42fb6f7 
  geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigImportDUnitTest.java c135f3d9b3370b4a756a2c7688530c63a4e06d77 
  geode-core/src/test/java/org/apache/geode/management/internal/configuration/ClusterConfigStartMemberDUnitTest.java cfcbeed86ade004eb305d9a01d6fb128132a21ac 
  geode-core/src/test/java/org/apache/geode/management/internal/configuration/ConfigGroup.java 971f66751505b57a98d07b8680873a249d92a30d 
  geode-core/src/test/java/org/apache/geode/management/internal/configuration/SharedConfigurationDUnitTest.java 675f5fe25c560fdcccfeb493c759320d12b64caf 
  geode-core/src/test/resources/org/apache/geode/codeAnalysis/sanctionedSerializables.txt d6b770f6c9b18e8c090045a66864a027f0ad8062 

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


Testing (updated)
-------

precheckin successful


Thanks,

Jinmei Liao