> On April 12, 2018, 11:49 p.m., Madhan Neethiraj wrote:
> > common/src/main/java/org/apache/atlas/repository/Constants.java
> > Line 56 (original), 56 (patched)
> > <https://reviews.apache.org/r/66064/diff/1/?file=1997842#file1997842line56>
> >
> >     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.

I believe I have a solution in place, but would like feedback on implementation.


> On April 12, 2018, 11:49 p.m., Madhan Neethiraj wrote:
> > pom.xml
> > Line 674 (original), 675 (patched)
> > <https://reviews.apache.org/r/66064/diff/1/?file=1997853#file1997853line675>
> >
> >     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.

Only for the standard -Pdist profile. This shouldn't be an issue since the 
newer elasticsearch jars contain no shadded classes and should not conflict.


- Pierre


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


On April 13, 2018, 2:55 a.m., Pierre Padovani wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66064/
> -----------------------------------------------------------
> 
> (Updated April 13, 2018, 2:55 a.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 01e49157d 
>   distro/pom.xml 1f4c6d557 
>   distro/src/bin/atlas_config.py 9062da649 
>   distro/src/bin/atlas_start.py 61d69eb21 
>   distro/src/bin/atlas_stop.py 94c3d6d46 
>   distro/src/conf/atlas-env.sh 9213f488b 
>   distro/src/main/assemblies/standalone-package.xml dc2a66ba9 
>   docs/src/site/twiki/Configuration.twiki 63c3fce96 
>   docs/src/site/twiki/HighAvailability.twiki 4270d0974 
>   docs/src/site/twiki/InstallationSteps.twiki dca0618e3 
>   graphdb/janus/pom.xml 016a09c33 
>   pom.xml 4d2ab7391 
>   webapp/pom.xml 284f538f7 
> 
> 
> Diff: https://reviews.apache.org/r/66064/diff/2/
> 
> 
> 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