You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tephra.apache.org by anew <gi...@git.apache.org> on 2017/08/31 21:51:13 UTC

[GitHub] incubator-tephra pull request #48: [TEPHRA-241] Introduce a way to limit the...

GitHub user anew opened a pull request:

    https://github.com/apache/incubator-tephra/pull/48

    [TEPHRA-241] Introduce a way to limit the size of a transaction

    - first commit introduces four new configurations for change set size limits, enforces these limits in a new method canCommitOrThrow() (we keep canCommit() for compatibility), and uses the new method throughout the higher level classes and tests
    - second commit refactors the transaction system test cases: Don't start a new TxManager for each test method, have a shared common configuration across subclasses, move the checkpoint test to the base class such that it also gets tested with Thrift
    - third commit adds a new test case for size limit validation
    
    Thrift changes are backward-compatible (we keep the existing methods the same), and so are the TransactionSystemClient changes. 
     

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/anew/incubator-tephra tephra-241

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/incubator-tephra/pull/48.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #48
    
----
commit 7227bcef076ccf762b78bf299003c373e7e73814
Author: anew <an...@apache.org>
Date:   2017-08-31T21:43:32Z

    add new config properties and a new method canCommitOrThrow() that validates the size of change sets

commit 4ad0f924c34ad4aea35443c8cd10bd81cf24eeb8
Author: anew <an...@apache.org>
Date:   2017-08-31T21:43:56Z

    refactor transaction system test

commit f3a1e786ae62f6bf05043c6fd770338361e615bc
Author: anew <an...@apache.org>
Date:   2017-08-31T21:44:32Z

    add new test for change set size validation

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-tephra pull request #48: [TEPHRA-241] Introduce a way to limit the...

Posted by anew <gi...@git.apache.org>.
Github user anew commented on a diff in the pull request:

    https://github.com/apache/incubator-tephra/pull/48#discussion_r136458250
  
    --- Diff: tephra-core/src/main/java/org/apache/tephra/distributed/thrift/TTransactionServer.java ---
    @@ -68,6 +68,8 @@
     
         public TBoolean canCommitTx(TTransaction tx, Set<ByteBuffer> changes) throws TTransactionNotInProgressException, org.apache.thrift.TException;
     
    +    public TBoolean canCommitOrThrow(TTransaction tx, Set<ByteBuffer> changes) throws TTransactionNotInProgressException, TGenericException, org.apache.thrift.TException;
    --- End diff --
    
    please do not review this file, it is generated by Thrift. Only need to review transaction.thrift


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-tephra pull request #48: [TEPHRA-241] Introduce a way to limit the...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/incubator-tephra/pull/48


---

[GitHub] incubator-tephra issue #48: [TEPHRA-241] Introduce a way to limit the size o...

Posted by anew <gi...@git.apache.org>.
Github user anew commented on the issue:

    https://github.com/apache/incubator-tephra/pull/48
  
    Travis build failed again in the known flaky test with Java 8. It does pass when I run it with Java 8 locally. Java 7 also passes in Travis. I am confident that this was not introduced by the changes in this PR, and will commit this now. 


---

[GitHub] incubator-tephra pull request #48: [TEPHRA-241] Introduce a way to limit the...

Posted by anew <gi...@git.apache.org>.
Github user anew commented on a diff in the pull request:

    https://github.com/apache/incubator-tephra/pull/48#discussion_r136457929
  
    --- Diff: tephra-core/src/test/java/org/apache/tephra/TransactionManagerTest.java ---
    @@ -54,115 +54,35 @@ protected TransactionStateStorage getStateStorage() {
         return txStateStorage;
       }
     
    -  @Before
    -  public void before() {
    +  @BeforeClass
    +  public static void beforeClass() {
    +    conf = getCommonConfiguration();
         conf.setInt(TxConstants.Manager.CFG_TX_CLEANUP_INTERVAL, 0); // no cleanup thread
    -    conf.setInt(TxConstants.Manager.CFG_TX_MAX_TIMEOUT, (int) TimeUnit.DAYS.toSeconds(5)); // very long limit
         // todo should create two sets of tests, one with LocalFileTxStateStorage and one with InMemoryTxStateStorage
         txStateStorage = new InMemoryTransactionStateStorage();
         txManager = new TransactionManager
           (conf, txStateStorage, new TxMetricsCollector());
         txManager.startAndWait();
       }
     
    -  @After
    -  public void after() {
    +  @AfterClass
    +  public static void afterClass() {
         txManager.stopAndWait();
       }
     
    -  @Test
    --- End diff --
    
    this test moved to TransactionSystemTest without change.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-tephra issue #48: [TEPHRA-241] Introduce a way to limit the size o...

Posted by anew <gi...@git.apache.org>.
Github user anew commented on the issue:

    https://github.com/apache/incubator-tephra/pull/48
  
    After rebasing on latest master, another travis build is running; waiting for that to finish.


---

[GitHub] incubator-tephra issue #48: [TEPHRA-241] Introduce a way to limit the size o...

Posted by anew <gi...@git.apache.org>.
Github user anew commented on the issue:

    https://github.com/apache/incubator-tephra/pull/48
  
    Same travis failure. This appears to happen a lot more frequently with Java 8. I will commit this now and try to fix the flaky test before 0.13 release.


---