----------------------------------------------------------- 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 > >
