> On May 7, 2018, 10:39 p.m., Madhan Neethiraj wrote: > > pom.xml > > Line 675 (original), 676 (patched) > > <https://reviews.apache.org/r/66064/diff/3/?file=2017714#file2017714line676> > > > > Will this change end up including elasticsearch libraries even when > > building for solr? > > Pierre Padovani wrote: > Yes... however the new 5.x version of the Elasticsearch jars no longer > shade 3rd party libraries so are far safer than the earlier versions. So we > have a choice, include the jars, or force users who wish to use Elasticsearch > as the search backend to copy in the jars manually. > > Madhan Neethiraj wrote: > Pierre - I would suggest to not package libraries from multiple search > backends in Atlas. It should be done either via profiles or as you suggest by > manually copying necessary libraries. > > Pierre Padovani wrote: > Not to be obstinate about this... but have you tried to install a > different set of libraries into the default atlas distribution? The lib > directory doesn't exist, so you have to: > > -start atlas > -have it unpack itself > -fail to start > -stop atlas > -copy in the libraries needed > -start atlas > > The above assumes the user isn't going to remove the solr jars... since > they likely don't know about them. In the end it is very likely that they > will end up with both search backend jars if they choose to use Elasticsearch. > > IMHO - This indicates that there is a bigger distribution packaging issue > in place. What we should do is not include any backend or search jars in the > default war build. Instead include them in the zip, and provide a setup > script that allows the user to choose their setup which in turn unpacks and > copies in the require jars. > > Finally, can you enlighten me as to the issue with including the jars in > the default distribution? I understand it is your preference, but I fail to > grasp what the conflict happens to be except for overall package size. > > Madhan Neethiraj wrote: > This removes existing exclusion of all elasicsearch libraries > (WEB-INF/lib/elasticsearch-*.jar). Can you list all libraries included in > Atlas packaging due to this change? > > >> can you enlighten me as to the issue with including the jars in the > default distribution? > Have you validated running Atlas with Solr after this change (which > packages elastic-search JARs)? If you already validated, it should be okay to > push this commit.
Here are the two jars that are included in the build: elasticsearch-rest-client-5.6.4.jar elasticsearch-rest-high-level-client-5.6.4.jar To verify that there is no conflict, I rebuilt with embedded hbase and solr, then copied in the above two jars. I was able to run, create an entity and search for it without any issues. - Pierre ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66064/#review202599 ----------------------------------------------------------- On May 16, 2018, 2:29 p.m., Pierre Padovani wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/66064/ > ----------------------------------------------------------- > > (Updated May 16, 2018, 2:29 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 3732556ae > distro/pom.xml 379ff0d39 > 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 053cbd500 > distro/src/main/assemblies/standalone-package.xml dc88add56 > 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 3469a393a > webapp/pom.xml 03b84087f > > > Diff: https://reviews.apache.org/r/66064/diff/5/ > > > 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 > >
