You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@atlas.apache.org by Madhan Neethiraj <ma...@apache.org> on 2016/07/12 01:29:09 UTC
Review Request 49941: ATLAS-998: determine HA mode from property
atlas.server.ids, instead of atlas.server.ha.enabled
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49941/
-----------------------------------------------------------
Review request for atlas.
Bugs: ATLAS-998
https://issues.apache.org/jira/browse/ATLAS-998
Repository: atlas
Description
-------
Updated to use atlas.server.ids property to determine if multiple instances of Atlas are configured to run (in HA mode), instead of atlas.server.ha.enabled
Diffs
-----
common/src/main/java/org/apache/atlas/ha/HAConfiguration.java f3c9bc5
common/src/test/java/org/apache/atlas/ha/HAConfigurationTest.java a3129c9
repository/src/test/java/org/apache/atlas/repository/audit/HBaseBasedAuditRepositoryHATest.java 2f7edb4
repository/src/test/java/org/apache/atlas/repository/graph/GraphBackedSearchIndexerMockTest.java 94dd298
repository/src/test/java/org/apache/atlas/services/DefaultMetadataServiceMockTest.java 393b539
webapp/src/test/java/org/apache/atlas/notification/NotificationHookConsumerTest.java 7860eb6
webapp/src/test/java/org/apache/atlas/web/service/ActiveInstanceElectorServiceTest.java 2b7691f
webapp/src/test/java/org/apache/atlas/web/service/ServiceStateTest.java 77aba88
Diff: https://reviews.apache.org/r/49941/diff/
Testing
-------
- Verified that all unit tests succeed
- Verified that when multiple instances are listed in 'atlas.server.ids' configuration, Atlas server takes HA code path
Thanks,
Madhan Neethiraj
Re: Review Request 49941: ATLAS-998: determine HA mode from property
atlas.server.ids, instead of atlas.server.ha.enabled
Posted by Shwetha GS <ss...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49941/#review142364
-----------------------------------------------------------
Fix it, then Ship it!
Minor comments. Will fix and commit. Thanks
common/src/test/java/org/apache/atlas/ha/HAConfigurationTest.java (line 76)
<https://reviews.apache.org/r/49941/#comment207940>
Add test which checks if haenabled=true when ha.configuration is not set and ids is set
repository/src/test/java/org/apache/atlas/repository/audit/HBaseBasedAuditRepositoryHATest.java (line 58)
<https://reviews.apache.org/r/49941/#comment207941>
The changes in these tests not required. revert?
- Shwetha GS
On July 13, 2016, 2:59 p.m., Madhan Neethiraj wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49941/
> -----------------------------------------------------------
>
> (Updated July 13, 2016, 2:59 p.m.)
>
>
> Review request for atlas.
>
>
> Bugs: ATLAS-998
> https://issues.apache.org/jira/browse/ATLAS-998
>
>
> Repository: atlas
>
>
> Description
> -------
>
> Updated to use atlas.server.ids property to determine if multiple instances of Atlas are configured to run (in HA mode), instead of atlas.server.ha.enabled
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/atlas/ha/HAConfiguration.java f3c9bc5
> common/src/test/java/org/apache/atlas/ha/HAConfigurationTest.java a3129c9
> repository/src/test/java/org/apache/atlas/repository/audit/HBaseBasedAuditRepositoryHATest.java 2f7edb4
> repository/src/test/java/org/apache/atlas/repository/graph/GraphBackedSearchIndexerMockTest.java 94dd298
> repository/src/test/java/org/apache/atlas/services/DefaultMetadataServiceMockTest.java 393b539
> webapp/src/test/java/org/apache/atlas/notification/NotificationHookConsumerTest.java 7860eb6
> webapp/src/test/java/org/apache/atlas/web/service/ActiveInstanceElectorServiceTest.java 2b7691f
> webapp/src/test/java/org/apache/atlas/web/service/ServiceStateTest.java 77aba88
>
> Diff: https://reviews.apache.org/r/49941/diff/
>
>
> Testing
> -------
>
> - Verified that all unit tests succeed
> - Verified that when multiple instances are listed in 'atlas.server.ids' configuration, Atlas server takes HA code path
>
>
> Thanks,
>
> Madhan Neethiraj
>
>
Re: Review Request 49941: ATLAS-998: determine HA mode from property
atlas.server.ids, instead of atlas.server.ha.enabled
Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49941/
-----------------------------------------------------------
(Updated July 13, 2016, 2:59 p.m.)
Review request for atlas.
Changes
-------
Addressed review comment to support "ha.enabled" property as well.
Bugs: ATLAS-998
https://issues.apache.org/jira/browse/ATLAS-998
Repository: atlas
Description
-------
Updated to use atlas.server.ids property to determine if multiple instances of Atlas are configured to run (in HA mode), instead of atlas.server.ha.enabled
Diffs (updated)
-----
common/src/main/java/org/apache/atlas/ha/HAConfiguration.java f3c9bc5
common/src/test/java/org/apache/atlas/ha/HAConfigurationTest.java a3129c9
repository/src/test/java/org/apache/atlas/repository/audit/HBaseBasedAuditRepositoryHATest.java 2f7edb4
repository/src/test/java/org/apache/atlas/repository/graph/GraphBackedSearchIndexerMockTest.java 94dd298
repository/src/test/java/org/apache/atlas/services/DefaultMetadataServiceMockTest.java 393b539
webapp/src/test/java/org/apache/atlas/notification/NotificationHookConsumerTest.java 7860eb6
webapp/src/test/java/org/apache/atlas/web/service/ActiveInstanceElectorServiceTest.java 2b7691f
webapp/src/test/java/org/apache/atlas/web/service/ServiceStateTest.java 77aba88
Diff: https://reviews.apache.org/r/49941/diff/
Testing
-------
- Verified that all unit tests succeed
- Verified that when multiple instances are listed in 'atlas.server.ids' configuration, Atlas server takes HA code path
Thanks,
Madhan Neethiraj
Re: Review Request 49941: ATLAS-998: determine HA mode from property
atlas.server.ids, instead of atlas.server.ha.enabled
Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49941/
-----------------------------------------------------------
(Updated July 13, 2016, 6:20 a.m.)
Review request for atlas.
Changes
-------
Addressed review comments
Bugs: ATLAS-998
https://issues.apache.org/jira/browse/ATLAS-998
Repository: atlas
Description
-------
Updated to use atlas.server.ids property to determine if multiple instances of Atlas are configured to run (in HA mode), instead of atlas.server.ha.enabled
Diffs (updated)
-----
common/src/main/java/org/apache/atlas/ha/HAConfiguration.java f3c9bc5
common/src/test/java/org/apache/atlas/ha/HAConfigurationTest.java a3129c9
repository/src/test/java/org/apache/atlas/repository/audit/HBaseBasedAuditRepositoryHATest.java 2f7edb4
repository/src/test/java/org/apache/atlas/repository/graph/GraphBackedSearchIndexerMockTest.java 94dd298
repository/src/test/java/org/apache/atlas/services/DefaultMetadataServiceMockTest.java 393b539
webapp/src/test/java/org/apache/atlas/notification/NotificationHookConsumerTest.java 7860eb6
webapp/src/test/java/org/apache/atlas/web/service/ActiveInstanceElectorServiceTest.java 2b7691f
webapp/src/test/java/org/apache/atlas/web/service/ServiceStateTest.java 77aba88
Diff: https://reviews.apache.org/r/49941/diff/
Testing
-------
- Verified that all unit tests succeed
- Verified that when multiple instances are listed in 'atlas.server.ids' configuration, Atlas server takes HA code path
Thanks,
Madhan Neethiraj
Re: Review Request 49941: ATLAS-998: determine HA mode from property
atlas.server.ids, instead of atlas.server.ha.enabled
Posted by Hemanth Yamijala <yh...@gmail.com>.
> On July 12, 2016, 5:52 a.m., Shwetha GS wrote:
> > common/src/main/java/org/apache/atlas/ha/HAConfiguration.java, line 65
> > <https://reviews.apache.org/r/49941/diff/1/?file=1441953#file1441953line65>
> >
> > To make it backward compatible, as 0.7 is already out, can the check be if ha.enabled or ids > 1?
>
> Madhan Neethiraj wrote:
> The fix is to remove 'ha.enabled' property. Instead of keeping it for backward compatibility, I would suggest to document the removal of this property in the next release notes.
>
> Shwetha GS wrote:
> The fix is to use ids property to enable HA. For someone upgrading, all these are manual changes and add to upgrade checklist and its better to minimise if possible.
>
> Can you update docs/src/site/twiki/HighAvailability.twiki?
>
> Its difficult to compile this list during release. Can you create upgrade.twiki under docs/src/site/twiki/ and add to it?
One other suggestion (can be done later in a follow-up JIRA as well): We could log a warning if we find the ha enabled property in the configuration and mention we're not using it as it is no longer supported.
We also need to change docs/src/site/twiki/Configuration.twiki, under High Availability properties.
- Hemanth
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49941/#review141855
-----------------------------------------------------------
On July 13, 2016, 6:20 a.m., Madhan Neethiraj wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49941/
> -----------------------------------------------------------
>
> (Updated July 13, 2016, 6:20 a.m.)
>
>
> Review request for atlas.
>
>
> Bugs: ATLAS-998
> https://issues.apache.org/jira/browse/ATLAS-998
>
>
> Repository: atlas
>
>
> Description
> -------
>
> Updated to use atlas.server.ids property to determine if multiple instances of Atlas are configured to run (in HA mode), instead of atlas.server.ha.enabled
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/atlas/ha/HAConfiguration.java f3c9bc5
> common/src/test/java/org/apache/atlas/ha/HAConfigurationTest.java a3129c9
> repository/src/test/java/org/apache/atlas/repository/audit/HBaseBasedAuditRepositoryHATest.java 2f7edb4
> repository/src/test/java/org/apache/atlas/repository/graph/GraphBackedSearchIndexerMockTest.java 94dd298
> repository/src/test/java/org/apache/atlas/services/DefaultMetadataServiceMockTest.java 393b539
> webapp/src/test/java/org/apache/atlas/notification/NotificationHookConsumerTest.java 7860eb6
> webapp/src/test/java/org/apache/atlas/web/service/ActiveInstanceElectorServiceTest.java 2b7691f
> webapp/src/test/java/org/apache/atlas/web/service/ServiceStateTest.java 77aba88
>
> Diff: https://reviews.apache.org/r/49941/diff/
>
>
> Testing
> -------
>
> - Verified that all unit tests succeed
> - Verified that when multiple instances are listed in 'atlas.server.ids' configuration, Atlas server takes HA code path
>
>
> Thanks,
>
> Madhan Neethiraj
>
>
Re: Review Request 49941: ATLAS-998: determine HA mode from property
atlas.server.ids, instead of atlas.server.ha.enabled
Posted by Shwetha GS <ss...@hortonworks.com>.
> On July 12, 2016, 5:52 a.m., Shwetha GS wrote:
> > common/src/main/java/org/apache/atlas/ha/HAConfiguration.java, line 65
> > <https://reviews.apache.org/r/49941/diff/1/?file=1441953#file1441953line65>
> >
> > To make it backward compatible, as 0.7 is already out, can the check be if ha.enabled or ids > 1?
>
> Madhan Neethiraj wrote:
> The fix is to remove 'ha.enabled' property. Instead of keeping it for backward compatibility, I would suggest to document the removal of this property in the next release notes.
The fix is to use ids property to enable HA. For someone upgrading, all these are manual changes and add to upgrade checklist and its better to minimise if possible.
Can you update docs/src/site/twiki/HighAvailability.twiki?
Its difficult to compile this list during release. Can you create upgrade.twiki under docs/src/site/twiki/ and add to it?
- Shwetha
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49941/#review141855
-----------------------------------------------------------
On July 13, 2016, 6:20 a.m., Madhan Neethiraj wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49941/
> -----------------------------------------------------------
>
> (Updated July 13, 2016, 6:20 a.m.)
>
>
> Review request for atlas.
>
>
> Bugs: ATLAS-998
> https://issues.apache.org/jira/browse/ATLAS-998
>
>
> Repository: atlas
>
>
> Description
> -------
>
> Updated to use atlas.server.ids property to determine if multiple instances of Atlas are configured to run (in HA mode), instead of atlas.server.ha.enabled
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/atlas/ha/HAConfiguration.java f3c9bc5
> common/src/test/java/org/apache/atlas/ha/HAConfigurationTest.java a3129c9
> repository/src/test/java/org/apache/atlas/repository/audit/HBaseBasedAuditRepositoryHATest.java 2f7edb4
> repository/src/test/java/org/apache/atlas/repository/graph/GraphBackedSearchIndexerMockTest.java 94dd298
> repository/src/test/java/org/apache/atlas/services/DefaultMetadataServiceMockTest.java 393b539
> webapp/src/test/java/org/apache/atlas/notification/NotificationHookConsumerTest.java 7860eb6
> webapp/src/test/java/org/apache/atlas/web/service/ActiveInstanceElectorServiceTest.java 2b7691f
> webapp/src/test/java/org/apache/atlas/web/service/ServiceStateTest.java 77aba88
>
> Diff: https://reviews.apache.org/r/49941/diff/
>
>
> Testing
> -------
>
> - Verified that all unit tests succeed
> - Verified that when multiple instances are listed in 'atlas.server.ids' configuration, Atlas server takes HA code path
>
>
> Thanks,
>
> Madhan Neethiraj
>
>
Re: Review Request 49941: ATLAS-998: determine HA mode from property
atlas.server.ids, instead of atlas.server.ha.enabled
Posted by Madhan Neethiraj <ma...@apache.org>.
> On July 12, 2016, 5:52 a.m., Shwetha GS wrote:
> > common/src/main/java/org/apache/atlas/ha/HAConfiguration.java, line 65
> > <https://reviews.apache.org/r/49941/diff/1/?file=1441953#file1441953line65>
> >
> > To make it backward compatible, as 0.7 is already out, can the check be if ha.enabled or ids > 1?
The fix is to remove 'ha.enabled' property. Instead of keeping it for backward compatibility, I would suggest to document the removal of this property in the next release notes.
- Madhan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49941/#review141855
-----------------------------------------------------------
On July 12, 2016, 1:29 a.m., Madhan Neethiraj wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49941/
> -----------------------------------------------------------
>
> (Updated July 12, 2016, 1:29 a.m.)
>
>
> Review request for atlas.
>
>
> Bugs: ATLAS-998
> https://issues.apache.org/jira/browse/ATLAS-998
>
>
> Repository: atlas
>
>
> Description
> -------
>
> Updated to use atlas.server.ids property to determine if multiple instances of Atlas are configured to run (in HA mode), instead of atlas.server.ha.enabled
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/atlas/ha/HAConfiguration.java f3c9bc5
> common/src/test/java/org/apache/atlas/ha/HAConfigurationTest.java a3129c9
> repository/src/test/java/org/apache/atlas/repository/audit/HBaseBasedAuditRepositoryHATest.java 2f7edb4
> repository/src/test/java/org/apache/atlas/repository/graph/GraphBackedSearchIndexerMockTest.java 94dd298
> repository/src/test/java/org/apache/atlas/services/DefaultMetadataServiceMockTest.java 393b539
> webapp/src/test/java/org/apache/atlas/notification/NotificationHookConsumerTest.java 7860eb6
> webapp/src/test/java/org/apache/atlas/web/service/ActiveInstanceElectorServiceTest.java 2b7691f
> webapp/src/test/java/org/apache/atlas/web/service/ServiceStateTest.java 77aba88
>
> Diff: https://reviews.apache.org/r/49941/diff/
>
>
> Testing
> -------
>
> - Verified that all unit tests succeed
> - Verified that when multiple instances are listed in 'atlas.server.ids' configuration, Atlas server takes HA code path
>
>
> Thanks,
>
> Madhan Neethiraj
>
>
Re: Review Request 49941: ATLAS-998: determine HA mode from property
atlas.server.ids, instead of atlas.server.ha.enabled
Posted by Shwetha GS <ss...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49941/#review141855
-----------------------------------------------------------
common/src/main/java/org/apache/atlas/ha/HAConfiguration.java (line 55)
<https://reviews.apache.org/r/49941/#comment207255>
move it to tests
common/src/main/java/org/apache/atlas/ha/HAConfiguration.java (line 64)
<https://reviews.apache.org/r/49941/#comment207256>
To make it backward compatible, as 0.7 is already out, can the check be if ha.enabled or ids > 1?
- Shwetha GS
On July 12, 2016, 1:29 a.m., Madhan Neethiraj wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49941/
> -----------------------------------------------------------
>
> (Updated July 12, 2016, 1:29 a.m.)
>
>
> Review request for atlas.
>
>
> Bugs: ATLAS-998
> https://issues.apache.org/jira/browse/ATLAS-998
>
>
> Repository: atlas
>
>
> Description
> -------
>
> Updated to use atlas.server.ids property to determine if multiple instances of Atlas are configured to run (in HA mode), instead of atlas.server.ha.enabled
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/atlas/ha/HAConfiguration.java f3c9bc5
> common/src/test/java/org/apache/atlas/ha/HAConfigurationTest.java a3129c9
> repository/src/test/java/org/apache/atlas/repository/audit/HBaseBasedAuditRepositoryHATest.java 2f7edb4
> repository/src/test/java/org/apache/atlas/repository/graph/GraphBackedSearchIndexerMockTest.java 94dd298
> repository/src/test/java/org/apache/atlas/services/DefaultMetadataServiceMockTest.java 393b539
> webapp/src/test/java/org/apache/atlas/notification/NotificationHookConsumerTest.java 7860eb6
> webapp/src/test/java/org/apache/atlas/web/service/ActiveInstanceElectorServiceTest.java 2b7691f
> webapp/src/test/java/org/apache/atlas/web/service/ServiceStateTest.java 77aba88
>
> Diff: https://reviews.apache.org/r/49941/diff/
>
>
> Testing
> -------
>
> - Verified that all unit tests succeed
> - Verified that when multiple instances are listed in 'atlas.server.ids' configuration, Atlas server takes HA code path
>
>
> Thanks,
>
> Madhan Neethiraj
>
>