You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@atlas.apache.org by Apoorv Naik <na...@gmail.com> on 2017/09/20 21:55:25 UTC
Review Request 62455: ATLAS-2092: Upfront edgelabel creation to avoid
concurrency issues
-----------------------------------------------------------
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
Re: Review Request 62455: ATLAS-2092: Upfront edgelabel creation to
avoid concurrency issues
Posted by Sarath Subramanian <sa...@apache.org>.
-----------------------------------------------------------
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)
<https://reviews.apache.org/r/62455/#comment262188>
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
Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
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
Posted by David Radley <da...@apache.org>.
-----------------------------------------------------------
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)
<https://reviews.apache.org/r/62455/#comment262536>
I am not sure what "while type creation" means here.
repository/src/main/java/org/apache/atlas/repository/graph/GraphBackedSearchIndexer.java
Lines 486 (patched)
<https://reviews.apache.org/r/62455/#comment262537>
I suggest we extend the comment to say "because ...."
repository/src/test/java/org/apache/atlas/service/DefaultMetadataServiceTest.java
Line 167 (original), 177 (patched)
<https://reviews.apache.org/r/62455/#comment262533>
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)
<https://reviews.apache.org/r/62455/#comment262534>
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)
<https://reviews.apache.org/r/62455/#comment262535>
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
Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
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
Posted by Apoorv Naik <na...@gmail.com>.
-----------------------------------------------------------
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
Posted by Apoorv Naik <na...@gmail.com>.
-----------------------------------------------------------
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
Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62455/#review185943
-----------------------------------------------------------
repository/src/main/java/org/apache/atlas/repository/graph/GraphBackedSearchIndexer.java
Lines 475 (patched)
<https://reviews.apache.org/r/62455/#comment262283>
field.name? This method expects attribute's data-type name. Please review.
- Madhan Neethiraj
On Sept. 22, 2017, 3:04 a.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: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
> -----
>
> 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/
>
>
> 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
Posted by Apoorv Naik <na...@gmail.com>.
-----------------------------------------------------------
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
Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
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)
<https://reviews.apache.org/r/62455/#comment262185>
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)
<https://reviews.apache.org/r/62455/#comment262186>
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
>
>
Re: Review Request 62455: ATLAS-2092: Upfront edgelabel creation to
avoid concurrency issues
Posted by Apoorv Naik <na...@gmail.com>.
-----------------------------------------------------------
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
Posted by Apoorv Naik <na...@gmail.com>.
> On Sept. 21, 2017, 3:19 p.m., David Radley wrote:
> > graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasGraphManagement.java
> > Line 61 (original), 61 (patched)
> > <https://reviews.apache.org/r/62455/diff/1/?file=1831803#file1831803line61>
> >
> > I think GraphBackedMetadataRepositoryTest needs to be changed lines 178. The logic is
> >
> > boolean validated1 = assertEdge(id1, type.typeName);
> > boolean validated2 = assertEdge(id2, type.typeName);
> > assertTrue(validated1 | validated2);
> >
> > It should be:
> > assertTrue(validated1 && validated2);
Yeah Sarath mentioned the same previously.
> On Sept. 21, 2017, 3:19 p.m., David Radley wrote:
> > repository/src/main/java/org/apache/atlas/repository/graph/GraphBackedSearchIndexer.java
> > Line 460 (original), 460 (patched)
> > <https://reviews.apache.org/r/62455/diff/1/?file=1831806#file1831806line460>
> >
> > this comment is incorrect asis.
> > I am unsure why this comment has been left in. I suggest leaving in the 3 comments or removing all three lines.
Will clean up the comments
> On Sept. 21, 2017, 3:19 p.m., David Radley wrote:
> > repository/src/main/java/org/apache/atlas/repository/graph/GraphBackedSearchIndexer.java
> > Line 461 (original), 461 (patched)
> > <https://reviews.apache.org/r/62455/diff/1/?file=1831806#file1831806line461>
> >
> > I think this comment needs to explain how this solves the concurrancy problem and what the transactional implications are of this fix.
Will add more details.
- Apoorv
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62455/#review185892
-----------------------------------------------------------
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
Posted by David Radley <da...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62455/#review185892
-----------------------------------------------------------
graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasGraphManagement.java
Line 61 (original), 61 (patched)
<https://reviews.apache.org/r/62455/#comment262228>
I think GraphBackedMetadataRepositoryTest needs to be changed lines 178. The logic is
boolean validated1 = assertEdge(id1, type.typeName);
boolean validated2 = assertEdge(id2, type.typeName);
assertTrue(validated1 | validated2);
It should be:
assertTrue(validated1 && validated2);
graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasGraphManagement.java
Line 62 (original), 62 (patched)
<https://reviews.apache.org/r/62455/#comment262229>
I am concerned with the testing. I would suggest that testing is performed by scaffolding the code for testing by adding sleeps reliably get the error - then apply the fix to show it is reliably working. We would not check this scaffold code in.
repository/src/main/java/org/apache/atlas/repository/graph/GraphBackedSearchIndexer.java
Line 460 (original), 460 (patched)
<https://reviews.apache.org/r/62455/#comment262230>
this comment is incorrect asis.
I am unsure why this comment has been left in. I suggest leaving in the 3 comments or removing all three lines.
repository/src/main/java/org/apache/atlas/repository/graph/GraphBackedSearchIndexer.java
Line 461 (original), 461 (patched)
<https://reviews.apache.org/r/62455/#comment262227>
I think this comment needs to explain how this solves the concurrancy problem and what the transactional implications are of this fix.
- David Radley
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
>
>