Re: Review Request 72722: Index Consistency: Java Patch Handler Implementation

2020-07-30 Thread Madhan Neethiraj 

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


Ship it!




Ship It!

- Madhan Neethiraj


On July 30, 2020, 10:49 p.m., Ashutosh Mestry wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72722/
> ---
> 
> (Updated July 30, 2020, 10:49 p.m.)
> 
> 
> Review request for atlas, Damian Warszawski, Madhan Neethiraj, Nikhil Bonte, 
> Nixon Rodrigues, and Sarath Subramanian.
> 
> 
> Bugs: ATLAS-3907
> https://issues.apache.org/jira/browse/ATLAS-3907
> 
> 
> Repository: atlas
> 
> 
> Description
> ---
> 
> **Approach**
> Please see JIRA for details.
> 
> Changes:
> - New: *IndexConsistencyPatch*: New java patch handler.
> - Modified: Added method: 
> *AtlasJanusGraphManagement.setConsistencyForAllIndexes* that enumerates all 
> the existing indexes and calls the *setConsistency* method.
> 
> 
> 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 
>   
> repository/src/main/java/org/apache/atlas/repository/patches/AtlasPatchManager.java
>  093edf978 
>   
> repository/src/main/java/org/apache/atlas/repository/patches/IndexConsistencyPatch.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/72722/diff/3/
> 
> 
> Testing
> ---
> 
> **Unit tests**
> None.
> 
> **Functional tests**
> Used the script below to concurrently create same entity. See attached 
> (t.json) that contains entity payload.
> 
> Steps to test:
> 1. Deploy Atlas without this patch (or any other patch) with *setConsistency*.
> 2. Execute the script below with the payload *t1.json*.
> 3. Notice that multiple entities of *hdfs_path* type get created. Few of the 
> REST calls may fail with *AtlasSchemaViolation* exception.
> 4. Deploy this change and restart Atlas. Notice that the new patch gets 
> applied.
> 5. Modify *t1.json* by replacing e1 with e2.
> 6. Execute the script below.
> 7. Notice that only 1 entity *hdfs_path* with name e2 gets created.
> 
> *multiple-curl.sh*
> ```bash
> #!/bin/bash
> URL=
> echo ${URL}
> for i in `seq 10`; do
> curl -u admin:admin123 -X POST http://${URL}:31000/api/atlas/v2/entity/bulk 
> --header 'Content-Type: application/json' -d @./t.json &
> done
> ```
> 
> **Pre-commit**
> https://builds.apache.org/view/A/view/Atlas/job/PreCommit-ATLAS-Build-Test/2074/:
>  Succeeded.
> 
> 
> File Attachments
> 
> 
> entity payload
>   
> https://reviews.apache.org/media/uploaded/files/2020/07/30/62c25f8c-87cb-458d-a49e-ce41a96855de__t.json
> 
> 
> Thanks,
> 
> Ashutosh Mestry
> 
>

Re: Review Request 72722: Index Consistency: Java Patch Handler Implementation

2020-07-30 Thread Ashutosh Mestry via Review Board 

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

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


Review request for atlas, Damian Warszawski, Madhan Neethiraj, Nikhil Bonte, 
Nixon Rodrigues, and Sarath Subramanian.


Changes
---

Updates include: Addressed review comments.


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


Repository: atlas


Description
---

**Approach**
Please see JIRA for details.

Changes:
- New: *IndexConsistencyPatch*: New java patch handler.
- Modified: Added method: 
*AtlasJanusGraphManagement.setConsistencyForAllIndexes* that enumerates all the 
existing indexes and calls the *setConsistency* method.


Diffs (updated)
-

  
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 
  
repository/src/main/java/org/apache/atlas/repository/patches/AtlasPatchManager.java
 093edf978 
  
repository/src/main/java/org/apache/atlas/repository/patches/IndexConsistencyPatch.java
 PRE-CREATION 


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

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


Testing
---

**Unit tests**
None.

**Functional tests**
Used the script below to concurrently create same entity. See attached (t.json) 
that contains entity payload.

Steps to test:
1. Deploy Atlas without this patch (or any other patch) with *setConsistency*.
2. Execute the script below with the payload *t1.json*.
3. Notice that multiple entities of *hdfs_path* type get created. Few of the 
REST calls may fail with *AtlasSchemaViolation* exception.
4. Deploy this change and restart Atlas. Notice that the new patch gets applied.
5. Modify *t1.json* by replacing e1 with e2.
6. Execute the script below.
7. Notice that only 1 entity *hdfs_path* with name e2 gets created.

*multiple-curl.sh*
```bash
#!/bin/bash
URL=
echo ${URL}
for i in `seq 10`; do
curl -u admin:admin123 -X POST http://${URL}:31000/api/atlas/v2/entity/bulk 
--header 'Content-Type: application/json' -d @./t.json &
done
```

**Pre-commit**
https://builds.apache.org/view/A/view/Atlas/job/PreCommit-ATLAS-Build-Test/2074/:
 Succeeded.


File Attachments


entity payload
  
https://reviews.apache.org/media/uploaded/files/2020/07/30/62c25f8c-87cb-458d-a49e-ce41a96855de__t.json


Thanks,

Ashutosh Mestry

Re: Review Request 72722: Index Consistency: Java Patch Handler Implementation

2020-07-30 Thread Madhan Neethiraj 

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


Fix it, then Ship it!





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


Perhaps following condition can be added as well, to skip updates if lock 
is already set (this can happen in fresh installs):
  index.getConsistency() == ConsistencyModifier.LOCK


- Madhan Neethiraj


On July 30, 2020, 8:45 p.m., Ashutosh Mestry wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72722/
> ---
> 
> (Updated July 30, 2020, 8:45 p.m.)
> 
> 
> Review request for atlas, Damian Warszawski, Madhan Neethiraj, Nikhil Bonte, 
> Nixon Rodrigues, and Sarath Subramanian.
> 
> 
> Bugs: ATLAS-3907
> https://issues.apache.org/jira/browse/ATLAS-3907
> 
> 
> Repository: atlas
> 
> 
> Description
> ---
> 
> **Approach**
> Please see JIRA for details.
> 
> Changes:
> - New: *IndexConsistencyPatch*: New java patch handler.
> - Modified: Added method: 
> *AtlasJanusGraphManagement.setConsistencyForAllIndexes* that enumerates all 
> the existing indexes and calls the *setConsistency* method.
> 
> 
> 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 
>   
> repository/src/main/java/org/apache/atlas/repository/patches/AtlasPatchManager.java
>  093edf978 
>   
> repository/src/main/java/org/apache/atlas/repository/patches/IndexConsistencyPatch.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/72722/diff/2/
> 
> 
> Testing
> ---
> 
> **Unit tests**
> None.
> 
> **Functional tests**
> Used the script below to concurrently create same entity. See attached 
> (t.json) that contains entity payload.
> 
> Steps to test:
> 1. Deploy Atlas without this patch (or any other patch) with *setConsistency*.
> 2. Execute the script below with the payload *t1.json*.
> 3. Notice that multiple entities of *hdfs_path* type get created. Few of the 
> REST calls may fail with *AtlasSchemaViolation* exception.
> 4. Deploy this change and restart Atlas. Notice that the new patch gets 
> applied.
> 5. Modify *t1.json* by replacing e1 with e2.
> 6. Execute the script below.
> 7. Notice that only 1 entity *hdfs_path* with name e2 gets created.
> 
> *multiple-curl.sh*
> ```bash
> #!/bin/bash
> URL=
> echo ${URL}
> for i in `seq 10`; do
> curl -u admin:admin123 -X POST http://${URL}:31000/api/atlas/v2/entity/bulk 
> --header 'Content-Type: application/json' -d @./t.json &
> done
> ```
> 
> **Pre-commit**
> https://builds.apache.org/view/A/view/Atlas/job/PreCommit-ATLAS-Build-Test/2074/:
>  Succeeded.
> 
> 
> File Attachments
> 
> 
> entity payload
>   
> https://reviews.apache.org/media/uploaded/files/2020/07/30/62c25f8c-87cb-458d-a49e-ce41a96855de__t.json
> 
> 
> Thanks,
> 
> Ashutosh Mestry
> 
>

Re: Review Request 72722: Index Consistency: Java Patch Handler Implementation

2020-07-30 Thread Ashutosh Mestry via Review Board 

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

(Updated July 30, 2020, 8:45 p.m.)


Review request for atlas, Damian Warszawski, Madhan Neethiraj, Nikhil Bonte, 
Nixon Rodrigues, and Sarath Subramanian.


Changes
---

Updates include: Addressed review comments.


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


Repository: atlas


Description
---

**Approach**
Please see JIRA for details.

Changes:
- New: *IndexConsistencyPatch*: New java patch handler.
- Modified: Added method: 
*AtlasJanusGraphManagement.setConsistencyForAllIndexes* that enumerates all the 
existing indexes and calls the *setConsistency* method.


Diffs (updated)
-

  
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 
  
repository/src/main/java/org/apache/atlas/repository/patches/AtlasPatchManager.java
 093edf978 
  
repository/src/main/java/org/apache/atlas/repository/patches/IndexConsistencyPatch.java
 PRE-CREATION 


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

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


Testing
---

**Unit tests**
None.

**Functional tests**
Used the script below to concurrently create same entity. See attached (t.json) 
that contains entity payload.

Steps to test:
1. Deploy Atlas without this patch (or any other patch) with *setConsistency*.
2. Execute the script below with the payload *t1.json*.
3. Notice that multiple entities of *hdfs_path* type get created. Few of the 
REST calls may fail with *AtlasSchemaViolation* exception.
4. Deploy this change and restart Atlas. Notice that the new patch gets applied.
5. Modify *t1.json* by replacing e1 with e2.
6. Execute the script below.
7. Notice that only 1 entity *hdfs_path* with name e2 gets created.

*multiple-curl.sh*
```bash
#!/bin/bash
URL=
echo ${URL}
for i in `seq 10`; do
curl -u admin:admin123 -X POST http://${URL}:31000/api/atlas/v2/entity/bulk 
--header 'Content-Type: application/json' -d @./t.json &
done
```

**Pre-commit**
https://builds.apache.org/view/A/view/Atlas/job/PreCommit-ATLAS-Build-Test/2074/


File Attachments


entity payload
  
https://reviews.apache.org/media/uploaded/files/2020/07/30/62c25f8c-87cb-458d-a49e-ce41a96855de__t.json


Thanks,

Ashutosh Mestry

Re: Review Request 72722: Index Consistency: Java Patch Handler Implementation

2020-07-30 Thread Madhan Neethiraj 

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




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


consistency is needed only for unique-indexes, right? If yes, I suggest to 
rename setConsistencyForAllIndexes() => updateUniqueIndexesForConsistencyLock()



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


why not use 'this.management'?



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


consider replacing #285, #286 with this.commit().



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


Add "!index.isUnique()" as well.


- Madhan Neethiraj


On July 30, 2020, 5:42 p.m., Ashutosh Mestry wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72722/
> ---
> 
> (Updated July 30, 2020, 5:42 p.m.)
> 
> 
> Review request for atlas, Damian Warszawski, Madhan Neethiraj, Nikhil Bonte, 
> Nixon Rodrigues, and Sarath Subramanian.
> 
> 
> Bugs: ATLAS-3907
> https://issues.apache.org/jira/browse/ATLAS-3907
> 
> 
> Repository: atlas
> 
> 
> Description
> ---
> 
> **Approach**
> Please see JIRA for details.
> 
> Changes:
> - New: *IndexConsistencyPatch*: New java patch handler.
> - Modified: Added method: 
> *AtlasJanusGraphManagement.setConsistencyForAllIndexes* that enumerates all 
> the existing indexes and calls the *setConsistency* method.
> 
> 
> 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 
>   
> repository/src/main/java/org/apache/atlas/repository/patches/AtlasPatchManager.java
>  093edf978 
>   
> repository/src/main/java/org/apache/atlas/repository/patches/IndexConsistencyPatch.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/72722/diff/1/
> 
> 
> Testing
> ---
> 
> **Unit tests**
> None.
> 
> **Functional tests**
> Used the script below to concurrently create same entity. See attached 
> (t.json) that contains entity payload.
> 
> Steps to test:
> 1. Deploy Atlas without this patch (or any other patch) with *setConsistency*.
> 2. Execute the script below with the payload *t1.json*.
> 3. Notice that multiple entities of *hdfs_path* type get created. Few of the 
> REST calls may fail with *AtlasSchemaViolation* exception.
> 4. Deploy this change and restart Atlas. Notice that the new patch gets 
> applied.
> 5. Modify *t1.json* by replacing e1 with e2.
> 6. Execute the script below.
> 7. Notice that only 1 entity *hdfs_path* with name e2 gets created.
> 
> *multiple-curl.sh*
> ```bash
> #!/bin/bash
> URL=
> echo ${URL}
> for i in `seq 10`; do
> curl -u admin:admin123 -X POST http://${URL}:31000/api/atlas/v2/entity/bulk 
> --header 'Content-Type: application/json' -d @./t.json &
> done
> ```
> 
> **Pre-commit**
> https://builds.apache.org/view/A/view/Atlas/job/PreCommit-ATLAS-Build-Test/2074/
> 
> 
> File Attachments
> 
> 
> entity payload
>   
> https://reviews.apache.org/media/uploaded/files/2020/07/30/62c25f8c-87cb-458d-a49e-ce41a96855de__t.json
> 
> 
> Thanks,
> 
> Ashutosh Mestry
> 
>

Re: Review Request 72722: Index Consistency: Java Patch Handler Implementation

2020-07-30 Thread Ashutosh Mestry via Review Board 

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

(Updated July 30, 2020, 5:42 p.m.)


Review request for atlas, Damian Warszawski, Madhan Neethiraj, Nikhil Bonte, 
Nixon Rodrigues, and Sarath Subramanian.


Changes
---

Updates include: Added steps to duplicate the problem and the resolution of the 
problem after the new update.


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


Repository: atlas


Description
---

**Approach**
Please see JIRA for details.

Changes:
- New: *IndexConsistencyPatch*: New java patch handler.
- Modified: Added method: 
*AtlasJanusGraphManagement.setConsistencyForAllIndexes* that enumerates all the 
existing indexes and calls the *setConsistency* method.


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 
  
repository/src/main/java/org/apache/atlas/repository/patches/AtlasPatchManager.java
 093edf978 
  
repository/src/main/java/org/apache/atlas/repository/patches/IndexConsistencyPatch.java
 PRE-CREATION 


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


Testing (updated)
---

**Unit tests**
None.

**Functional tests**
Used the script below to concurrently create same entity. See attached (t.json) 
that contains entity payload.

Steps to test:
1. Deploy Atlas without this patch (or any other patch) with *setConsistency*.
2. Execute the script below with the payload *t1.json*.
3. Notice that multiple entities of *hdfs_path* type get created. Few of the 
REST calls may fail with *AtlasSchemaViolation* exception.
4. Deploy this change and restart Atlas. Notice that the new patch gets applied.
5. Modify *t1.json* by replacing e1 with e2.
6. Execute the script below.
7. Notice that only 1 entity *hdfs_path* with name e2 gets created.

*multiple-curl.sh*
```bash
#!/bin/bash
URL=
echo ${URL}
for i in `seq 10`; do
curl -u admin:admin123 -X POST http://${URL}:31000/api/atlas/v2/entity/bulk 
--header 'Content-Type: application/json' -d @./t.json &
done
```

**Pre-commit**
https://builds.apache.org/view/A/view/Atlas/job/PreCommit-ATLAS-Build-Test/2074/


File Attachments


entity payload
  
https://reviews.apache.org/media/uploaded/files/2020/07/30/62c25f8c-87cb-458d-a49e-ce41a96855de__t.json


Thanks,

Ashutosh Mestry

Review Request 72722: Index Consistency: Java Patch Handler Implementation

2020-07-30 Thread Ashutosh Mestry via Review Board 

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

Review request for atlas, Damian Warszawski, Madhan Neethiraj, Nikhil Bonte, 
Nixon Rodrigues, and Sarath Subramanian.


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


Repository: atlas


Description
---

**Approach**
Please see JIRA for details.

Changes:
- New: *IndexConsistencyPatch*: New java patch handler.
- Modified: Added method: 
*AtlasJanusGraphManagement.setConsistencyForAllIndexes* that enumerates all the 
existing indexes and calls the *setConsistency* method.


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 
  
repository/src/main/java/org/apache/atlas/repository/patches/AtlasPatchManager.java
 093edf978 
  
repository/src/main/java/org/apache/atlas/repository/patches/IndexConsistencyPatch.java
 PRE-CREATION 


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


Testing
---

**Unit tests**
None.

**Functional tests**
Used the script below to concurrently create same entity. See attached (t.json) 
that contains entity payload.

*multiple-curl.sh*
```bash
#!/bin/bash
URL=
echo ${URL}
for i in `seq 10`; do
curl -u admin:admin123 -X POST http://${URL}:31000/api/atlas/v2/entity/bulk 
--header 'Content-Type: application/json' -d @./t.json &
done
```

**Pre-commit**
https://builds.apache.org/view/A/view/Atlas/job/PreCommit-ATLAS-Build-Test/2074/


File Attachments


entity payload
  
https://reviews.apache.org/media/uploaded/files/2020/07/30/62c25f8c-87cb-458d-a49e-ce41a96855de__t.json


Thanks,

Ashutosh Mestry