Re: Review Request 62455: ATLAS-2092: Upfront edgelabel creation to avoid concurrency issues

2017-09-25 Thread David Radley 

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




repository/src/main/java/org/apache/atlas/repository/graph/GraphBackedSearchIndexer.java
Lines 485 (patched)


I am not sure what "while type creation" means here.



repository/src/main/java/org/apache/atlas/repository/graph/GraphBackedSearchIndexer.java
Lines 486 (patched)


I suggest we extend the comment to say "because "



repository/src/test/java/org/apache/atlas/service/DefaultMetadataServiceTest.java
Line 167 (original), 177 (patched)


I assume the reason for this cange is that thee was a clash of randomly 
generated strings. I sugest not using randum string generators as part of the 
units tests- as this makes the unit tests non-deterministic. If a particular 
string format fails but hardly ever comes up them we will not spot it. If we 
want unique strings - it would be best to incorporate a counter we could rely 
to produce unque strings.



repository/src/test/java/org/apache/atlas/service/DefaultMetadataServiceTest.java
Lines 1241 (patched)


I suggest removing this Jira comments - as it implies there might still be 
an issue here.



repository/src/test/java/org/apache/atlas/service/DefaultMetadataServiceTest.java
Lines 1246 (patched)


I suggest we test V2 API code paths as well as V1 API code paths.


- David Radley


On Sept. 22, 2017, 9:46 p.m., Apoorv Naik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62455/
> ---
> 
> (Updated Sept. 22, 2017, 9:46 p.m.)
> 
> 
> Review request for atlas and Madhan Neethiraj.
> 
> 
> Bugs: ATLAS-2092
> https://issues.apache.org/jira/browse/ATLAS-2092
> 
> 
> Repository: atlas
> 
> 
> Description
> ---
> 
> See ATLAS-2092 for the discussion
> 
> 
> Diffs
> -
> 
>   
> graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasEdgeLabel.java
>  PRE-CREATION 
>   
> graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasGraphManagement.java
>  0db46d46 
>   
> graphdb/titan0/src/main/java/org/apache/atlas/repository/graphdb/titan0/GraphDbObjectFactory.java
>  0e8519e7 
>   
> graphdb/titan0/src/main/java/org/apache/atlas/repository/graphdb/titan0/Titan0EdgeLabel.java
>  PRE-CREATION 
>   
> graphdb/titan0/src/main/java/org/apache/atlas/repository/graphdb/titan0/Titan0GraphManagement.java
>  ec4d6c45 
>   
> graphdb/titan1/src/main/java/org/apache/atlas/repository/graphdb/titan1/GraphDbObjectFactory.java
>  c27bc35e 
>   
> graphdb/titan1/src/main/java/org/apache/atlas/repository/graphdb/titan1/Titan1EdgeLabel.java
>  PRE-CREATION 
>   
> graphdb/titan1/src/main/java/org/apache/atlas/repository/graphdb/titan1/Titan1GraphManagement.java
>  61dc2980 
>   
> repository/src/main/java/org/apache/atlas/repository/graph/GraphBackedSearchIndexer.java
>  be6a0a06 
>   repository/src/test/java/org/apache/atlas/TestUtils.java 1e439f7d 
>   
> repository/src/test/java/org/apache/atlas/discovery/DataSetLineageServiceTest.java
>  c700a61f 
>   
> repository/src/test/java/org/apache/atlas/lineage/EntityLineageServiceTest.java
>  8a88f322 
>   
> repository/src/test/java/org/apache/atlas/repository/audit/AuditRepositoryTestBase.java
>  5bde6059 
>   
> repository/src/test/java/org/apache/atlas/repository/graph/GraphBackedMetadataRepositoryTest.java
>  f372891e 
>   
> repository/src/test/java/org/apache/atlas/repository/graph/GraphBackedSearchIndexerTest.java
>  ef5d4696 
>   
> repository/src/test/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityStoreV1Test.java
>  9b4cbd36 
>   
> repository/src/test/java/org/apache/atlas/service/DefaultMetadataServiceTest.java
>  d8d8d91b 
>   repository/src/test/scala/org/apache/atlas/query/QueryTestsUtils.scala 
> c844558a 
> 
> 
> Diff: https://reviews.apache.org/r/62455/diff/4/
> 
> 
> Testing
> ---
> 
> mvn clean package -Dberkeley-elasticsearch went through fine
> Build was done 3-4 times to ensure that the concurrency tests passed every 
> time
> 
> 
> Thanks,
> 
> Apoorv Naik
> 
>

Re: Review Request 62455: ATLAS-2092: Upfront edgelabel creation to avoid concurrency issues

2017-09-22 Thread Madhan Neethiraj 

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


Ship it!




Ship It!

- Madhan Neethiraj


On Sept. 22, 2017, 9:46 p.m., Apoorv Naik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62455/
> ---
> 
> (Updated Sept. 22, 2017, 9:46 p.m.)
> 
> 
> Review request for atlas and Madhan Neethiraj.
> 
> 
> Bugs: ATLAS-2092
> https://issues.apache.org/jira/browse/ATLAS-2092
> 
> 
> Repository: atlas
> 
> 
> Description
> ---
> 
> See ATLAS-2092 for the discussion
> 
> 
> Diffs
> -
> 
>   
> graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasEdgeLabel.java
>  PRE-CREATION 
>   
> graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasGraphManagement.java
>  0db46d46 
>   
> graphdb/titan0/src/main/java/org/apache/atlas/repository/graphdb/titan0/GraphDbObjectFactory.java
>  0e8519e7 
>   
> graphdb/titan0/src/main/java/org/apache/atlas/repository/graphdb/titan0/Titan0EdgeLabel.java
>  PRE-CREATION 
>   
> graphdb/titan0/src/main/java/org/apache/atlas/repository/graphdb/titan0/Titan0GraphManagement.java
>  ec4d6c45 
>   
> graphdb/titan1/src/main/java/org/apache/atlas/repository/graphdb/titan1/GraphDbObjectFactory.java
>  c27bc35e 
>   
> graphdb/titan1/src/main/java/org/apache/atlas/repository/graphdb/titan1/Titan1EdgeLabel.java
>  PRE-CREATION 
>   
> graphdb/titan1/src/main/java/org/apache/atlas/repository/graphdb/titan1/Titan1GraphManagement.java
>  61dc2980 
>   
> repository/src/main/java/org/apache/atlas/repository/graph/GraphBackedSearchIndexer.java
>  be6a0a06 
>   repository/src/test/java/org/apache/atlas/TestUtils.java 1e439f7d 
>   
> repository/src/test/java/org/apache/atlas/discovery/DataSetLineageServiceTest.java
>  c700a61f 
>   
> repository/src/test/java/org/apache/atlas/lineage/EntityLineageServiceTest.java
>  8a88f322 
>   
> repository/src/test/java/org/apache/atlas/repository/audit/AuditRepositoryTestBase.java
>  5bde6059 
>   
> repository/src/test/java/org/apache/atlas/repository/graph/GraphBackedMetadataRepositoryTest.java
>  f372891e 
>   
> repository/src/test/java/org/apache/atlas/repository/graph/GraphBackedSearchIndexerTest.java
>  ef5d4696 
>   
> repository/src/test/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityStoreV1Test.java
>  9b4cbd36 
>   
> repository/src/test/java/org/apache/atlas/service/DefaultMetadataServiceTest.java
>  d8d8d91b 
>   repository/src/test/scala/org/apache/atlas/query/QueryTestsUtils.scala 
> c844558a 
> 
> 
> Diff: https://reviews.apache.org/r/62455/diff/4/
> 
> 
> Testing
> ---
> 
> mvn clean package -Dberkeley-elasticsearch went through fine
> Build was done 3-4 times to ensure that the concurrency tests passed every 
> time
> 
> 
> Thanks,
> 
> Apoorv Naik
> 
>

Re: Review Request 62455: ATLAS-2092: Upfront edgelabel creation to avoid concurrency issues

2017-09-22 Thread Apoorv Naik 

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

(Updated Sept. 22, 2017, 9:46 p.m.)


Review request for atlas and Madhan Neethiraj.


Changes
---

Added abstraction around EdgeLabel and modified randomString generation in tests


Bugs: ATLAS-2092
https://issues.apache.org/jira/browse/ATLAS-2092


Repository: atlas


Description
---

See ATLAS-2092 for the discussion


Diffs (updated)
-

  
graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasEdgeLabel.java
 PRE-CREATION 
  
graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasGraphManagement.java
 0db46d46 
  
graphdb/titan0/src/main/java/org/apache/atlas/repository/graphdb/titan0/GraphDbObjectFactory.java
 0e8519e7 
  
graphdb/titan0/src/main/java/org/apache/atlas/repository/graphdb/titan0/Titan0EdgeLabel.java
 PRE-CREATION 
  
graphdb/titan0/src/main/java/org/apache/atlas/repository/graphdb/titan0/Titan0GraphManagement.java
 ec4d6c45 
  
graphdb/titan1/src/main/java/org/apache/atlas/repository/graphdb/titan1/GraphDbObjectFactory.java
 c27bc35e 
  
graphdb/titan1/src/main/java/org/apache/atlas/repository/graphdb/titan1/Titan1EdgeLabel.java
 PRE-CREATION 
  
graphdb/titan1/src/main/java/org/apache/atlas/repository/graphdb/titan1/Titan1GraphManagement.java
 61dc2980 
  
repository/src/main/java/org/apache/atlas/repository/graph/GraphBackedSearchIndexer.java
 be6a0a06 
  repository/src/test/java/org/apache/atlas/TestUtils.java 1e439f7d 
  
repository/src/test/java/org/apache/atlas/discovery/DataSetLineageServiceTest.java
 c700a61f 
  
repository/src/test/java/org/apache/atlas/lineage/EntityLineageServiceTest.java 
8a88f322 
  
repository/src/test/java/org/apache/atlas/repository/audit/AuditRepositoryTestBase.java
 5bde6059 
  
repository/src/test/java/org/apache/atlas/repository/graph/GraphBackedMetadataRepositoryTest.java
 f372891e 
  
repository/src/test/java/org/apache/atlas/repository/graph/GraphBackedSearchIndexerTest.java
 ef5d4696 
  
repository/src/test/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityStoreV1Test.java
 9b4cbd36 
  
repository/src/test/java/org/apache/atlas/service/DefaultMetadataServiceTest.java
 d8d8d91b 
  repository/src/test/scala/org/apache/atlas/query/QueryTestsUtils.scala 
c844558a 


Diff: https://reviews.apache.org/r/62455/diff/4/

Changes: https://reviews.apache.org/r/62455/diff/3-4/


Testing
---

mvn clean package -Dberkeley-elasticsearch went through fine
Build was done 3-4 times to ensure that the concurrency tests passed every time


Thanks,

Apoorv Naik

Re: Review Request 62455: ATLAS-2092: Upfront edgelabel creation to avoid concurrency issues

2017-09-22 Thread Madhan Neethiraj 

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


Ship it!




Ship It!

- Madhan Neethiraj


On Sept. 22, 2017, 3:44 p.m., Apoorv Naik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62455/
> ---
> 
> (Updated Sept. 22, 2017, 3:44 p.m.)
> 
> 
> Review request for atlas and Madhan Neethiraj.
> 
> 
> Bugs: ATLAS-2092
> https://issues.apache.org/jira/browse/ATLAS-2092
> 
> 
> Repository: atlas
> 
> 
> Description
> ---
> 
> See ATLAS-2092 for the discussion
> 
> 
> Diffs
> -
> 
>   
> graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasGraphManagement.java
>  0db46d46 
>   
> graphdb/titan0/src/main/java/org/apache/atlas/repository/graphdb/titan0/Titan0GraphManagement.java
>  ec4d6c45 
>   
> graphdb/titan1/src/main/java/org/apache/atlas/repository/graphdb/titan1/Titan1GraphManagement.java
>  61dc2980 
>   
> repository/src/main/java/org/apache/atlas/repository/graph/GraphBackedSearchIndexer.java
>  be6a0a06 
>   
> repository/src/test/java/org/apache/atlas/service/DefaultMetadataServiceTest.java
>  d8d8d91b 
> 
> 
> Diff: https://reviews.apache.org/r/62455/diff/3/
> 
> 
> Testing
> ---
> 
> mvn clean package -Dberkeley-elasticsearch went through fine
> Build was done 3-4 times to ensure that the concurrency tests passed every 
> time
> 
> 
> Thanks,
> 
> Apoorv Naik
> 
>

Re: Review Request 62455: ATLAS-2092: Upfront edgelabel creation to avoid concurrency issues

2017-09-22 Thread Apoorv Naik 

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

(Updated Sept. 22, 2017, 3:44 p.m.)


Review request for atlas and Madhan Neethiraj.


Changes
---

Fixed parameter passing and removed duplicate comments.


Bugs: ATLAS-2092
https://issues.apache.org/jira/browse/ATLAS-2092


Repository: atlas


Description
---

See ATLAS-2092 for the discussion


Diffs (updated)
-

  
graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasGraphManagement.java
 0db46d46 
  
graphdb/titan0/src/main/java/org/apache/atlas/repository/graphdb/titan0/Titan0GraphManagement.java
 ec4d6c45 
  
graphdb/titan1/src/main/java/org/apache/atlas/repository/graphdb/titan1/Titan1GraphManagement.java
 61dc2980 
  
repository/src/main/java/org/apache/atlas/repository/graph/GraphBackedSearchIndexer.java
 be6a0a06 
  
repository/src/test/java/org/apache/atlas/service/DefaultMetadataServiceTest.java
 d8d8d91b 


Diff: https://reviews.apache.org/r/62455/diff/3/

Changes: https://reviews.apache.org/r/62455/diff/2-3/


Testing
---

mvn clean package -Dberkeley-elasticsearch went through fine
Build was done 3-4 times to ensure that the concurrency tests passed every time


Thanks,

Apoorv Naik

Re: Review Request 62455: ATLAS-2092: Upfront edgelabel creation to avoid concurrency issues

2017-09-21 Thread Apoorv Naik 

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

(Updated Sept. 22, 2017, 3:04 a.m.)


Review request for atlas and Madhan Neethiraj.


Bugs: ATLAS-2092
https://issues.apache.org/jira/browse/ATLAS-2092


Repository: atlas


Description
---

See ATLAS-2092 for the discussion


Diffs (updated)
-

  
graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasGraphManagement.java
 0db46d46 
  
graphdb/titan0/src/main/java/org/apache/atlas/repository/graphdb/titan0/Titan0GraphManagement.java
 ec4d6c45 
  
graphdb/titan1/src/main/java/org/apache/atlas/repository/graphdb/titan1/Titan1GraphManagement.java
 61dc2980 
  
repository/src/main/java/org/apache/atlas/repository/graph/GraphBackedSearchIndexer.java
 be6a0a06 
  
repository/src/test/java/org/apache/atlas/service/DefaultMetadataServiceTest.java
 d8d8d91b 


Diff: https://reviews.apache.org/r/62455/diff/2/

Changes: https://reviews.apache.org/r/62455/diff/1-2/


Testing
---

mvn clean package -Dberkeley-elasticsearch went through fine
Build was done 3-4 times to ensure that the concurrency tests passed every time


Thanks,

Apoorv Naik

Re: Review Request 62455: ATLAS-2092: Upfront edgelabel creation to avoid concurrency issues

2017-09-20 Thread Apoorv Naik 

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



Will update the code per the review comments

- Apoorv Naik


On Sept. 20, 2017, 9:55 p.m., Apoorv Naik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62455/
> ---
> 
> (Updated Sept. 20, 2017, 9:55 p.m.)
> 
> 
> Review request for atlas and Madhan Neethiraj.
> 
> 
> Bugs: ATLAS-2092
> https://issues.apache.org/jira/browse/ATLAS-2092
> 
> 
> Repository: atlas
> 
> 
> Description
> ---
> 
> See ATLAS-2092 for the discussion
> 
> 
> Diffs
> -
> 
>   
> graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasGraphManagement.java
>  0db46d46 
>   
> graphdb/titan0/src/main/java/org/apache/atlas/repository/graphdb/titan0/Titan0GraphManagement.java
>  ec4d6c45 
>   
> graphdb/titan1/src/main/java/org/apache/atlas/repository/graphdb/titan1/Titan1GraphManagement.java
>  61dc2980 
>   
> repository/src/main/java/org/apache/atlas/repository/graph/GraphBackedSearchIndexer.java
>  be6a0a06 
> 
> 
> Diff: https://reviews.apache.org/r/62455/diff/1/
> 
> 
> Testing
> ---
> 
> mvn clean package -Dberkeley-elasticsearch went through fine
> Build was done 3-4 times to ensure that the concurrency tests passed every 
> time
> 
> 
> Thanks,
> 
> Apoorv Naik
> 
>

Re: Review Request 62455: ATLAS-2092: Upfront edgelabel creation to avoid concurrency issues

2017-09-20 Thread Sarath Subramanian 

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




graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasGraphManagement.java
Lines 69 (patched)


The test to validate this 
GraphBackedMetadataRepositoryTest.testConcurrentCalls() is tweaked to work 
because of this issue. This needs to change => line#180 assertTrue(validated1 
&& validated2);


- Sarath Subramanian


On Sept. 20, 2017, 2:55 p.m., Apoorv Naik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62455/
> ---
> 
> (Updated Sept. 20, 2017, 2:55 p.m.)
> 
> 
> Review request for atlas and Madhan Neethiraj.
> 
> 
> Bugs: ATLAS-2092
> https://issues.apache.org/jira/browse/ATLAS-2092
> 
> 
> Repository: atlas
> 
> 
> Description
> ---
> 
> See ATLAS-2092 for the discussion
> 
> 
> Diffs
> -
> 
>   
> graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasGraphManagement.java
>  0db46d46 
>   
> graphdb/titan0/src/main/java/org/apache/atlas/repository/graphdb/titan0/Titan0GraphManagement.java
>  ec4d6c45 
>   
> graphdb/titan1/src/main/java/org/apache/atlas/repository/graphdb/titan1/Titan1GraphManagement.java
>  61dc2980 
>   
> repository/src/main/java/org/apache/atlas/repository/graph/GraphBackedSearchIndexer.java
>  be6a0a06 
> 
> 
> Diff: https://reviews.apache.org/r/62455/diff/1/
> 
> 
> Testing
> ---
> 
> mvn clean package -Dberkeley-elasticsearch went through fine
> Build was done 3-4 times to ensure that the concurrency tests passed every 
> time
> 
> 
> Thanks,
> 
> Apoorv Naik
> 
>

Re: Review Request 62455: ATLAS-2092: Upfront edgelabel creation to avoid concurrency issues

2017-09-20 Thread Madhan Neethiraj 

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




repository/src/main/java/org/apache/atlas/repository/graph/GraphBackedSearchIndexer.java
Line 445 (original), 445 (patched)


Edge label should be created for arrays whose element is of type CLASS or 
STRUCT.



repository/src/main/java/org/apache/atlas/repository/graph/GraphBackedSearchIndexer.java
Line 451 (original), 451 (patched)


Edge label should be created for struct type attributes as well.


- Madhan Neethiraj


On Sept. 20, 2017, 9:55 p.m., Apoorv Naik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62455/
> ---
> 
> (Updated Sept. 20, 2017, 9:55 p.m.)
> 
> 
> Review request for atlas and Madhan Neethiraj.
> 
> 
> Bugs: ATLAS-2092
> https://issues.apache.org/jira/browse/ATLAS-2092
> 
> 
> Repository: atlas
> 
> 
> Description
> ---
> 
> See ATLAS-2092 for the discussion
> 
> 
> Diffs
> -
> 
>   
> graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasGraphManagement.java
>  0db46d46 
>   
> graphdb/titan0/src/main/java/org/apache/atlas/repository/graphdb/titan0/Titan0GraphManagement.java
>  ec4d6c45 
>   
> graphdb/titan1/src/main/java/org/apache/atlas/repository/graphdb/titan1/Titan1GraphManagement.java
>  61dc2980 
>   
> repository/src/main/java/org/apache/atlas/repository/graph/GraphBackedSearchIndexer.java
>  be6a0a06 
> 
> 
> Diff: https://reviews.apache.org/r/62455/diff/1/
> 
> 
> Testing
> ---
> 
> mvn clean package -Dberkeley-elasticsearch went through fine
> Build was done 3-4 times to ensure that the concurrency tests passed every 
> time
> 
> 
> Thanks,
> 
> Apoorv Naik
> 
>

Review Request 62455: ATLAS-2092: Upfront edgelabel creation to avoid concurrency issues

2017-09-20 Thread Apoorv Naik 

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

Review request for atlas and Madhan Neethiraj.


Bugs: ATLAS-2092
https://issues.apache.org/jira/browse/ATLAS-2092


Repository: atlas


Description
---

See ATLAS-2092 for the discussion


Diffs
-

  
graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasGraphManagement.java
 0db46d46 
  
graphdb/titan0/src/main/java/org/apache/atlas/repository/graphdb/titan0/Titan0GraphManagement.java
 ec4d6c45 
  
graphdb/titan1/src/main/java/org/apache/atlas/repository/graphdb/titan1/Titan1GraphManagement.java
 61dc2980 
  
repository/src/main/java/org/apache/atlas/repository/graph/GraphBackedSearchIndexer.java
 be6a0a06 


Diff: https://reviews.apache.org/r/62455/diff/1/


Testing
---

mvn clean package -Dberkeley-elasticsearch went through fine
Build was done 3-4 times to ensure that the concurrency tests passed every time


Thanks,

Apoorv Naik