You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@atlas.apache.org by Jeff Hagelberg <jn...@us.ibm.com> on 2017/02/08 00:47:46 UTC

Review Request 56417: ATLAS-1535: Some webapp tests are failing due to a stale Titan transaction

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

Review request for atlas and David Kantor.

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

Repository: atlas

Description
-------

I've added a new filter to consistently clean up stale transactions when processing http requests. I removed the old logic, which was only used by two service classes. The new filter is used everywhere.

Diffs
-----

webapp/src/main/java/org/apache/atlas/web/filters/StaleTransactionCleanupFilter.java PRE-CREATION
webapp/src/main/java/org/apache/atlas/web/listeners/GuiceServletConfig.java d0437fc54087e6803ead83af59b85e9f6df333ad
webapp/src/main/java/org/apache/atlas/web/resources/BaseService.java fb77b11e04c875a3d443dfb49be50cb818bed441
webapp/src/test/java/org/apache/atlas/web/resources/TaxonomyServiceTest.java e1734e465ad553d95735b466f4dfe0fa8ad061b8

Diff: https://reviews.apache.org/r/56417/diff/

Testing
-------

Ran webapp tests. Now, we're only left with the following failures:

QuickStartIT.runQuickStart:44 � AtlasService Metadata service API org.apache.a...
QuickStartV2IT.runQuickStart:47 � AtlasService Metadata service API org.apache...
NotificationHookConsumerIT.testUpdatePartialUpdatingQualifiedName:170 expected:<0> but was:<1>

Previously, there were a number of other tests that were failing sporatically with a NullPointerException coming from Titan. These changes fix that.

Thanks,

Jeff Hagelberg

RE: Review Request 56417: ATLAS-1535: Some webapp tests are failing due to a stale Titan transaction

Posted by Jeffrey N Hagelberg <jn...@us.ibm.com>.


Sure, I'll investigate.

-----Original Message-----
From: Madhan Neethiraj [mailto:madhan@apache.org]
Sent: Saturday, February 18, 2017 9:36 PM
To: dev@atlas.incubator.apache.org; Jeffrey N Hagelberg
<jn...@us.ibm.com>; Dave Kantor <dk...@us.ibm.com>
Subject: Re: Review Request 56417: ATLAS-1535: Some webapp tests are
failing due to a stale Titan transaction

Jeff,

Following test failures seem to be caused by removal of
TaxonomyServiceTest.initializeGraphTransaction() method in this patch. Can
you please review?

  TaxonomyServiceTest.testCreateSubTerm:456 expected:<true> but was:<false>
  TaxonomyServiceTest.testCreateTaxonomy:158 expected:<true> but
was:<false>
  TaxonomyServiceTest.testCreateTerm:416 expected:<true> but was:<false>
  TaxonomyServiceTest.testDeleteSubTerm:529 expected:<true> but was:<false>
  TaxonomyServiceTest.testDeleteTaxonomy:194 expected:<true> but
was:<false>
  TaxonomyServiceTest.testDeleteTerm:493 expected:<true> but was:<false>
  TaxonomyServiceTest.testGetSubTerms_collection:378 expected:<true> but
was:<false>
  TaxonomyServiceTest.testGetSubTerms_instance:326 expected:<true> but
was:<false>
  TaxonomyServiceTest.testGetTaxonomies:125 expected:<true> but was:<false>
  TaxonomyServiceTest.testGetTaxonomy:84 expected:<true> but was:<false>
  TaxonomyServiceTest.testGetTaxonomyTerm:235 expected:<true> but
was:<false>
  TaxonomyServiceTest.testGetTaxonomyTerms:277 expected:<true> but
was:<false>

Thanks,
Madhan

On 2/8/17, 2:29 PM, "Jeff Hagelberg" <noreply@reviews.apache.org on behalf
of jnhagelberg@us.ibm.com> wrote:


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

    (Updated Feb. 8, 2017, 10:29 p.m.)


    Review request for atlas and David Kantor.


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


    Repository: atlas


    Description (updated)
    -------

    When debugging some of the test failures in webapp, I found that many
are occurring when a http request runs in the context of a stale Titan
transaction. There is logic in BaseService to rollback the transaction
associated with a thread whenever a new request comes in. However, this
logic is not used in all places.

    This changes add logic to fix this.  I've added a new filter to
consistently clean up stale transactions when processing http requests.  I
removed the old logic, which was only used by two service classes.  The new
filter is applied for every http request that comes in, so the transaction
used during request processing will never be stale now.


    Diffs
    -----


webapp/src/main/java/org/apache/atlas/web/filters/StaleTransactionCleanupFilter.java
 PRE-CREATION

webapp/src/main/java/org/apache/atlas/web/listeners/GuiceServletConfig.java
d0437fc54087e6803ead83af59b85e9f6df333ad
      webapp/src/main/java/org/apache/atlas/web/resources/BaseService.java
fb77b11e04c875a3d443dfb49be50cb818bed441

webapp/src/test/java/org/apache/atlas/web/resources/TaxonomyServiceTest.java
 e1734e465ad553d95735b466f4dfe0fa8ad061b8

    Diff: https://reviews.apache.org/r/56417/diff/


    Testing
    -------

    Ran webapp tests.  Now, we're only left with the following failures:

    QuickStartIT.runQuickStart:44 » AtlasService Metadata service API
org.apache.a...
    QuickStartV2IT.runQuickStart:47 » AtlasService Metadata service API
org.apache...
    NotificationHookConsumerIT.testUpdatePartialUpdatingQualifiedName:170
expected:<0> but was:<1>

    Previously, there were a number of other tests that were failing
sporatically with a NullPointerException coming from Titan.  These changes
fix that.


    Thanks,

    Jeff Hagelberg

Re: Review Request 56417: ATLAS-1535: Some webapp tests are failing due to a stale Titan transaction

Posted by Madhan Neethiraj <ma...@apache.org>.

Jeff,

Following test failures seem to be caused by removal of TaxonomyServiceTest.initializeGraphTransaction() method in this patch. Can you please review?

  TaxonomyServiceTest.testCreateSubTerm:456 expected:<true> but was:<false>
  TaxonomyServiceTest.testCreateTaxonomy:158 expected:<true> but was:<false>
  TaxonomyServiceTest.testCreateTerm:416 expected:<true> but was:<false>
  TaxonomyServiceTest.testDeleteSubTerm:529 expected:<true> but was:<false>
  TaxonomyServiceTest.testDeleteTaxonomy:194 expected:<true> but was:<false>
  TaxonomyServiceTest.testDeleteTerm:493 expected:<true> but was:<false>
  TaxonomyServiceTest.testGetSubTerms_collection:378 expected:<true> but was:<false>
  TaxonomyServiceTest.testGetSubTerms_instance:326 expected:<true> but was:<false>
  TaxonomyServiceTest.testGetTaxonomies:125 expected:<true> but was:<false>
  TaxonomyServiceTest.testGetTaxonomy:84 expected:<true> but was:<false>
  TaxonomyServiceTest.testGetTaxonomyTerm:235 expected:<true> but was:<false>
  TaxonomyServiceTest.testGetTaxonomyTerms:277 expected:<true> but was:<false>

Thanks,
Madhan

On 2/8/17, 2:29 PM, "Jeff Hagelberg" <noreply@reviews.apache.org on behalf of jnhagelberg@us.ibm.com> wrote:

    
    -----------------------------------------------------------
    This is an automatically generated e-mail. To reply, visit:
    https://reviews.apache.org/r/56417/
    -----------------------------------------------------------
    
    (Updated Feb. 8, 2017, 10:29 p.m.)
    
    
    Review request for atlas and David Kantor.
    
    
    Bugs: ATLAS-1535
        https://issues.apache.org/jira/browse/ATLAS-1535
    
    
    Repository: atlas
    
    
    Description (updated)
    -------
    
    When debugging some of the test failures in webapp, I found that many are occurring when a http request runs in the context of a stale Titan transaction. There is logic in BaseService to rollback the transaction associated with a thread whenever a new request comes in. However, this logic is not used in all places.  
    
    This changes add logic to fix this.  I've added a new filter to consistently clean up stale transactions when processing http requests.  I removed the old logic, which was only used by two service classes.  The new filter is applied for every http request that comes in, so the transaction used during request processing will never be stale now.
    
    
    Diffs
    -----
    
      webapp/src/main/java/org/apache/atlas/web/filters/StaleTransactionCleanupFilter.java PRE-CREATION 
      webapp/src/main/java/org/apache/atlas/web/listeners/GuiceServletConfig.java d0437fc54087e6803ead83af59b85e9f6df333ad 
      webapp/src/main/java/org/apache/atlas/web/resources/BaseService.java fb77b11e04c875a3d443dfb49be50cb818bed441 
      webapp/src/test/java/org/apache/atlas/web/resources/TaxonomyServiceTest.java e1734e465ad553d95735b466f4dfe0fa8ad061b8 
    
    Diff: https://reviews.apache.org/r/56417/diff/
    
    
    Testing
    -------
    
    Ran webapp tests.  Now, we're only left with the following failures:
    
    QuickStartIT.runQuickStart:44 » AtlasService Metadata service API org.apache.a...
    QuickStartV2IT.runQuickStart:47 » AtlasService Metadata service API org.apache...
    NotificationHookConsumerIT.testUpdatePartialUpdatingQualifiedName:170 expected:<0> but was:<1>
    
    Previously, there were a number of other tests that were failing sporatically with a NullPointerException coming from Titan.  These changes fix that.
    
    
    Thanks,
    
    Jeff Hagelberg

Re: Review Request 56417: ATLAS-1535: Some webapp tests are failing due to a stale Titan transaction

Posted by Jeff Hagelberg <jn...@us.ibm.com>.

(Updated Feb. 8, 2017, 10:29 p.m.)

Review request for atlas and David Kantor.

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

Repository: atlas

Description (updated)
-------

When debugging some of the test failures in webapp, I found that many are occurring when a http request runs in the context of a stale Titan transaction. There is logic in BaseService to rollback the transaction associated with a thread whenever a new request comes in. However, this logic is not used in all places.

This changes add logic to fix this. I've added a new filter to consistently clean up stale transactions when processing http requests. I removed the old logic, which was only used by two service classes. The new filter is applied for every http request that comes in, so the transaction used during request processing will never be stale now.

Diffs
-----

Diff: https://reviews.apache.org/r/56417/diff/

Testing
-------

Ran webapp tests. Now, we're only left with the following failures:

Previously, there were a number of other tests that were failing sporatically with a NullPointerException coming from Titan. These changes fix that.

Thanks,

Jeff Hagelberg

Re: Review Request 56417: ATLAS-1535: Some webapp tests are failing due to a stale Titan transaction

Posted by Jeff Hagelberg <jn...@us.ibm.com>.

(Updated Feb. 8, 2017, 10:28 p.m.)

Review request for atlas and David Kantor.

Changes
-------

Addressed review comments.

If there are no other review comments, I plan to commit these changes at EOD tomorrow.

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

Repository: atlas

Description (updated)
-------

When debugging some of the test failures in webapp, I found that many are occurring when the http request that runs in the context of a stale Titan transaction. There is logic in BaseService to rollback the transaction associated with a thread whenever a new request comes in. However, this logic is not used in all places.

Diffs
-----

Diff: https://reviews.apache.org/r/56417/diff/

Testing
-------

Ran webapp tests. Now, we're only left with the following failures:

Previously, there were a number of other tests that were failing sporatically with a NullPointerException coming from Titan. These changes fix that.

Thanks,

Jeff Hagelberg

Re: Review Request 56417: ATLAS-1535: Some webapp tests are failing due to a stale Titan transaction

Posted by Jeff Hagelberg <jn...@us.ibm.com>.


> On Feb. 8, 2017, 8:41 p.m., David Kantor wrote:
> > webapp/src/main/java/org/apache/atlas/web/filters/StaleTransactionCleanupFilter.java, line 55
> > <https://reviews.apache.org/r/56417/diff/1/?file=1627084#file1627084line55>
> >
> >     What happens if there is no open transaction?  Is rollback() a no-op or otherwise innocuous?  I assume it must be innocuous otherwise there would be other problems.

It's a noop.


- Jeff


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


On Feb. 8, 2017, 10:28 p.m., Jeff Hagelberg wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56417/
> -----------------------------------------------------------
> 
> (Updated Feb. 8, 2017, 10:28 p.m.)
> 
> 
> Review request for atlas and David Kantor.
> 
> 
> Bugs: ATLAS-1535
>     https://issues.apache.org/jira/browse/ATLAS-1535
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> When debugging some of the test failures in webapp, I found that many are occurring when the http request that runs in the context of a stale Titan transaction. There is logic in BaseService to rollback the transaction associated with a thread whenever a new request comes in. However, this logic is not used in all places.  
> 
> This changes add logic to fix this.  I've added a new filter to consistently clean up stale transactions when processing http requests.  I removed the old logic, which was only used by two service classes.  The new filter is applied for every http request that comes in, so the transaction used during request processing will never be stale now.
> 
> 
> Diffs
> -----
> 
>   webapp/src/main/java/org/apache/atlas/web/filters/StaleTransactionCleanupFilter.java PRE-CREATION 
>   webapp/src/main/java/org/apache/atlas/web/listeners/GuiceServletConfig.java d0437fc54087e6803ead83af59b85e9f6df333ad 
>   webapp/src/main/java/org/apache/atlas/web/resources/BaseService.java fb77b11e04c875a3d443dfb49be50cb818bed441 
>   webapp/src/test/java/org/apache/atlas/web/resources/TaxonomyServiceTest.java e1734e465ad553d95735b466f4dfe0fa8ad061b8 
> 
> Diff: https://reviews.apache.org/r/56417/diff/
> 
> 
> Testing
> -------
> 
> Ran webapp tests.  Now, we're only left with the following failures:
> 
> QuickStartIT.runQuickStart:44 � AtlasService Metadata service API org.apache.a...
> QuickStartV2IT.runQuickStart:47 � AtlasService Metadata service API org.apache...
> NotificationHookConsumerIT.testUpdatePartialUpdatingQualifiedName:170 expected:<0> but was:<1>
> 
> Previously, there were a number of other tests that were failing sporatically with a NullPointerException coming from Titan.  These changes fix that.
> 
> 
> Thanks,
> 
> Jeff Hagelberg
> 
>

Re: Review Request 56417: ATLAS-1535: Some webapp tests are failing due to a stale Titan transaction

Posted by David Kantor <dk...@us.ibm.com>.

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


Ship it!




Please start the request description with the motiviation for making these changes i.e. several webapp tests were failing in Titan due to usage of stale transactions left open by previous usage of the thread.


webapp/src/main/java/org/apache/atlas/web/filters/StaleTransactionCleanupFilter.java (line 55)
<https://reviews.apache.org/r/56417/#comment236526>

    What happens if there is no open transaction?  Is rollback() a no-op or otherwise innocuous?  I assume it must be innocuous otherwise there would be other problems.


- David Kantor


On Feb. 8, 2017, 12:47 a.m., Jeff Hagelberg wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56417/
> -----------------------------------------------------------
> 
> (Updated Feb. 8, 2017, 12:47 a.m.)
> 
> 
> Review request for atlas and David Kantor.
> 
> 
> Bugs: ATLAS-1535
>     https://issues.apache.org/jira/browse/ATLAS-1535
> 
> 
> Repository: atlas
> 
> 
> Description
> -------
> 
> I've added a new filter to consistently clean up stale transactions when processing http requests.  I removed the old logic, which was only used by two service classes.  The new filter is used everywhere.
> 
> 
> Diffs
> -----
> 
>   webapp/src/main/java/org/apache/atlas/web/filters/StaleTransactionCleanupFilter.java PRE-CREATION 
>   webapp/src/main/java/org/apache/atlas/web/listeners/GuiceServletConfig.java d0437fc54087e6803ead83af59b85e9f6df333ad 
>   webapp/src/main/java/org/apache/atlas/web/resources/BaseService.java fb77b11e04c875a3d443dfb49be50cb818bed441 
>   webapp/src/test/java/org/apache/atlas/web/resources/TaxonomyServiceTest.java e1734e465ad553d95735b466f4dfe0fa8ad061b8 
> 
> Diff: https://reviews.apache.org/r/56417/diff/
> 
> 
> Testing
> -------
> 
> Ran webapp tests.  Now, we're only left with the following failures:
> 
> QuickStartIT.runQuickStart:44 � AtlasService Metadata service API org.apache.a...
> QuickStartV2IT.runQuickStart:47 � AtlasService Metadata service API org.apache...
> NotificationHookConsumerIT.testUpdatePartialUpdatingQualifiedName:170 expected:<0> but was:<1>
> 
> Previously, there were a number of other tests that were failing sporatically with a NullPointerException coming from Titan.  These changes fix that.
> 
> 
> Thanks,
> 
> Jeff Hagelberg
> 
>