[GitHub] flink pull request #4733: [FLINK-7701][network] really fix watermark configu...

[NicoK <gi...@git.apache.org>]

GitHub user NicoK opened a pull request:

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

    [FLINK-7701][network] really fix watermark configuration order this time

    ## What is the purpose of the change
    
    Netty (in the version we use) only allows setting high and low watermarks after another but complains about invalid values and ignores them, e.g. if the new high watermark is less than the (old) low watermark. This makes it important to get the order of the configuration options right. #4391 fixed this for large memory segment sizes (where the low watermark would have been larger than the high watermark) but broke the of small segment sizes which lead to the scenario outlined above. This should fix both.
    
    ## Brief change log
    
    - configure `ServerBootstrap` based on the current values vs. the defaults (those are unfortunately not exposed, so we need to inline the current one)
    - adapt `NettyServerLowAndHighWatermarkTest` to reflect both use cases (outlined above)
    
    ## Verifying this change
    
    This change added tests and can be verified as follows:
    
    - adapted `NettyServerLowAndHighWatermarkTest` to cover both high and low segment sizes
    - as before, no test for the `KvStateServer` since watermarks cannot be configured there (make sure it works the same way, though!)
    
    ## 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: JobManager (and its components), Checkpointing, Yarn/Mesos, ZooKeeper: (no)
    
    ## Documentation
    
      - Does this pull request introduce a new feature? (no)
      - If yes, how is the feature documented? (not applicable)

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

    $ git pull https://github.com/NicoK/flink flink-7701

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

    https://github.com/apache/flink/pull/4733.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 #4733
    
----
commit 509a3267d4613f48100a1e813ac49847e8bd5494
Author: Nico Kruber <ni...@data-artisans.com>
Date:   2017-09-27T11:44:35Z

    [FLINK-7701][network] really fix watermark configuration order this time
    
    FLINK-7258 fixed this for large memory segment sizes but broke it for small
    ones. This should fix both.
    
    FYI: Newer Netty versions actually circumvent the problem by allowing to set
    both watermarks at the same time.

----


---

[GitHub] flink pull request #4733: [FLINK-7701][network] really fix watermark configu...

[asfgit <gi...@git.apache.org>]

Github user asfgit closed the pull request at:

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


---

[GitHub] flink issue #4733: [FLINK-7701][network] really fix watermark configuration ...

[NicoK <gi...@git.apache.org>]

Github user NicoK commented on the issue:

    https://github.com/apache/flink/pull/4733
  
    yes, unfortunately this was for netty 4.1 only (it changed the API)...
    
    I rebased the PR to fix the conflict (which I did not have during the rebase though...)


---