You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@atlas.apache.org by Graham Wallis <gr...@uk.ibm.com> on 2018/11/30 16:36:04 UTC
Review Request 69494: ATLAS-2985 fixes for delete handlers and
relationship store
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69494/
-----------------------------------------------------------
Review request for atlas.
Repository: atlas
Description
-------
ATLAS-2985: critical fixes for delete handlers and relationship dtore
Diffs
-----
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/DeleteHandlerV1.java c57e30ab5cdb34aee1cb13c463b1b0af827d2a2e
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/HardDeleteHandlerV1.java edf1eed4c3efdd41f77155e51c8f2908b3dbbab2
repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasRelationshipStoreV2.java 86cc98cda61d85954fd227501d691e3a3b542611
Diff: https://reviews.apache.org/r/69494/diff/1/
Testing
-------
Tested with full run of Egeria CTS suite.
Thanks,
Graham Wallis
Re: Review Request 69494: ATLAS-2985 fixes for delete handlers and
relationship store
Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69494/#review210977
-----------------------------------------------------------
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/DeleteHandlerV1.java
Lines 106 (patched)
<https://reviews.apache.org/r/69494/#comment295807>
Couple of comments:
1. If the entity was already deleted in this request (i.e. requestContext.isDeletedEntity(guid) is true), further delete attempts should be skipped. 'if' condition here should be modified as:
if ((softDelete && state == DELETED) || requestContext.isDeletedEntity(guid)) {
// skip
continue;
}
2. Please make sure that deleting an already-deleted entity doesn't generate an entity-delete event. Else applications processing the events might end up with incorrect state - by deleting another active entity with the same qualifiedName (if exists); applications are likely to rely on matching an entity with qualifiedName, instead of guids.
Please review and update.
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/DeleteHandlerV1.java
Lines 151 (patched)
<https://reviews.apache.org/r/69494/#comment295810>
Consider rearranging the condition as below, for better readability:
if (!isInternal && (softDelete && getState(edge) == DELETED))
Also, comment at line #106 about skipping events applies here as well.
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/DeleteHandlerV1.java
Lines 185 (patched)
<https://reviews.apache.org/r/69494/#comment295812>
Please review comments at line #106 for the change here as well.
- Madhan Neethiraj
On Nov. 30, 2018, 4:36 p.m., Graham Wallis wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69494/
> -----------------------------------------------------------
>
> (Updated Nov. 30, 2018, 4:36 p.m.)
>
>
> Review request for atlas.
>
>
> Repository: atlas
>
>
> Description
> -------
>
> ATLAS-2985: critical fixes for delete handlers and relationship dtore
>
>
> Diffs
> -----
>
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/DeleteHandlerV1.java c57e30ab5cdb34aee1cb13c463b1b0af827d2a2e
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/HardDeleteHandlerV1.java edf1eed4c3efdd41f77155e51c8f2908b3dbbab2
> repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasRelationshipStoreV2.java 86cc98cda61d85954fd227501d691e3a3b542611
>
>
> Diff: https://reviews.apache.org/r/69494/diff/1/
>
>
> Testing
> -------
>
> Tested with full run of Egeria CTS suite.
>
>
> Thanks,
>
> Graham Wallis
>
>
Re: Review Request 69494: ATLAS-2985 fixes for delete handlers and
relationship store
Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69494/#review211058
-----------------------------------------------------------
> did not change event generation considerations as Atlas already produces multiple notifications per delete
Every entity delete must result in a single entity-delete notification. If you see multiple entity-delete notifications being sent, please add details. That needs to be fixed; else applications that process the notifications can end up with incorrect state.
> a notification is needed on a soft delete and on a hard delete.
I am not certain of this need. Splitting current entity-delete event into entity-soft-delete and entity-hard-delete will be a breaking change for existing applications (like Ranger TagSync) that depend on current event notifications.
> I would suggest that event duplication is addressed in a separate JIRA
To avoid breaking existing event consumers, it is critcal that Atlas doesn't send multiple entity-delete events for an entity. Please make sure that no entity-delete event is sent out while hard-deleting an entity in soft-deleted state.
- Madhan Neethiraj
On Dec. 5, 2018, 3:43 p.m., Graham Wallis wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/69494/
> -----------------------------------------------------------
>
> (Updated Dec. 5, 2018, 3:43 p.m.)
>
>
> Review request for atlas.
>
>
> Repository: atlas
>
>
> Description
> -------
>
> ATLAS-2985: critical fixes for delete handlers and relationship dtore
>
>
> Diffs
> -----
>
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/DeleteHandlerV1.java c57e30ab5cdb34aee1cb13c463b1b0af827d2a2e
> repository/src/main/java/org/apache/atlas/repository/store/graph/v1/HardDeleteHandlerV1.java edf1eed4c3efdd41f77155e51c8f2908b3dbbab2
> repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasRelationshipStoreV2.java 86cc98cda61d85954fd227501d691e3a3b542611
>
>
> Diff: https://reviews.apache.org/r/69494/diff/2/
>
>
> Testing
> -------
>
> Tested with full run of Egeria CTS suite.
>
>
> Thanks,
>
> Graham Wallis
>
>
Re: Review Request 69494: ATLAS-2985 fixes for delete handlers and
relationship store
Posted by Graham Wallis <gr...@uk.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/69494/
-----------------------------------------------------------
(Updated Dec. 5, 2018, 3:43 p.m.)
Review request for atlas.
Changes
-------
Addressed issues from first review - did not change event generation considerations as Atlas already produces multiple notifications per delete, and a notification is needed on a soft delete and on a hard delete. I would suggest that event duplication is addressed in a separate JIRA that would suggest that a dedicated event type is introduced for a HARD delete - as this is a different operation to a SOFT delete.
Repository: atlas
Description
-------
ATLAS-2985: critical fixes for delete handlers and relationship dtore
Diffs (updated)
-----
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/DeleteHandlerV1.java c57e30ab5cdb34aee1cb13c463b1b0af827d2a2e
repository/src/main/java/org/apache/atlas/repository/store/graph/v1/HardDeleteHandlerV1.java edf1eed4c3efdd41f77155e51c8f2908b3dbbab2
repository/src/main/java/org/apache/atlas/repository/store/graph/v2/AtlasRelationshipStoreV2.java 86cc98cda61d85954fd227501d691e3a3b542611
Diff: https://reviews.apache.org/r/69494/diff/2/
Changes: https://reviews.apache.org/r/69494/diff/1-2/
Testing
-------
Tested with full run of Egeria CTS suite.
Thanks,
Graham Wallis