GitHub user tzulitai opened a pull request:

    https://github.com/apache/flink/pull/5201

    [FLINK-8283] [kafka] Stabalize FlinkKafkaConsumerBaseTest::testScaleUp()

    ## What is the purpose of the change
    
    This PR sits upon other `FlinkKafkaConsumerBaseTest` changes from #5188 and 
#5200 .
    Only the last commit d53409c is relevant.
    
    As it seems, the previous parameters for `testScaleUp()` was too aggressive 
and took up too many resources, which cause the test resource to be terminated 
before the test could finish.
    
    This PR adjust the `testScaleUp()` arguments by testing the same behaviour, 
while consuming less resources.
    Previous settings were: `initialPartitions: 5 -> scale to 15, with 1000 
Kafka partitions on restore`
    Changed to: `initialPartitions: 5 -> scale to 8, with 30 Kafka partitions 
on restore`
    
    ## Brief change log
    
    - Modify test arguments for `testScaleUp()`
    
    ## Verifying this change
    
    The `testScaleUp()` should still work fine.
    I'm also running several Travis iterations of the tests on my local 
branches, to see if the instability persists.
    
    ## Does this pull request potentially affect one of the following parts:
    
      - Dependencies (does it add or upgrade a dependency): no
      - The public API, i.e., is any changed class annotated with 
`@Public(Evolving)`: no
      - The serializers: no
      - The runtime per-record code paths (performance sensitive): no
      - Anything that affects deployment or recovery: no
      - The S3 file system connector: no
    
    ## Documentation
    
      - Does this pull request introduce a new feature? no
      - If yes, how is the feature documented? n/a


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

    $ git pull https://github.com/tzulitai/flink FLINK-8283

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

    https://github.com/apache/flink/pull/5201.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 #5201
    
----
commit cb3f488877b7fb2ab0dfcc5c24fe53035bc765e7
Author: Tzu-Li (Gordon) Tai <tzulitai@...>
Date:   2017-12-20T00:10:44Z

    [FLINK-8296] [kafka] Rework FlinkKafkaConsumerBaseTest to not rely on Java 
reflection
    
    Reflection was mainly used to inject mocks into private fields of the
    FlinkKafkaConsumerBase, without the need to fully execute all operator
    life cycle methods. This, however, caused the unit tests to be too
    implementation-specific.
    
    This commit reworks the FlinkKafkaConsumerBaseTest to remove test
    consumer instantiation methods that rely on reflection for dependency
    injection. All tests now instantiate dummy test consumers normally, and
    let all tests properly execute all operator life cycle methods
    regardless of the tested logic.

commit 780744fd02f657364faf975d4d0b78ecf530236e
Author: Tzu-Li (Gordon) Tai <tzulitai@...>
Date:   2017-12-19T19:55:16Z

    [FLINK-8306] [kafka] Introduce KafkaOffsetCommitter interface
    
    Prior to this commit, offset committing was coupled tightly with the
    AbstractFetcher, making unit tests for offset committing behaviours hard
    to compose concisely. For example, we had tests that mock the offset
    commit methods on AbstractFetcher, while ideally, it would be best that
    those methods are made final to prevent accidental overrides.
    
    This commit decouples offset committing as a separate service behind a
    new KafkaOffsetCommitter interface. For now, the AbstractFetcher is
    reused as an implementation of this service.
    
    Unit tests that verify offset committing behaviour now provide a dummy
    verifyable implementation of the KafkaOffsetCommitter, instead of using
    mocks on AbstractFetcher.

commit e637145f77c5acf50d625bd40acf71738d7292fa
Author: Tzu-Li (Gordon) Tai <tzulitai@...>
Date:   2017-12-20T19:54:40Z

    [hotfix] [kafka] Remove stale comment on publishing procedures of 
AbstractFetcher
    
    The previous comment mentioned "only now will the fetcher return at
    least the restored offsets when calling snapshotCurrentState()". This is
    a remnant of the previous fetcher initialization behaviour, where in the
    past the fetcher wasn't directly seeded with restored offsets on
    instantiation.
    
    Since this is no longer true, this commit fixes the stale comment to
    avoid confusion.

commit d53409c2c229c7132fd84d756d6a6c85d7f7a579
Author: Tzu-Li (Gordon) Tai <tzulitai@...>
Date:   2017-12-21T21:41:48Z

    [FLINK-8283] [kafka] Stabalize FlinkKafkaConsumerBaseTest::testScaleUp()
    
    Previously, the testScaleUp() test was taking too much resources and
    causing test resources to be terminated before the test could finish.
    This commit lowers the intensity of the test, while still retaining the
    verified behaviour (i.e., when restoring the Kafka consumer with higher
    parallelism and more Kafka partitions).

----


---

Reply via email to