[FLINK-13442][network] Remove unnecessary notifySubpartitionConsumed method from view reader

[zhijiangW]

What is the purpose of the change

Currently the methods of NetworkSequenceViewReader#notifySubpartitionConsumed and NetworkSequenceViewReader#releaseAllResources would be called meanwhile in netty stack during releasing resources.

As confirmed in FLINK-13245, in order to make this release logic simple and clean, we could remove the redundant notifySubpartitionConsumed from NetworkSequenceViewReader side, and also remove it from ResultSubpartitionView side. In the implementation of ResultSubpartitionView#releaseAllResources it would further notify the parent subpartition of consumed state via ResultSubpartition#notifySubpartitionConsumed which further feedback to parent ResultPartition layer via onConsumedSubpartition. Finally ResultPartition could decide whether to release itself or not.

E.g. for the case of ReleaseOnConsumptionResultPartition which is mainly used for pipelined partition, it would release partition after reference counter decreased to
0. For the case of ResultPartition which would be generated for blocking partition by default, it would never be released after notifying consumed. And the JM/scheduler
would decide when to release partition properly. In addition, InputChannel#notifySubpartitionConsumed could also be removed as a result of above.

Brief change log

• Remove notifySubpartitionConsumed from NetworkSequenceViewReader
• Remove notifySubpartitionConsumed from ResultSubpartitionView
• Remove notifySubpartitionConsumed from InputChannel

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): (no)
• 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)

[flinkbot]

Thanks a lot for your contribution to the Apache Flink project. I'm the @flinkbot. I help the community
to review your pull request. We will use this comment to track the progress of the review.

Automated Checks

Last check on commit a42027d (Tue Aug 27 09:17:50 UTC 2019)

Warnings:

• No documentation files were touched! Remember to keep the Flink docs up to date!

_{Mention the bot in a comment to re-run the automated checks.}

Review Progress

• ❓ 1. The [description] looks good.
• ❓ 2. There is [consensus] that the contribution should go into to Flink.
• ❗ 3. Needs [attention] from.
  • Needs attention by @pnowojski [committer], @StephanEwen
• ❓ 4. The change fits into the overall [architecture].
• ❓ 5. Overall code [quality] is good.

Please see the Pull Request Review Guide for a full explanation of the review process.

Details

The Bot is tracking the review progress through labels. Labels are applied according to the order of the review items. For consensus, approval by a Flink committer of PMC member is required

Bot commands

The @flinkbot bot supports the following commands:

• @flinkbot approve description to approve one or more aspects (aspects: description, consensus, architecture and quality)
• @flinkbot approve all to approve all aspects
• @flinkbot approve-until architecture to approve everything until architecture
• @flinkbot attention @username1 [@username2 ..] to require somebody's attention
• @flinkbot disapprove architecture to remove an approval you gave earlier

[flinkbot]

CI report:

• bfc3f20 : FAILURE Build
• 6ef24d8 : FAILURE Build
• 44c5782 : FAILURE Build
• a42027d : SUCCESS Build

[zhijiangW]

The refactoring work which was confirmed in FLINK-13245 should not be the blocker for release-1.9. But during refactoring to remove the notifySubpartitionConsumed methods, I found another possible bug in previous implementation. So I submitted these two things in separate commit.

@flinkbot attention @StephanEwen @pnowojski

[zhijiangW]

I removed the last commit for fixing the problem of notifySubpartitionConsumed which does not consider multiple readers in blocking subpartition. That issue would be traced separately in FLINK-13493 and this PR is not necessary for current release-1.9 now.

[pnowojski]

LGTM.

@flinkbot run travis

@zhijiangW can you merge this with green travis?

[zhijiangW]

Thanks for review @pnowojski ! I would handle it later.