atlas-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Apoorv Naik <naik.apo...@gmail.com>
Subject Re: Review Request 59378: Basic Search Performance Improvement
Date Thu, 18 May 2017 23:42:55 GMT

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




distro/src/conf/atlas-application.properties
Lines 66 (patched)
<https://reviews.apache.org/r/59378/#comment248884>

    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.



distro/src/conf/solr/solrconfig.xml
Line 38 (original), 39 (patched)
<https://reviews.apache.org/r/59378/#comment248882>

    Are you sure this is safe ? I remember there was some issue when using berkeley and elasticsearch
with Lucene 5.5.1



repository/src/main/java/org/apache/atlas/discovery/EntityDiscoveryService.java
Line 74 (original), 78 (patched)
<https://reviews.apache.org/r/59378/#comment248886>

    * for only 3 imports ? 
     
    Revert this change.



repository/src/main/java/org/apache/atlas/discovery/EntityDiscoveryService.java
Lines 97 (patched)
<https://reviews.apache.org/r/59378/#comment248883>

    Can you add a default value in case the property is missing ? Also would be better to
inject the Configuration object instead of ApplicationProperties.get()



repository/src/main/java/org/apache/atlas/discovery/EntityDiscoveryService.java
Lines 265 (patched)
<https://reviews.apache.org/r/59378/#comment248888>

    No need of String.format here, instead pull the expression out and replace %s with {}.



repository/src/main/java/org/apache/atlas/discovery/EntityDiscoveryService.java
Lines 413 (patched)
<https://reviews.apache.org/r/59378/#comment248889>

    Can subtypes be null here ?



repository/src/test/java/org/apache/atlas/services/EntityDiscoveryServiceTest.java
Lines 102 (patched)
<https://reviews.apache.org/r/59378/#comment248894>

    Let's not use Whitebox in UT, ideally you never want to test the private methods directly
only via a public method route.


- Apoorv Naik


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