Re: Review Request 69494: ATLAS-2985 fixes for delete handlers and relationship store

2018-12-05 Thread Madhan Neethiraj 

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

2018-12-05 Thread Graham Wallis 

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

Re: Review Request 69494: ATLAS-2985 fixes for delete handlers and relationship store

2018-11-30 Thread Madhan Neethiraj 

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


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)


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)


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

Review Request 69494: ATLAS-2985 fixes for delete handlers and relationship store

2018-11-30 Thread Graham Wallis 

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