[GitHub] flink pull request #5260: [FLINK-8375][network] Remove unnecessary synchroni...

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

GitHub user NicoK opened a pull request:

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

    [FLINK-8375][network] Remove unnecessary synchronization

    ## What is the purpose of the change
    
    Synchronized blocks in ResultPartition could affect only:
    1. totalNumberOfBuffers and totalNumberOfBytes counters
    2. subpartition add(), finish() and release() calls.
    
    However:
    1. accessors to the counters where not synchronized.
    2a. add(), finish() and release() methods for PipelinedSubpartition were already threads safe
    2b. add(), finish() and release() methods for SpillableSubpartition were made thread safe in
    this commit, by basically pushing synchronized section down one level.
    
    Please refer to dataArtisans/flink#1 (commits by @pnowojski, review by me: LGTM).
    This PR is based on #5259 and #4559.
    
    ## Brief change log
    
    - push synchronization down from `ResultPartition` where necessary
    - remove collection unnecessary statistics in `ResultPartition` (those required synchronization but could be gathered from the subpartitions if we decided to ever include them again)
    
    ## Verifying this change
    
    This change is a trivial rework / code cleanup without any test coverage.
    
    ## 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): **yes**
      - Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Yarn/Mesos, ZooKeeper: **no**
      - The S3 file system connector: **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-8375

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

    https://github.com/apache/flink/pull/5260.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 #5260
    
----
commit fada3022186670851a3956a961f490e6d86e2a53
Author: Nico Kruber <ni...@...>
Date:   2017-12-13T14:28:08Z

    [hotfix][checkstyle] only ignore checkstyle in existing packages under runtime.io.network
    
    - ignore runtime.io.(async|disk)
    - ignore runtime.io.network.(api|buffer|netty|partition|serialization|util)
    -> everything else will be checked against the ruleset
    - fix checkstyle errors in classes directly under runtime.io.network

commit f8dff47a707c4e7572d02e072197927ec2ce2ef7
Author: Nico Kruber <ni...@...>
Date:   2017-12-14T16:30:19Z

    [FLINK-8252][benchmarks] convert network benchmarks to streaming benchmarks
    
    This allows us to use the output flushing interval as a parameter to evaluate,
    too.

commit a4980ae85bac1dca9f8939e0dc3c8839991ed5e8
Author: Zhijiang <wa...@...>
Date:   2017-08-17T11:38:45Z

    [FLINK-7468][network] Implement sender backlog logic for credit-based

commit 0da032b3e7b90da2cbee5ca6f051667add104ac6
Author: Piotr Nowojski <pi...@...>
Date:   2018-01-05T14:28:40Z

    [FLINK-8375][network] Remove unnecessary synchronization
    
    Synchronized blocks in ResultPartition could affect only:
    1. totalNumberOfBuffers and totalNumberOfBytes counters
    2. subpartition add(), finish() and release() calls.
    
    However:
    1. counters were not used anywhere - they are removed by this commit
    2a. add(), finish() and release() methods for PipelinedSubpartition were already threads safe
    2b. add(), finish() and release() methods for SpillableSubpartition were made thread safe in
    this commit, by basically pushing synchronized section down one level.

----


---

[GitHub] flink pull request #5260: [FLINK-8375][network] Remove unnecessary synchroni...

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

Github user asfgit closed the pull request at:

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


---