atlas-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ashutosh Mestry <ames...@hortonworks.com>
Subject Re: Review Request 59378: Basic Search Performance Improvement
Date Thu, 18 May 2017 23:52:29 GMT


> On May 18, 2017, 11:42 p.m., Apoorv Naik wrote:
> > distro/src/conf/atlas-application.properties
> > Lines 66 (patched)
> > <https://reviews.apache.org/r/59378/diff/1/?file=1724166#file1724166line66>
> >
> >     Will this be the default value when installing atlas and running for the first
time ? If 150 is the desired value then this has to change.

Yes, this will be default.


> On May 18, 2017, 11:42 p.m., Apoorv Naik wrote:
> > distro/src/conf/solr/solrconfig.xml
> > Line 38 (original), 39 (patched)
> > <https://reviews.apache.org/r/59378/diff/1/?file=1724167#file1724167line39>
> >
> >     Are you sure this is safe ? I remember there was some issue when using berkeley
and elasticsearch with Lucene 5.5.1

I have addressed this.


> On May 18, 2017, 11:42 p.m., Apoorv Naik wrote:
> > repository/src/main/java/org/apache/atlas/discovery/EntityDiscoveryService.java
> > Line 74 (original), 78 (patched)
> > <https://reviews.apache.org/r/59378/diff/1/?file=1724172#file1724172line80>
> >
> >     * for only 3 imports ? 
> >      
> >     Revert this change.

This is my IntelliJ setting (optimize imports). I will fix this.


> On May 18, 2017, 11:42 p.m., Apoorv Naik wrote:
> > repository/src/test/java/org/apache/atlas/services/EntityDiscoveryServiceTest.java
> > Lines 102 (patched)
> > <https://reviews.apache.org/r/59378/diff/1/?file=1724173#file1724173line102>
> >
> >     Let's not use Whitebox in UT, ideally you never want to test the private methods
directly only via a public method route.

Actually private method testing is more common. Reason is a small %age of methods are public.
Testing entire functionality black box means more setup and more complex tests. This is possible
only if we have fine grained classes. That is not the case in majority of cases. Hence I resort
to unit testing private methods.

If I had some more time, I would have broken the method in atleast 3 classes. Hence dropping
this comment.


- Ashutosh


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


On May 18, 2017, 11:33 p.m., Ashutosh Mestry wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59378/
> -----------------------------------------------------------
> 
> (Updated May 18, 2017, 11:33 p.m.)
> 
> 
> Review request for atlas, Apoorv Naik, Madhan Neethiraj, and Sarath Subramanian.
> 
> 
> Bugs: ATLAS-1818
>     https://issues.apache.org/jira/browse/ATLAS-1818
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> Background
> ==========
> Please refer to [ATLAS-1818](https://issues.apache.org/jira/browse/ATLAS-1818) for background,
analysis and solution.
> 
> Implementation
> ==============
> 
> * _Solrconfig.xml_ updated.
> * _atlas-application.properties_: New property _atlas.graph.index.search.max-result-set-size_=150.
> * _EntityDiscoveryService.searchUsingBasicQuery_ updated to pass type and classification
as part of search query passed to Solr.
> * Added overload to _AtlasGraph.indexQuery_ to include offset.
> 
> **CURL**
> ```
> curl -s -u 'admin:admin' -H 'Content-type: application/json' "http://localhost:21000/api/atlas/v2/search/basic?typeName=hive_table&query=testtable_*&classification=tag0"

> ```
> 
> References
> ==========
> * _Solr Cookbook 3rd Ed by Rafal Kuc_
> 
> 
> Diffs
> -----
> 
>   distro/src/conf/atlas-application.properties b2b8e745 
>   distro/src/conf/solr/solrconfig.xml ce2e20bb 
>   graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasGraph.java a3a27bfd

>   graphdb/titan0/src/main/java/org/apache/atlas/repository/graphdb/titan0/Titan0Graph.java
9624c99f 
>   graphdb/titan0/src/test/resources/atlas-application.properties 636a9ff3 
>   graphdb/titan1/src/main/java/org/apache/atlas/repository/graphdb/titan1/Titan1Graph.java
6a610755 
>   repository/src/main/java/org/apache/atlas/discovery/EntityDiscoveryService.java 874487cb

>   repository/src/test/java/org/apache/atlas/services/EntityDiscoveryServiceTest.java
PRE-CREATION 
>   typesystem/src/test/resources/atlas-application.properties 5ffde5e5 
> 
> 
> Diff: https://reviews.apache.org/r/59378/diff/3/
> 
> 
> Testing
> -------
> 
> **Unit tests**
> *EntityDiscoveryServiceTest* covers new code added during this exercise.
> 
> **Performance tests**
> Performance testing suite in performance benchmarking environment.
> 
> |-----------------------------|-----------|----------|
> | Test                        | Existing  |  New     |
> |-----------------------------|-----------|----------|
> |basic search:                |           |          |
> |type specified               |           |          |
> |classification not specified |   ~75 secs|    ~1 sec|
> |search string: testtable     |           |          | 
> |-----------------------------|-----------|----------|
> |basic search:                |           |          |
> |type specified               |           |          |
> |classification specified     |   ~75 secs|    ~1 sec|
> |search string: testtable     |           |          | 
> |-----------------------------|-----------|----------|
> 
> **Functional tests**
> Execute the following API:
> ```
> /v2/search/basic?typeName=hive_table&query=testtable_*&classification=tag0" 
> ```
> 
> Page navigation.
> 
> 
> Thanks,
> 
> Ashutosh Mestry
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message