This is an automatically generated e-mail. To reply, visit:

Line 56 (original), 56 (patched)

    As you noted in the description, this would require re-indexing of existing 
deployments - which can be very expensive depending upon the amount of data. 
This should be avoided.
    If an index software has restrictions on the keynames, consider replacing 
these constants with calls to instance-specific property key provider. And 
implement a default one with current values and ElasticSearch specific one with 
necessary key names.

Line 674 (original), 675 (patched)

    Would this change cause elasticsearch libraries to be included for Solr 
deployments as well? If yes, please consider alternate approach (like a 
separate profile) for Elasticsearch - to avoid potential issues in deployments 
that use Solr.
    Same applies for line #694 below.

Lines 398 (patched)

    Consider adding elastic-search libraries only for specific profiles.

- Madhan Neethiraj

On April 12, 2018, 5:35 p.m., Pierre Padovani wrote:
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66064/
> -----------------------------------------------------------
> (Updated April 12, 2018, 5:35 p.m.)
> Review request for atlas, David Radley, Madhan Neethiraj, and Sarath 
> Subramanian.
> Bugs: ATLAS-2478
>     https://issues.apache.org/jira/browse/ATLAS-2478
> Repository: atlas
> Description
> -------
> This patch fixes the Elasticsearch support for JanusGraph 0.2.0 and updates 
> documentation.
> Included with this patch is an update to the berkley-elasticsearch profile to 
> automatically download and include elasticsearch as a side application much 
> like solr is. Updates to the start/stop/conf scripts are included as well.
> NOTE: This patch includes a **BACKWARDS INCOMPATIBLE** change to 
> /atlas/common/src/main/java/org/apache/atlas/repository/Constants.java. There 
> are six constants that are incorrectly named with a '.' (dot). This is not 
> supported in Elasticsearch 5 and beyond when defining a mapping **UNLESS** 
> the field names can be collectively thought of as an object. In the case of 
> the fields defined in the Constants.java file, 'type' is defined as a string 
> field, and 'type.name' is also defined as a string field. Elasticsearch sees 
> this as an error, since it cannot convert type to an object. The fix included 
> simply changes the field names from using a '.' (dot) to an '_' (underscore). 
> This should NOT affect compatibility with hbase/solr for new installs. For 
> existing installations, a reindex will be required as the field names will 
> have changed.
> **Query**: There is a way we can simplify integration/unit tests for the 
> in-memory graph store by using a maven plugin that will download and run an 
> elasticsearch node. This is nothing more than a maven change, and change to 
> the atlas-application.properties to switch to elasticsearch from solr. I did 
> not implement this, but am curious if this change would be desired. If so, 
> this can be done with a separate ticket.
> Diffs
> -----
>   common/src/main/java/org/apache/atlas/repository/Constants.java 605742dd 
>   distro/pom.xml 0103bef6 
>   distro/src/bin/atlas_config.py e6415cf4 
>   distro/src/bin/atlas_start.py 39be6b7c 
>   distro/src/bin/atlas_stop.py 66edd904 
>   distro/src/conf/atlas-env.sh 68b24e93 
>   distro/src/main/assemblies/standalone-package.xml 1881082e 
>   docs/src/site/twiki/Configuration.twiki 63c3fce9 
>   docs/src/site/twiki/HighAvailability.twiki 4270d097 
>   docs/src/site/twiki/InstallationSteps.twiki 6b9f0313 
>   graphdb/janus/pom.xml 143b775f 
>   pom.xml 34e1ad04 
>   webapp/pom.xml 284f538f 
> Diff: https://reviews.apache.org/r/66064/diff/1/
> Testing
> -------
> We currently use this fix with our Atlas setup in a fork of the Atlas code, 
> and have found no issues with it. Additionally, with the update to the 
> berkley-elasticsearch profile, I have extensively tested that profile to make 
> sure that management of Elasticsearch functioned correctly.
> Thanks,
> Pierre Padovani

Reply via email to