Re: Review Request 72695: Optional configuration to support locks on JanusGraph to ensure data consitency.

2020-07-30 Thread Sarath Subramanian 

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


Ship it!




Ship It!

- Sarath Subramanian


On July 30, 2020, 3:23 p.m., Damian Warszawski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72695/
> ---
> 
> (Updated July 30, 2020, 3:23 p.m.)
> 
> 
> Review request for atlas, Ashutosh Mestry, Bolke de Bruin, madhan, and Sarath 
> Subramanian.
> 
> 
> Repository: atlas
> 
> 
> Description
> ---
> 
> Optional configuration to support locks on JanusGraph to ensure data 
> consitency.
> 
> JanusGraph is eventually consistent by default which is efficient but results 
> in duplicates when race condition occurs.
> 
> 
> Reference to jira 
> https://issues.apache.org/jira/projects/ATLAS/issues/ATLAS-3398
> 
> 
> Diffs
> -
> 
>   
> graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusGraphManagement.java
>  6ef9cb76c 
>   intg/src/main/java/org/apache/atlas/AtlasConfiguration.java 2c007ca01 
>   
> repository/src/test/java/org/apache/atlas/discovery/FreeTextSearchProcessorTest.java
>  464b281fc 
>   test-tools/src/main/resources/solr/core-template/solrconfig.xml 39cc6ab45 
> 
> 
> Diff: https://reviews.apache.org/r/72695/diff/4/
> 
> 
> Testing
> ---
> 
> Not possible to reproduce the error on local machine. Enable locking on our 
> dev env and have not introduce any regression.
> 
> 
> Thanks,
> 
> Damian Warszawski
> 
>

Re: Review Request 72695: Optional configuration to support locks on JanusGraph to ensure data consitency.

2020-07-30 Thread Damian Warszawski 

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

(Updated July 30, 2020, 10:23 p.m.)


Review request for atlas, Ashutosh Mestry, Bolke de Bruin, madhan, and Sarath 
Subramanian.


Changes
---

rebase with master, fix field mapping for unit tests


Repository: atlas


Description
---

Optional configuration to support locks on JanusGraph to ensure data consitency.

JanusGraph is eventually consistent by default which is efficient but results 
in duplicates when race condition occurs.


Reference to jira 
https://issues.apache.org/jira/projects/ATLAS/issues/ATLAS-3398


Diffs (updated)
-

  
graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusGraphManagement.java
 6ef9cb76c 
  intg/src/main/java/org/apache/atlas/AtlasConfiguration.java 2c007ca01 
  
repository/src/test/java/org/apache/atlas/discovery/FreeTextSearchProcessorTest.java
 464b281fc 
  test-tools/src/main/resources/solr/core-template/solrconfig.xml 39cc6ab45 


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

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


Testing
---

Not possible to reproduce the error on local machine. Enable locking on our dev 
env and have not introduce any regression.


Thanks,

Damian Warszawski

Re: Review Request 72695: Optional configuration to support locks on JanusGraph to ensure data consitency.

2020-07-29 Thread Damian Warszawski 
It appears that by setting consistency to lock, there is different field
mapping in freetext handler. I am working to fix that.

On Wed, 29 Jul 2020 at 22:31, Madhan Neethiraj  wrote:

> Damian,
>
> A number of tests fail with this patch - see below. Can you please run
> 'mvn clean package' and investigate these failures?
>
> Thanks,
> Madhan
>
> Tests run: 4, Failures: 2, Errors: 0, Skipped: 0, Time elapsed: 105.055
> sec <<< FAILURE! - in org.apache.atlas.discovery.FreeTextSearchProcessorTest
> searchByNameSortBy(org.apache.atlas.discovery.FreeTextSearchProcessorTest)
> Time elapsed: 0.068 sec  <<< FAILURE!
> java.lang.AssertionError: expected [3] but found [0]
> at org.testng.Assert.fail(Assert.java:94)
> at org.testng.Assert.failNotEquals(Assert.java:496)
> at org.testng.Assert.assertEquals(Assert.java:125)
> at org.testng.Assert.assertEquals(Assert.java:267)
> at org.testng.Assert.assertEquals(Assert.java:277)
> at
> org.apache.atlas.discovery.FreeTextSearchProcessorTest.searchByNameSortBy(FreeTextSearchProcessorTest.java:91)
>
> searchTablesByName(org.apache.atlas.discovery.FreeTextSearchProcessorTest)
> Time elapsed: 0.01 sec  <<< FAILURE!
> java.lang.AssertionError: expected [3] but found [0]
> at org.testng.Assert.fail(Assert.java:94)
> at org.testng.Assert.failNotEquals(Assert.java:496)
> at org.testng.Assert.assertEquals(Assert.java:12T5e)
> at org.testng.Assert.asssts run: 2, Failures: 0, Errors: 0,
> Skipped: 0, Time elapsed: 99.643 sec - in
> org.apache.atlas.services.MetricsServiceTestertEquals(Assert.java:267)
> at org.testng.Assert.assertEquals(Assert.java:277)
> at
> org.apache.atlas.discovery.FreeTextSearchProcessorTest.searchTablesByName(FreeTextSearchProcessorTest.java:71)
>
> Tests run: 19, Failures: 13, Errors: 0, Skipped: 0, Time elapsed: 107.395
> sec <<< FAILURE! - in org.apache.atlas.discovery.AtlasDiscoveryServiceTest
> query_ALLTag(org.apache.atlas.discovery.AtlasDiscoveryServiceTest)  Time
> elapsed: 0.073 sec  <<< FAILURE!
> java.lang.AssertionError: expected [5] but found [2]
> at org.testng.Assert.fail(Assert.java:94)
> at org.testng.Assert.failNotEquals(Assert.java:496)
> at org.testng.Assert.assertEquals(Assert.java:125)
> at org.testng.Assert.assertEquals(Assert.java:372)
> at org.testng.Assert.assertEquals(Assert.java:382)
> at
> org.apache.atlas.discovery.AtlasDiscoveryServiceTest.query_ALLTag(AtlasDiscoveryServiceTest.java:125)
>
> query_ALLTag_tagFilter(org.apache.atlas.discovery.AtlasDiscoveryServiceTest)
> Time elapsed: 0.044 sec  <<< FAILURE!
> java.lang.AssertionError: expected [4] but found [1]
> at org.testng.Assert.fail(Assert.java:94)
> at org.testng.Assert.failNotEquals(Assert.java:496)
> at org.testng.Assert.assertEquals(Assert.java:125)
> at org.testng.Assert.assertEquals(Assert.java:372)
> at org.testng.Assert.assertEquals(Assert.java:382)
> at
> org.apache.atlas.discovery.AtlasDiscoveryServiceTest.query_ALLTag_tagFilter(AtlasDiscoveryServiceTest.java:140)
>
> query_ALLWildcardTag(org.apache.atlas.discovery.AtlasDiscoveryServiceTest)
> Time elapsed: 0.016 sec  <<< FAILURE!
> java.lang.AssertionError: expected [5] but found [2]
> at org.testng.Assert.fail(Assert.java:94)
> at org.testng.Assert.failNotEquals(Assert.java:496)
> at org.testng.Assert.assertEquals(Assert.java:125)
> at org.testng.Assert.assertEquals(Assert.java:372)
> at org.testng.Assert.assertEquals(Assert.java:382)
> at
> org.apache.atlas.discovery.AtlasDiscoveryServiceTest.query_ALLWildcardTag(AtlasDiscoveryServiceTest.java:165)
>
> query_NOTCLASSIFIEDTag(org.apache.atlas.discovery.AtlasDiscoveryServiceTest)
> Time elapsed: 0.026 sec  <<< FAILURE!
> java.lang.AssertionError: expected [true] but found [false]
> at org.testng.Assert.fail(Assert.java:94)
> at org.testng.Assert.failNotEquals(Assert.java:496)
> at org.testng.Assert.assertTrue(Assert.java:42)
> at org.testng.Assert.assertTrue(Assert.java:52)
> at
> org.apache.atlas.discovery.AtlasDiscoveryServiceTest.query_NOTCLASSIFIEDTag(AtlasDiscoveryServiceTest.java:151)
>
> query_entity(org.apache.atlas.discovery.AtlasDiscoveryServiceTest)  Time
> elapsed: 0.036 sec  <<< FAILURE!
> java.lang.AssertionError: expected [true] but found [false]
> at org.testng.Assert.fail(Assert.java:94)
> at org.testng.Assert.failNotEquals(Assert.java:496)
> at org.testng.Assert.assertTrue(Assert.java:42)
> at org.testng.Assert.assertTrue(Assert.java:52)
> at
> org.apache.atlas.discovery.AtlasDiscoveryServiceTest.query_entity(AtlasDiscoveryServiceTest.java:214)
>
> query_entity_entityFilter(org.apache.atlas.discovery.AtlasDiscoveryServiceTest)
> Time elapsed: 0.029 sec  <<< FAILURE!
> java.lang.AssertionError: expected [true] but found [false]
>

Re: Review Request 72695: Optional configuration to support locks on JanusGraph to ensure data consitency.

2020-07-29 Thread Madhan Neethiraj 
Damian,

A number of tests fail with this patch - see below. Can you please run 'mvn 
clean package' and investigate these failures?

Thanks,
Madhan

Tests run: 4, Failures: 2, Errors: 0, Skipped: 0, Time elapsed: 105.055 sec <<< 
FAILURE! - in org.apache.atlas.discovery.FreeTextSearchProcessorTest
searchByNameSortBy(org.apache.atlas.discovery.FreeTextSearchProcessorTest)  
Time elapsed: 0.068 sec  <<< FAILURE!
java.lang.AssertionError: expected [3] but found [0]
at org.testng.Assert.fail(Assert.java:94)
at org.testng.Assert.failNotEquals(Assert.java:496)
at org.testng.Assert.assertEquals(Assert.java:125)
at org.testng.Assert.assertEquals(Assert.java:267)
at org.testng.Assert.assertEquals(Assert.java:277)
at 
org.apache.atlas.discovery.FreeTextSearchProcessorTest.searchByNameSortBy(FreeTextSearchProcessorTest.java:91)

searchTablesByName(org.apache.atlas.discovery.FreeTextSearchProcessorTest)  
Time elapsed: 0.01 sec  <<< FAILURE!
java.lang.AssertionError: expected [3] but found [0]
at org.testng.Assert.fail(Assert.java:94)
at org.testng.Assert.failNotEquals(Assert.java:496)
at org.testng.Assert.assertEquals(Assert.java:12T5e)
at org.testng.Assert.asssts run: 2, Failures: 0, Errors: 0, Skipped: 0, 
Time elapsed: 99.643 sec - in 
org.apache.atlas.services.MetricsServiceTestertEquals(Assert.java:267)
at org.testng.Assert.assertEquals(Assert.java:277)
at 
org.apache.atlas.discovery.FreeTextSearchProcessorTest.searchTablesByName(FreeTextSearchProcessorTest.java:71)

Tests run: 19, Failures: 13, Errors: 0, Skipped: 0, Time elapsed: 107.395 sec 
<<< FAILURE! - in org.apache.atlas.discovery.AtlasDiscoveryServiceTest
query_ALLTag(org.apache.atlas.discovery.AtlasDiscoveryServiceTest)  Time 
elapsed: 0.073 sec  <<< FAILURE!
java.lang.AssertionError: expected [5] but found [2]
at org.testng.Assert.fail(Assert.java:94)
at org.testng.Assert.failNotEquals(Assert.java:496)
at org.testng.Assert.assertEquals(Assert.java:125)
at org.testng.Assert.assertEquals(Assert.java:372)
at org.testng.Assert.assertEquals(Assert.java:382)
at 
org.apache.atlas.discovery.AtlasDiscoveryServiceTest.query_ALLTag(AtlasDiscoveryServiceTest.java:125)

query_ALLTag_tagFilter(org.apache.atlas.discovery.AtlasDiscoveryServiceTest)  
Time elapsed: 0.044 sec  <<< FAILURE!
java.lang.AssertionError: expected [4] but found [1]
at org.testng.Assert.fail(Assert.java:94)
at org.testng.Assert.failNotEquals(Assert.java:496)
at org.testng.Assert.assertEquals(Assert.java:125)
at org.testng.Assert.assertEquals(Assert.java:372)
at org.testng.Assert.assertEquals(Assert.java:382)
at 
org.apache.atlas.discovery.AtlasDiscoveryServiceTest.query_ALLTag_tagFilter(AtlasDiscoveryServiceTest.java:140)

query_ALLWildcardTag(org.apache.atlas.discovery.AtlasDiscoveryServiceTest)  
Time elapsed: 0.016 sec  <<< FAILURE!
java.lang.AssertionError: expected [5] but found [2]
at org.testng.Assert.fail(Assert.java:94)
at org.testng.Assert.failNotEquals(Assert.java:496)
at org.testng.Assert.assertEquals(Assert.java:125)
at org.testng.Assert.assertEquals(Assert.java:372)
at org.testng.Assert.assertEquals(Assert.java:382)
at 
org.apache.atlas.discovery.AtlasDiscoveryServiceTest.query_ALLWildcardTag(AtlasDiscoveryServiceTest.java:165)

query_NOTCLASSIFIEDTag(org.apache.atlas.discovery.AtlasDiscoveryServiceTest)  
Time elapsed: 0.026 sec  <<< FAILURE!
java.lang.AssertionError: expected [true] but found [false]
at org.testng.Assert.fail(Assert.java:94)
at org.testng.Assert.failNotEquals(Assert.java:496)
at org.testng.Assert.assertTrue(Assert.java:42)
at org.testng.Assert.assertTrue(Assert.java:52)
at 
org.apache.atlas.discovery.AtlasDiscoveryServiceTest.query_NOTCLASSIFIEDTag(AtlasDiscoveryServiceTest.java:151)

query_entity(org.apache.atlas.discovery.AtlasDiscoveryServiceTest)  Time 
elapsed: 0.036 sec  <<< FAILURE!
java.lang.AssertionError: expected [true] but found [false]
at org.testng.Assert.fail(Assert.java:94)
at org.testng.Assert.failNotEquals(Assert.java:496)
at org.testng.Assert.assertTrue(Assert.java:42)
at org.testng.Assert.assertTrue(Assert.java:52)
at 
org.apache.atlas.discovery.AtlasDiscoveryServiceTest.query_entity(AtlasDiscoveryServiceTest.java:214)

query_entity_entityFilter(org.apache.atlas.discovery.AtlasDiscoveryServiceTest) 
 Time elapsed: 0.029 sec  <<< FAILURE!
java.lang.AssertionError: expected [true] but found [false]
at org.testng.Assert.fail(Assert.java:94)
at org.testng.Assert.failNotEquals(Assert.java:496)
at org.testng.Assert.assertTrue(Assert.java:42)
at org.testng.Assert.assertTrue(Assert.java:52)
at

Re: Review Request 72695: Optional configuration to support locks on JanusGraph to ensure data consitency.

2020-07-29 Thread Damian Warszawski 


> On July 28, 2020, 10:55 p.m., Madhan Neethiraj wrote:
> > Ship It!

Can I ask you to merge it? I don't think I have permissions to merge it to 
master.


- Damian


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


On July 25, 2020, 8:53 p.m., Damian Warszawski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72695/
> ---
> 
> (Updated July 25, 2020, 8:53 p.m.)
> 
> 
> Review request for atlas, Ashutosh Mestry, Bolke de Bruin, madhan, and Sarath 
> Subramanian.
> 
> 
> Repository: atlas
> 
> 
> Description
> ---
> 
> Optional configuration to support locks on JanusGraph to ensure data 
> consitency.
> 
> JanusGraph is eventually consistent by default which is efficient but results 
> in duplicates when race condition occurs.
> 
> 
> Reference to jira 
> https://issues.apache.org/jira/projects/ATLAS/issues/ATLAS-3398
> 
> 
> Diffs
> -
> 
>   
> graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusGraphManagement.java
>  6ef9cb76c 
>   intg/src/main/java/org/apache/atlas/AtlasConfiguration.java 2c007ca01 
> 
> 
> Diff: https://reviews.apache.org/r/72695/diff/3/
> 
> 
> Testing
> ---
> 
> Not possible to reproduce the error on local machine. Enable locking on our 
> dev env and have not introduce any regression.
> 
> 
> Thanks,
> 
> Damian Warszawski
> 
>

Re: Review Request 72695: Optional configuration to support locks on JanusGraph to ensure data consitency.

2020-07-28 Thread Madhan Neethiraj 

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


Ship it!




Ship It!

- Madhan Neethiraj


On July 25, 2020, 8:53 p.m., Damian Warszawski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72695/
> ---
> 
> (Updated July 25, 2020, 8:53 p.m.)
> 
> 
> Review request for atlas, Ashutosh Mestry, Bolke de Bruin, madhan, and Sarath 
> Subramanian.
> 
> 
> Repository: atlas
> 
> 
> Description
> ---
> 
> Optional configuration to support locks on JanusGraph to ensure data 
> consitency.
> 
> JanusGraph is eventually consistent by default which is efficient but results 
> in duplicates when race condition occurs.
> 
> 
> Reference to jira 
> https://issues.apache.org/jira/projects/ATLAS/issues/ATLAS-3398
> 
> 
> Diffs
> -
> 
>   
> graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusGraphManagement.java
>  6ef9cb76c 
>   intg/src/main/java/org/apache/atlas/AtlasConfiguration.java 2c007ca01 
> 
> 
> Diff: https://reviews.apache.org/r/72695/diff/3/
> 
> 
> Testing
> ---
> 
> Not possible to reproduce the error on local machine. Enable locking on our 
> dev env and have not introduce any regression.
> 
> 
> Thanks,
> 
> Damian Warszawski
> 
>

Re: Review Request 72695: Optional configuration to support locks on JanusGraph to ensure data consitency.

2020-07-25 Thread Damian Warszawski 

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

(Updated July 25, 2020, 8:53 p.m.)


Review request for atlas, Ashutosh Mestry, Bolke de Bruin, madhan, and Sarath 
Subramanian.


Changes
---

fix condition for uniqueness, rename property corresponding to consistency lock


Repository: atlas


Description
---

Optional configuration to support locks on JanusGraph to ensure data consitency.

JanusGraph is eventually consistent by default which is efficient but results 
in duplicates when race condition occurs.


Reference to jira 
https://issues.apache.org/jira/projects/ATLAS/issues/ATLAS-3398


Diffs (updated)
-

  
graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusGraphManagement.java
 6ef9cb76c 
  intg/src/main/java/org/apache/atlas/AtlasConfiguration.java 2c007ca01 


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

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


Testing
---

Not possible to reproduce the error on local machine. Enable locking on our dev 
env and have not introduce any regression.


Thanks,

Damian Warszawski

Re: Review Request 72695: Optional configuration to support locks on JanusGraph to ensure data consitency.

2020-07-24 Thread Madhan Neethiraj 

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




graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusGraphManagement.java
Lines 50 (patched)


lockEnabled => CONSISTENCY_LOCK_ENABLED



graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusGraphManagement.java
Line 271 (original), 267 (patched)


call to indexBuilder.unique() shouldn't require lockEnabled=true. Please 
retain current if condition:
  "(lockEnabled && isUnique)" => "(isUnique)"



intg/src/main/java/org/apache/atlas/AtlasConfiguration.java
Lines 75 (patched)


STORAGE_LOCK_ENABLED("atlas.graph.storage.lock.enabled") => 
STORAGE_CONSISTENCY_LOCK_ENABLED("atlas.graph.storage.consistency-lock.enabled")


- Madhan Neethiraj


On July 24, 2020, 8:23 a.m., Damian Warszawski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72695/
> ---
> 
> (Updated July 24, 2020, 8:23 a.m.)
> 
> 
> Review request for atlas, Ashutosh Mestry, Bolke de Bruin, madhan, and Sarath 
> Subramanian.
> 
> 
> Repository: atlas
> 
> 
> Description
> ---
> 
> Optional configuration to support locks on JanusGraph to ensure data 
> consitency.
> 
> JanusGraph is eventually consistent by default which is efficient but results 
> in duplicates when race condition occurs.
> 
> 
> Reference to jira 
> https://issues.apache.org/jira/projects/ATLAS/issues/ATLAS-3398
> 
> 
> Diffs
> -
> 
>   
> graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusGraphManagement.java
>  6ef9cb76c 
>   intg/src/main/java/org/apache/atlas/AtlasConfiguration.java 2c007ca01 
> 
> 
> Diff: https://reviews.apache.org/r/72695/diff/2/
> 
> 
> Testing
> ---
> 
> Not possible to reproduce the error on local machine. Enable locking on our 
> dev env and have not introduce any regression.
> 
> 
> Thanks,
> 
> Damian Warszawski
> 
>

Re: Review Request 72695: Optional configuration to support locks on JanusGraph to ensure data consitency.

2020-07-24 Thread Ashutosh Mestry via Review Board 


> On July 23, 2020, 9:28 p.m., Ashutosh Mestry wrote:
> > repository/src/main/java/org/apache/atlas/repository/patches/ConcurrentPatchProcessor.java
> > Line 39 (original), 39 (patched)
> > 
> >
> > We will need to add another *JavaPatch* to update existing data.
> 
> Damian Warszawski wrote:
> Could you elaborate on it? Does the locking itself change the structure 
> of the index?
> We usually do the full-import when index is changed.

I need to experiment with this little more. Not sure what the final 
implementation will be, but the JavaPatch will perhaps run a reindex on the 
entire dataset. This may take a while depending on size of the data.


- Ashutosh


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


On July 24, 2020, 8:23 a.m., Damian Warszawski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72695/
> ---
> 
> (Updated July 24, 2020, 8:23 a.m.)
> 
> 
> Review request for atlas, Ashutosh Mestry, Bolke de Bruin, madhan, and Sarath 
> Subramanian.
> 
> 
> Repository: atlas
> 
> 
> Description
> ---
> 
> Optional configuration to support locks on JanusGraph to ensure data 
> consitency.
> 
> JanusGraph is eventually consistent by default which is efficient but results 
> in duplicates when race condition occurs.
> 
> 
> Reference to jira 
> https://issues.apache.org/jira/projects/ATLAS/issues/ATLAS-3398
> 
> 
> Diffs
> -
> 
>   
> graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusGraphManagement.java
>  6ef9cb76c 
>   intg/src/main/java/org/apache/atlas/AtlasConfiguration.java 2c007ca01 
> 
> 
> Diff: https://reviews.apache.org/r/72695/diff/2/
> 
> 
> Testing
> ---
> 
> Not possible to reproduce the error on local machine. Enable locking on our 
> dev env and have not introduce any regression.
> 
> 
> Thanks,
> 
> Damian Warszawski
> 
>

Re: Review Request 72695: Optional configuration to support locks on JanusGraph to ensure data consitency.

2020-07-24 Thread Ashutosh Mestry via Review Board 


> On July 23, 2020, 4:52 a.m., Ashutosh Mestry wrote:
> > repository/src/main/java/org/apache/atlas/repository/patches/ConcurrentPatchProcessor.java
> > Lines 44 (patched)
> > 
> >
> > I suggest use AtlasConfiguration for this. Also, I think this should be 
> > true by default.
> 
> Damian Warszawski wrote:
> Great simplification. Thanks Ashutosh.
> 
> Damian Warszawski wrote:
> Not sure if that should be true by default as long as not having a proper 
> benchmarks how the performance is degradated.

I ran few tests that I have for making sure that performance does not get 
impacted. My test results did not show that to be a problem.

I test involved creating large number of entities via REST calls. During that 
time, I had the metrics enabled. I did not see any deterioration.


- Ashutosh


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


On July 24, 2020, 8:23 a.m., Damian Warszawski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72695/
> ---
> 
> (Updated July 24, 2020, 8:23 a.m.)
> 
> 
> Review request for atlas, Ashutosh Mestry, Bolke de Bruin, madhan, and Sarath 
> Subramanian.
> 
> 
> Repository: atlas
> 
> 
> Description
> ---
> 
> Optional configuration to support locks on JanusGraph to ensure data 
> consitency.
> 
> JanusGraph is eventually consistent by default which is efficient but results 
> in duplicates when race condition occurs.
> 
> 
> Reference to jira 
> https://issues.apache.org/jira/projects/ATLAS/issues/ATLAS-3398
> 
> 
> Diffs
> -
> 
>   
> graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusGraphManagement.java
>  6ef9cb76c 
>   intg/src/main/java/org/apache/atlas/AtlasConfiguration.java 2c007ca01 
> 
> 
> Diff: https://reviews.apache.org/r/72695/diff/2/
> 
> 
> Testing
> ---
> 
> Not possible to reproduce the error on local machine. Enable locking on our 
> dev env and have not introduce any regression.
> 
> 
> Thanks,
> 
> Damian Warszawski
> 
>

Re: Review Request 72695: Optional configuration to support locks on JanusGraph to ensure data consitency.

2020-07-24 Thread Damian Warszawski 


> On July 23, 2020, 4:52 a.m., Ashutosh Mestry wrote:
> > repository/src/main/java/org/apache/atlas/repository/patches/ConcurrentPatchProcessor.java
> > Lines 44 (patched)
> > 
> >
> > I suggest use AtlasConfiguration for this. Also, I think this should be 
> > true by default.
> 
> Damian Warszawski wrote:
> Great simplification. Thanks Ashutosh.

Not sure if that should be true by default as long as not having a proper 
benchmarks how the performance is degradated.


- Damian


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


On July 24, 2020, 8:23 a.m., Damian Warszawski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72695/
> ---
> 
> (Updated July 24, 2020, 8:23 a.m.)
> 
> 
> Review request for atlas, Ashutosh Mestry, Bolke de Bruin, madhan, and Sarath 
> Subramanian.
> 
> 
> Repository: atlas
> 
> 
> Description
> ---
> 
> Optional configuration to support locks on JanusGraph to ensure data 
> consitency.
> 
> JanusGraph is eventually consistent by default which is efficient but results 
> in duplicates when race condition occurs.
> 
> 
> Reference to jira 
> https://issues.apache.org/jira/projects/ATLAS/issues/ATLAS-3398
> 
> 
> Diffs
> -
> 
>   
> graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusGraphManagement.java
>  6ef9cb76c 
>   intg/src/main/java/org/apache/atlas/AtlasConfiguration.java 2c007ca01 
> 
> 
> Diff: https://reviews.apache.org/r/72695/diff/2/
> 
> 
> Testing
> ---
> 
> Not possible to reproduce the error on local machine. Enable locking on our 
> dev env and have not introduce any regression.
> 
> 
> Thanks,
> 
> Damian Warszawski
> 
>

Re: Review Request 72695: Optional configuration to support locks on JanusGraph to ensure data consitency.

2020-07-24 Thread Damian Warszawski 


> On July 23, 2020, 9:28 p.m., Ashutosh Mestry wrote:
> > repository/src/main/java/org/apache/atlas/repository/patches/ConcurrentPatchProcessor.java
> > Line 39 (original), 39 (patched)
> > 
> >
> > We will need to add another *JavaPatch* to update existing data.

Could you elaborate on it? Does the locking itself change the structure of the 
index?
We usually do the full-import when index is changed.


- Damian


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


On July 24, 2020, 8:23 a.m., Damian Warszawski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72695/
> ---
> 
> (Updated July 24, 2020, 8:23 a.m.)
> 
> 
> Review request for atlas, Ashutosh Mestry, Bolke de Bruin, madhan, and Sarath 
> Subramanian.
> 
> 
> Repository: atlas
> 
> 
> Description
> ---
> 
> Optional configuration to support locks on JanusGraph to ensure data 
> consitency.
> 
> JanusGraph is eventually consistent by default which is efficient but results 
> in duplicates when race condition occurs.
> 
> 
> Reference to jira 
> https://issues.apache.org/jira/projects/ATLAS/issues/ATLAS-3398
> 
> 
> Diffs
> -
> 
>   
> graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusGraphManagement.java
>  6ef9cb76c 
>   intg/src/main/java/org/apache/atlas/AtlasConfiguration.java 2c007ca01 
> 
> 
> Diff: https://reviews.apache.org/r/72695/diff/2/
> 
> 
> Testing
> ---
> 
> Not possible to reproduce the error on local machine. Enable locking on our 
> dev env and have not introduce any regression.
> 
> 
> Thanks,
> 
> Damian Warszawski
> 
>

Re: Review Request 72695: Optional configuration to support locks on JanusGraph to ensure data consitency.

2020-07-24 Thread Damian Warszawski 


> On July 23, 2020, 4:52 a.m., Ashutosh Mestry wrote:
> > repository/src/main/java/org/apache/atlas/repository/patches/ConcurrentPatchProcessor.java
> > Lines 44 (patched)
> > 
> >
> > I suggest use AtlasConfiguration for this. Also, I think this should be 
> > true by default.

Great simplification. Thanks Ashutosh.


- Damian


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


On July 24, 2020, 8:23 a.m., Damian Warszawski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72695/
> ---
> 
> (Updated July 24, 2020, 8:23 a.m.)
> 
> 
> Review request for atlas, Ashutosh Mestry, Bolke de Bruin, madhan, and Sarath 
> Subramanian.
> 
> 
> Repository: atlas
> 
> 
> Description
> ---
> 
> Optional configuration to support locks on JanusGraph to ensure data 
> consitency.
> 
> JanusGraph is eventually consistent by default which is efficient but results 
> in duplicates when race condition occurs.
> 
> 
> Reference to jira 
> https://issues.apache.org/jira/projects/ATLAS/issues/ATLAS-3398
> 
> 
> Diffs
> -
> 
>   
> graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusGraphManagement.java
>  6ef9cb76c 
>   intg/src/main/java/org/apache/atlas/AtlasConfiguration.java 2c007ca01 
> 
> 
> Diff: https://reviews.apache.org/r/72695/diff/2/
> 
> 
> Testing
> ---
> 
> Not possible to reproduce the error on local machine. Enable locking on our 
> dev env and have not introduce any regression.
> 
> 
> Thanks,
> 
> Damian Warszawski
> 
>

Re: Review Request 72695: Optional configuration to support locks on JanusGraph to ensure data consitency.

2020-07-24 Thread Damian Warszawski 


> On July 21, 2020, 9:06 p.m., Madhan Neethiraj wrote:
> > graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusGraphManagement.java
> > Lines 260 (patched)
> > 
> >
> > Consistency lock might be relevant/needed only for unique-index i.e. 
> > isUnique=true. If this true, consider calling setConsistency(LOCK) when 
> > isUnique is true, without requiring additional argument. Same applies for 
> > #278 as well.
> > 
> > It will be useful support following configuration, to optionally 
> > disable consistentcy-lock:
> >   atlas.graph.storage.unique-key.consitency-lock.enabled
> > 
> > This configuration can be sent to AtlasJanusGraphManagement during 
> > construction - from AtlasJanusGraph.getManagementSystem().
> > 
> > Above will avoid updates to many methods for the addition of 
> > lockEnabled argument.

Great suggestion. Changed accordingly. Thanks.


- Damian


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


On July 24, 2020, 8:23 a.m., Damian Warszawski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72695/
> ---
> 
> (Updated July 24, 2020, 8:23 a.m.)
> 
> 
> Review request for atlas, Ashutosh Mestry, Bolke de Bruin, madhan, and Sarath 
> Subramanian.
> 
> 
> Repository: atlas
> 
> 
> Description
> ---
> 
> Optional configuration to support locks on JanusGraph to ensure data 
> consitency.
> 
> JanusGraph is eventually consistent by default which is efficient but results 
> in duplicates when race condition occurs.
> 
> 
> Reference to jira 
> https://issues.apache.org/jira/projects/ATLAS/issues/ATLAS-3398
> 
> 
> Diffs
> -
> 
>   
> graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusGraphManagement.java
>  6ef9cb76c 
>   intg/src/main/java/org/apache/atlas/AtlasConfiguration.java 2c007ca01 
> 
> 
> Diff: https://reviews.apache.org/r/72695/diff/2/
> 
> 
> Testing
> ---
> 
> Not possible to reproduce the error on local machine. Enable locking on our 
> dev env and have not introduce any regression.
> 
> 
> Thanks,
> 
> Damian Warszawski
> 
>

Re: Review Request 72695: Optional configuration to support locks on JanusGraph to ensure data consitency.

2020-07-24 Thread Damian Warszawski 

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

(Updated July 24, 2020, 8:23 a.m.)


Review request for atlas, Ashutosh Mestry, Bolke de Bruin, madhan, and Sarath 
Subramanian.


Changes
---

move locking conf to AtlasConfiguration


Repository: atlas


Description
---

Optional configuration to support locks on JanusGraph to ensure data consitency.

JanusGraph is eventually consistent by default which is efficient but results 
in duplicates when race condition occurs.


Reference to jira 
https://issues.apache.org/jira/projects/ATLAS/issues/ATLAS-3398


Diffs (updated)
-

  
graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusGraphManagement.java
 6ef9cb76c 
  intg/src/main/java/org/apache/atlas/AtlasConfiguration.java 2c007ca01 


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

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


Testing
---

Not possible to reproduce the error on local machine. Enable locking on our dev 
env and have not introduce any regression.


Thanks,

Damian Warszawski

Re: Review Request 72695: Optional configuration to support locks on JanusGraph to ensure data consitency.

2020-07-23 Thread Ashutosh Mestry via Review Board 

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




repository/src/main/java/org/apache/atlas/repository/patches/ConcurrentPatchProcessor.java
Line 39 (original), 39 (patched)


We will need to add another *JavaPatch* to update existing data.


- Ashutosh Mestry


On July 20, 2020, 9:48 p.m., Damian Warszawski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72695/
> ---
> 
> (Updated July 20, 2020, 9:48 p.m.)
> 
> 
> Review request for atlas, Ashutosh Mestry, Bolke de Bruin, madhan, and Sarath 
> Subramanian.
> 
> 
> Repository: atlas
> 
> 
> Description
> ---
> 
> Optional configuration to support locks on JanusGraph to ensure data 
> consitency.
> 
> JanusGraph is eventually consistent by default which is efficient but results 
> in duplicates when race condition occurs.
> 
> 
> Reference to jira 
> https://issues.apache.org/jira/projects/ATLAS/issues/ATLAS-3398
> 
> 
> Diffs
> -
> 
>   
> graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasGraphManagement.java
>  fca789027 
>   
> graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusGraphManagement.java
>  6ef9cb76c 
>   
> graphdb/janus/src/test/java/org/apache/atlas/repository/graphdb/janus/AbstractGraphDatabaseTest.java
>  35004157f 
>   
> graphdb/janus/src/test/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusDatabaseTest.java
>  5cd55093e 
>   intg/src/main/java/org/apache/atlas/ApplicationProperties.java e662c8fae 
>   
> repository/src/main/java/org/apache/atlas/repository/graph/GraphBackedSearchIndexer.java
>  e35f3594f 
>   
> repository/src/main/java/org/apache/atlas/repository/patches/ConcurrentPatchProcessor.java
>  5a9ac2abe 
>   
> repository/src/main/java/org/apache/atlas/repository/patches/UniqueAttributePatch.java
>  d3111f110 
> 
> 
> Diff: https://reviews.apache.org/r/72695/diff/1/
> 
> 
> Testing
> ---
> 
> Not possible to reproduce the error on local machine. Enable locking on our 
> dev env and have not introduce any regression.
> 
> 
> Thanks,
> 
> Damian Warszawski
> 
>

Re: Review Request 72695: Optional configuration to support locks on JanusGraph to ensure data consitency.

2020-07-22 Thread Ashutosh Mestry via Review Board 

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




repository/src/main/java/org/apache/atlas/repository/patches/ConcurrentPatchProcessor.java
Lines 44 (patched)


I suggest use AtlasConfiguration for this. Also, I think this should be 
true by default.


- Ashutosh Mestry


On July 20, 2020, 9:48 p.m., Damian Warszawski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72695/
> ---
> 
> (Updated July 20, 2020, 9:48 p.m.)
> 
> 
> Review request for atlas, Ashutosh Mestry, Bolke de Bruin, madhan, and Sarath 
> Subramanian.
> 
> 
> Repository: atlas
> 
> 
> Description
> ---
> 
> Optional configuration to support locks on JanusGraph to ensure data 
> consitency.
> 
> JanusGraph is eventually consistent by default which is efficient but results 
> in duplicates when race condition occurs.
> 
> 
> Reference to jira 
> https://issues.apache.org/jira/projects/ATLAS/issues/ATLAS-3398
> 
> 
> Diffs
> -
> 
>   
> graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasGraphManagement.java
>  fca789027 
>   
> graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusGraphManagement.java
>  6ef9cb76c 
>   
> graphdb/janus/src/test/java/org/apache/atlas/repository/graphdb/janus/AbstractGraphDatabaseTest.java
>  35004157f 
>   
> graphdb/janus/src/test/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusDatabaseTest.java
>  5cd55093e 
>   intg/src/main/java/org/apache/atlas/ApplicationProperties.java e662c8fae 
>   
> repository/src/main/java/org/apache/atlas/repository/graph/GraphBackedSearchIndexer.java
>  e35f3594f 
>   
> repository/src/main/java/org/apache/atlas/repository/patches/ConcurrentPatchProcessor.java
>  5a9ac2abe 
>   
> repository/src/main/java/org/apache/atlas/repository/patches/UniqueAttributePatch.java
>  d3111f110 
> 
> 
> Diff: https://reviews.apache.org/r/72695/diff/1/
> 
> 
> Testing
> ---
> 
> Not possible to reproduce the error on local machine. Enable locking on our 
> dev env and have not introduce any regression.
> 
> 
> Thanks,
> 
> Damian Warszawski
> 
>

Re: Review Request 72695: Optional configuration to support locks on JanusGraph to ensure data consitency.

2020-07-22 Thread Ashutosh Mestry via Review Board 


> On July 21, 2020, 8:38 p.m., Ashutosh Mestry wrote:
> > Here's what I have tried so far:
> > - Concurrent entity creation using my own test rig. This creates entities 
> > concurrently but it does not allow for entity with same qualifiedName to be 
> > created by different workers.
> > - Ingest via Kafka queue. This is to verify that there is not performance 
> > degradation. 
> > 
> > So far results are positive. I did not obseve any performance or impact on 
> > accuracy.
> > 
> > Yet to try:
> > - Migration from 0.8 to latest.
> > - Migration import using ZipDirect format.
> 
> Madhan Neethiraj wrote:
> Ashutosh - what is the behavior seen in case of simultaneous attempts to 
> entity an entity? Do all but 1 attempt fail?

Please see my comment below.


- Ashutosh


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


On July 20, 2020, 9:48 p.m., Damian Warszawski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72695/
> ---
> 
> (Updated July 20, 2020, 9:48 p.m.)
> 
> 
> Review request for atlas, Ashutosh Mestry, Bolke de Bruin, madhan, and Sarath 
> Subramanian.
> 
> 
> Repository: atlas
> 
> 
> Description
> ---
> 
> Optional configuration to support locks on JanusGraph to ensure data 
> consitency.
> 
> JanusGraph is eventually consistent by default which is efficient but results 
> in duplicates when race condition occurs.
> 
> 
> Reference to jira 
> https://issues.apache.org/jira/projects/ATLAS/issues/ATLAS-3398
> 
> 
> Diffs
> -
> 
>   
> graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasGraphManagement.java
>  fca789027 
>   
> graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusGraphManagement.java
>  6ef9cb76c 
>   
> graphdb/janus/src/test/java/org/apache/atlas/repository/graphdb/janus/AbstractGraphDatabaseTest.java
>  35004157f 
>   
> graphdb/janus/src/test/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusDatabaseTest.java
>  5cd55093e 
>   intg/src/main/java/org/apache/atlas/ApplicationProperties.java e662c8fae 
>   
> repository/src/main/java/org/apache/atlas/repository/graph/GraphBackedSearchIndexer.java
>  e35f3594f 
>   
> repository/src/main/java/org/apache/atlas/repository/patches/ConcurrentPatchProcessor.java
>  5a9ac2abe 
>   
> repository/src/main/java/org/apache/atlas/repository/patches/UniqueAttributePatch.java
>  d3111f110 
> 
> 
> Diff: https://reviews.apache.org/r/72695/diff/1/
> 
> 
> Testing
> ---
> 
> Not possible to reproduce the error on local machine. Enable locking on our 
> dev env and have not introduce any regression.
> 
> 
> Thanks,
> 
> Damian Warszawski
> 
>

Re: Review Request 72695: Optional configuration to support locks on JanusGraph to ensure data consitency.

2020-07-22 Thread Ashutosh Mestry via Review Board 


> On July 22, 2020, 4:30 p.m., Ashutosh Mestry wrote:
> > I attempted to create same entity (entity with same qualifiedName) from 
> > multiple requests.
> > 
> > I did this experiment:
> > - Created an entity payload.
> > - Used bulk entity CURL calls from 5 different requests. 
> > 
> > Repeated this exercise on database with:
> > - Existing data: I was able to create duplicate entities.
> > - Blank database: Only 1 entity with the same qualifiedName was created.
> > 
> > My thinking is that we will see real value of this if we have a Java patch 
> > handler that fixes existing data.

Additionally: The zip-file-based migration import proceeded without any 
performance deterioration.


- Ashutosh


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


On July 20, 2020, 9:48 p.m., Damian Warszawski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72695/
> ---
> 
> (Updated July 20, 2020, 9:48 p.m.)
> 
> 
> Review request for atlas, Ashutosh Mestry, Bolke de Bruin, madhan, and Sarath 
> Subramanian.
> 
> 
> Repository: atlas
> 
> 
> Description
> ---
> 
> Optional configuration to support locks on JanusGraph to ensure data 
> consitency.
> 
> JanusGraph is eventually consistent by default which is efficient but results 
> in duplicates when race condition occurs.
> 
> 
> Reference to jira 
> https://issues.apache.org/jira/projects/ATLAS/issues/ATLAS-3398
> 
> 
> Diffs
> -
> 
>   
> graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasGraphManagement.java
>  fca789027 
>   
> graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusGraphManagement.java
>  6ef9cb76c 
>   
> graphdb/janus/src/test/java/org/apache/atlas/repository/graphdb/janus/AbstractGraphDatabaseTest.java
>  35004157f 
>   
> graphdb/janus/src/test/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusDatabaseTest.java
>  5cd55093e 
>   intg/src/main/java/org/apache/atlas/ApplicationProperties.java e662c8fae 
>   
> repository/src/main/java/org/apache/atlas/repository/graph/GraphBackedSearchIndexer.java
>  e35f3594f 
>   
> repository/src/main/java/org/apache/atlas/repository/patches/ConcurrentPatchProcessor.java
>  5a9ac2abe 
>   
> repository/src/main/java/org/apache/atlas/repository/patches/UniqueAttributePatch.java
>  d3111f110 
> 
> 
> Diff: https://reviews.apache.org/r/72695/diff/1/
> 
> 
> Testing
> ---
> 
> Not possible to reproduce the error on local machine. Enable locking on our 
> dev env and have not introduce any regression.
> 
> 
> Thanks,
> 
> Damian Warszawski
> 
>

Re: Review Request 72695: Optional configuration to support locks on JanusGraph to ensure data consitency.

2020-07-22 Thread Ashutosh Mestry via Review Board 

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



I attempted to create same entity (entity with same qualifiedName) from 
multiple requests.

I did this experiment:
- Created an entity payload.
- Used bulk entity CURL calls from 5 different requests. 

Repeated this exercise on database with:
- Existing data: I was able to create duplicate entities.
- Blank database: Only 1 entity with the same qualifiedName was created.

My thinking is that we will see real value of this if we have a Java patch 
handler that fixes existing data.

- Ashutosh Mestry


On July 20, 2020, 9:48 p.m., Damian Warszawski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72695/
> ---
> 
> (Updated July 20, 2020, 9:48 p.m.)
> 
> 
> Review request for atlas, Ashutosh Mestry, Bolke de Bruin, madhan, and Sarath 
> Subramanian.
> 
> 
> Repository: atlas
> 
> 
> Description
> ---
> 
> Optional configuration to support locks on JanusGraph to ensure data 
> consitency.
> 
> JanusGraph is eventually consistent by default which is efficient but results 
> in duplicates when race condition occurs.
> 
> 
> Reference to jira 
> https://issues.apache.org/jira/projects/ATLAS/issues/ATLAS-3398
> 
> 
> Diffs
> -
> 
>   
> graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasGraphManagement.java
>  fca789027 
>   
> graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusGraphManagement.java
>  6ef9cb76c 
>   
> graphdb/janus/src/test/java/org/apache/atlas/repository/graphdb/janus/AbstractGraphDatabaseTest.java
>  35004157f 
>   
> graphdb/janus/src/test/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusDatabaseTest.java
>  5cd55093e 
>   intg/src/main/java/org/apache/atlas/ApplicationProperties.java e662c8fae 
>   
> repository/src/main/java/org/apache/atlas/repository/graph/GraphBackedSearchIndexer.java
>  e35f3594f 
>   
> repository/src/main/java/org/apache/atlas/repository/patches/ConcurrentPatchProcessor.java
>  5a9ac2abe 
>   
> repository/src/main/java/org/apache/atlas/repository/patches/UniqueAttributePatch.java
>  d3111f110 
> 
> 
> Diff: https://reviews.apache.org/r/72695/diff/1/
> 
> 
> Testing
> ---
> 
> Not possible to reproduce the error on local machine. Enable locking on our 
> dev env and have not introduce any regression.
> 
> 
> Thanks,
> 
> Damian Warszawski
> 
>

Re: Review Request 72695: Optional configuration to support locks on JanusGraph to ensure data consitency.

2020-07-21 Thread Madhan Neethiraj 


> On July 21, 2020, 8:38 p.m., Ashutosh Mestry wrote:
> > Here's what I have tried so far:
> > - Concurrent entity creation using my own test rig. This creates entities 
> > concurrently but it does not allow for entity with same qualifiedName to be 
> > created by different workers.
> > - Ingest via Kafka queue. This is to verify that there is not performance 
> > degradation. 
> > 
> > So far results are positive. I did not obseve any performance or impact on 
> > accuracy.
> > 
> > Yet to try:
> > - Migration from 0.8 to latest.
> > - Migration import using ZipDirect format.

Ashutosh - what is the behavior seen in case of simultaneous attempts to entity 
an entity? Do all but 1 attempt fail?


- Madhan


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


On July 20, 2020, 9:48 p.m., Damian Warszawski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72695/
> ---
> 
> (Updated July 20, 2020, 9:48 p.m.)
> 
> 
> Review request for atlas, Ashutosh Mestry, Bolke de Bruin, madhan, and Sarath 
> Subramanian.
> 
> 
> Repository: atlas
> 
> 
> Description
> ---
> 
> Optional configuration to support locks on JanusGraph to ensure data 
> consitency.
> 
> JanusGraph is eventually consistent by default which is efficient but results 
> in duplicates when race condition occurs.
> 
> 
> Reference to jira 
> https://issues.apache.org/jira/projects/ATLAS/issues/ATLAS-3398
> 
> 
> Diffs
> -
> 
>   
> graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasGraphManagement.java
>  fca789027 
>   
> graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusGraphManagement.java
>  6ef9cb76c 
>   
> graphdb/janus/src/test/java/org/apache/atlas/repository/graphdb/janus/AbstractGraphDatabaseTest.java
>  35004157f 
>   
> graphdb/janus/src/test/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusDatabaseTest.java
>  5cd55093e 
>   intg/src/main/java/org/apache/atlas/ApplicationProperties.java e662c8fae 
>   
> repository/src/main/java/org/apache/atlas/repository/graph/GraphBackedSearchIndexer.java
>  e35f3594f 
>   
> repository/src/main/java/org/apache/atlas/repository/patches/ConcurrentPatchProcessor.java
>  5a9ac2abe 
>   
> repository/src/main/java/org/apache/atlas/repository/patches/UniqueAttributePatch.java
>  d3111f110 
> 
> 
> Diff: https://reviews.apache.org/r/72695/diff/1/
> 
> 
> Testing
> ---
> 
> Not possible to reproduce the error on local machine. Enable locking on our 
> dev env and have not introduce any regression.
> 
> 
> Thanks,
> 
> Damian Warszawski
> 
>

Re: Review Request 72695: Optional configuration to support locks on JanusGraph to ensure data consitency.

2020-07-21 Thread Madhan Neethiraj 

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




graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusGraphManagement.java
Lines 260 (patched)


Consistency lock might be relevant/needed only for unique-index i.e. 
isUnique=true. If this true, consider calling setConsistency(LOCK) when 
isUnique is true, without requiring additional argument. Same applies for #278 
as well.

It will be useful support following configuration, to optionally disable 
consistentcy-lock:
  atlas.graph.storage.unique-key.consitency-lock.enabled

This configuration can be sent to AtlasJanusGraphManagement during 
construction - from AtlasJanusGraph.getManagementSystem().

Above will avoid updates to many methods for the addition of lockEnabled 
argument.


- Madhan Neethiraj


On July 20, 2020, 9:48 p.m., Damian Warszawski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72695/
> ---
> 
> (Updated July 20, 2020, 9:48 p.m.)
> 
> 
> Review request for atlas, Ashutosh Mestry, Bolke de Bruin, madhan, and Sarath 
> Subramanian.
> 
> 
> Repository: atlas
> 
> 
> Description
> ---
> 
> Optional configuration to support locks on JanusGraph to ensure data 
> consitency.
> 
> JanusGraph is eventually consistent by default which is efficient but results 
> in duplicates when race condition occurs.
> 
> 
> Reference to jira 
> https://issues.apache.org/jira/projects/ATLAS/issues/ATLAS-3398
> 
> 
> Diffs
> -
> 
>   
> graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasGraphManagement.java
>  fca789027 
>   
> graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusGraphManagement.java
>  6ef9cb76c 
>   
> graphdb/janus/src/test/java/org/apache/atlas/repository/graphdb/janus/AbstractGraphDatabaseTest.java
>  35004157f 
>   
> graphdb/janus/src/test/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusDatabaseTest.java
>  5cd55093e 
>   intg/src/main/java/org/apache/atlas/ApplicationProperties.java e662c8fae 
>   
> repository/src/main/java/org/apache/atlas/repository/graph/GraphBackedSearchIndexer.java
>  e35f3594f 
>   
> repository/src/main/java/org/apache/atlas/repository/patches/ConcurrentPatchProcessor.java
>  5a9ac2abe 
>   
> repository/src/main/java/org/apache/atlas/repository/patches/UniqueAttributePatch.java
>  d3111f110 
> 
> 
> Diff: https://reviews.apache.org/r/72695/diff/1/
> 
> 
> Testing
> ---
> 
> Not possible to reproduce the error on local machine. Enable locking on our 
> dev env and have not introduce any regression.
> 
> 
> Thanks,
> 
> Damian Warszawski
> 
>

Re: Review Request 72695: Optional configuration to support locks on JanusGraph to ensure data consitency.

2020-07-21 Thread Ashutosh Mestry via Review Board 

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



Here's what I have tried so far:
- Concurrent entity creation using my own test rig. This creates entities 
concurrently but it does not allow for entity with same qualifiedName to be 
created by different workers.
- Ingest via Kafka queue. This is to verify that there is not performance 
degradation. 

So far results are positive. I did not obseve any performance or impact on 
accuracy.

Yet to try:
- Migration from 0.8 to latest.
- Migration import using ZipDirect format.

- Ashutosh Mestry


On July 20, 2020, 9:48 p.m., Damian Warszawski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72695/
> ---
> 
> (Updated July 20, 2020, 9:48 p.m.)
> 
> 
> Review request for atlas, Ashutosh Mestry, Bolke de Bruin, madhan, and Sarath 
> Subramanian.
> 
> 
> Repository: atlas
> 
> 
> Description
> ---
> 
> Optional configuration to support locks on JanusGraph to ensure data 
> consitency.
> 
> JanusGraph is eventually consistent by default which is efficient but results 
> in duplicates when race condition occurs.
> 
> 
> Reference to jira 
> https://issues.apache.org/jira/projects/ATLAS/issues/ATLAS-3398
> 
> 
> Diffs
> -
> 
>   
> graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasGraphManagement.java
>  fca789027 
>   
> graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusGraphManagement.java
>  6ef9cb76c 
>   
> graphdb/janus/src/test/java/org/apache/atlas/repository/graphdb/janus/AbstractGraphDatabaseTest.java
>  35004157f 
>   
> graphdb/janus/src/test/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusDatabaseTest.java
>  5cd55093e 
>   intg/src/main/java/org/apache/atlas/ApplicationProperties.java e662c8fae 
>   
> repository/src/main/java/org/apache/atlas/repository/graph/GraphBackedSearchIndexer.java
>  e35f3594f 
>   
> repository/src/main/java/org/apache/atlas/repository/patches/ConcurrentPatchProcessor.java
>  5a9ac2abe 
>   
> repository/src/main/java/org/apache/atlas/repository/patches/UniqueAttributePatch.java
>  d3111f110 
> 
> 
> Diff: https://reviews.apache.org/r/72695/diff/1/
> 
> 
> Testing
> ---
> 
> Not possible to reproduce the error on local machine. Enable locking on our 
> dev env and have not introduce any regression.
> 
> 
> Thanks,
> 
> Damian Warszawski
> 
>

Review Request 72695: Optional configuration to support locks on JanusGraph to ensure data consitency.

2020-07-20 Thread Damian Warszawski 

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

Review request for atlas, Ashutosh Mestry, Bolke de Bruin, madhan, and Sarath 
Subramanian.


Repository: atlas


Description
---

Optional configuration to support locks on JanusGraph to ensure data consitency.

JanusGraph is eventually consistent by default which is efficient but results 
in duplicates when race condition occurs.


Reference to jira 
https://issues.apache.org/jira/projects/ATLAS/issues/ATLAS-3398


Diffs
-

  
graphdb/api/src/main/java/org/apache/atlas/repository/graphdb/AtlasGraphManagement.java
 fca789027 
  
graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusGraphManagement.java
 6ef9cb76c 
  
graphdb/janus/src/test/java/org/apache/atlas/repository/graphdb/janus/AbstractGraphDatabaseTest.java
 35004157f 
  
graphdb/janus/src/test/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusDatabaseTest.java
 5cd55093e 
  intg/src/main/java/org/apache/atlas/ApplicationProperties.java e662c8fae 
  
repository/src/main/java/org/apache/atlas/repository/graph/GraphBackedSearchIndexer.java
 e35f3594f 
  
repository/src/main/java/org/apache/atlas/repository/patches/ConcurrentPatchProcessor.java
 5a9ac2abe 
  
repository/src/main/java/org/apache/atlas/repository/patches/UniqueAttributePatch.java
 d3111f110 


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


Testing
---

Not possible to reproduce the error on local machine. Enable locking on our dev 
env and have not introduce any regression.


Thanks,

Damian Warszawski