atlas-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Graham Wallis <graham_wal...@uk.ibm.com>
Subject Re: Review Request 63879: ATLAS-2262 Fix intermittent relationship failures
Date Wed, 06 Dec 2017 16:56:41 GMT

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




repository/src/main/java/org/apache/atlas/repository/graph/GraphHelper.java
Lines 194 (patched)
<https://reviews.apache.org/r/63879/#comment271436>

    Added in revised patch



repository/src/main/java/org/apache/atlas/repository/graph/GraphHelper.java
Line 202 (original), 209 (patched)
<https://reviews.apache.org/r/63879/#comment271437>

    I don't think that can occur - the get of adjacent edges (immediately above) will only
retrieve edges with the specified label. Does that cover your concern?



repository/src/main/java/org/apache/atlas/repository/graph/GraphHelper.java
Line 204 (original), 211 (patched)
<https://reviews.apache.org/r/63879/#comment271435>

    I think they can be ignored.



repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityGraphDiscoveryV1.java
Lines 364 (patched)
<https://reviews.apache.org/r/63879/#comment271443>

    This was to cater for what happens during import service, when you get a partial entity.
i.e. you could be reading an entity that *should* have a reference to another but you haven't
read the other entity yet. So you have a kind of dangling, incomplete reference. You can find
out from the entityType that it should have a relationship attribute, but if you try to use
getRelationshipAttribute() to read it you will not get it. But knowing from the type that
this is a relationship, we can use the more general getAttribute(). If you don't do this then
the result is the object reference will not be recorded, which means that the entity store
will not do a preCreate and the entity graph mapper will not be able to find the attribute
vertex, so it barfs and gives up.



repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityGraphDiscoveryV1.java
Lines 370 (patched)
<https://reviews.apache.org/r/63879/#comment271444>

    Personally I quite like it the way it is - I think it is clearer.



repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphMapper.java
Line 805 (original), 856 (patched)
<https://reviews.apache.org/r/63879/#comment271445>

    Sounds a good suggestion - I'll have to get you to show me how to do that.



repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphMapper.java
Lines 941 (patched)
<https://reviews.apache.org/r/63879/#comment271438>

    I don't know - it is one of the enumeration values that is not processed within this switch.



repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphMapper.java
Lines 964 (patched)
<https://reviews.apache.org/r/63879/#comment271439>

    Actually I think they are covered. If empty then we do no more processing; if not empty
we check index to make sure we have not already reached the end; assuming neither empty nor
off-the-end we retrieve the current edge and that may be present or not but the distinction
is delegated to mapStructValue (which gets the currentEdge value in the AttributeMutationContext)



repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphMapper.java
Lines 965 (patched)
<https://reviews.apache.org/r/63879/#comment271440>

    ?



repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphMapper.java
Lines 1012 (patched)
<https://reviews.apache.org/r/63879/#comment271446>

    I'm not sure I can comment on that; other than what I found during the import testing
was that there were partially complete objects that only later became properly formed - i.e.
when we read the remaining import data. So I get the impression that having sufficient error
handling in here is necessary. It *could* be supplemented by additional barriers behind the
REST interface but I think that is a separate thing.



repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphMapper.java
Lines 1028 (patched)
<https://reviews.apache.org/r/63879/#comment271441>

    Have added to revised patch



repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphMapper.java
Lines 1033 (patched)
<https://reviews.apache.org/r/63879/#comment271447>

    I don't know - it is not obvious to me that that's possible; I was seeing entity guids,
types and unique attributes in the AtlasObjectIds in the mutation contexts. Is it possible
to retrieve a relationship GUID as well?



repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphMapper.java
Lines 1037 (patched)
<https://reviews.apache.org/r/63879/#comment271448>

    getArrayElementsUsingRelationship will only get edges for the appropriate label



repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphMapper.java
Lines 1146 (patched)
<https://reviews.apache.org/r/63879/#comment271442>

    Because it adds to the overall number of elements processed; some of the original variable
names are a bit odd but I was trying to minimise changes.


- Graham Wallis


On Nov. 20, 2017, 5:15 p.m., Graham Wallis wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63879/
> -----------------------------------------------------------
> 
> (Updated Nov. 20, 2017, 5:15 p.m.)
> 
> 
> Review request for atlas.
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> ATLAS-2262.patch
> This patch contains fixes for the EntityGraphMapper and related classes, to fix intermittent
relationship failures.
> This change means that an UPDATE operation will inspect the existing and new relationships
and compare them fully before adding/modifying edges. Matching elements will be reused, new
elements will be added, and redundant elements will be discarded. The behaviour should be
consistent regardless of which graph provider is being used.
> 
> 
> Diffs
> -----
> 
>   repository/src/main/java/org/apache/atlas/repository/graph/GraphHelper.java 3e602431400677cbe0d8fe440732b02bb4a30a62

>   repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityGraphDiscoveryV1.java
739d610263a1b5bb8a0a9ff8183196ebe2c6294b 
>   repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityStoreV1.java
b0f9845d6e92a6ef4bb53cccec537a0a54afb5e8 
>   repository/src/main/java/org/apache/atlas/repository/store/graph/v1/AttributeMutationContext.java
b6d82dd834b8cc8b3bac630c084b91ac34dd0cec 
>   repository/src/main/java/org/apache/atlas/repository/store/graph/v1/EntityGraphMapper.java
f6a15b695e676bc035ce8336bcce7bacf1852ff9 
>   repository/src/test/java/org/apache/atlas/repository/impexp/ImportServiceTest.java
b24774d6376765f054022b9220b61769505e488c 
>   repository/src/test/java/org/apache/atlas/repository/store/graph/v1/AtlasEntityStoreV1Test.java
fd1b6db0c4dd57cd70ba56f49b9c4751f6915858 
>   repository/src/test/java/org/apache/atlas/repository/store/graph/v1/AtlasRelationshipStoreV1Test.java
d207a6958ab9dce0bd9f8e08c8e49232ab7d2e8a 
> 
> 
> Diff: https://reviews.apache.org/r/63879/diff/3/
> 
> 
> Testing
> -------
> 
> Build with full tests with janus grap provider
> 
> 
> Thanks,
> 
> Graham Wallis
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message