You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@atlas.apache.org by Ashutosh Mestry via Review Board <no...@reviews.apache.org> on 2020/07/30 17:00:35 UTC
Review Request 72722: Index Consistency: Java Patch Handler
Implementation
-----------------------------------------------------------
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=<server>
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
Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
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=<server>
> 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
Posted by Ashutosh Mestry via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
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=<server>
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
Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
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)
<https://reviews.apache.org/r/72722/#comment310425>
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=<server>
> 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
Posted by Ashutosh Mestry via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
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=<server>
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
Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
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)
<https://reviews.apache.org/r/72722/#comment310406>
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)
<https://reviews.apache.org/r/72722/#comment310407>
why not use 'this.management'?
graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusGraphManagement.java
Lines 286 (patched)
<https://reviews.apache.org/r/72722/#comment310408>
consider replacing #285, #286 with this.commit().
graphdb/janus/src/main/java/org/apache/atlas/repository/graphdb/janus/AtlasJanusGraphManagement.java
Lines 298 (patched)
<https://reviews.apache.org/r/72722/#comment310409>
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=<server>
> 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
Posted by Ashutosh Mestry via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
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=<server>
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