[FLINK-12067][network] Refactor the constructor of NetworkEnvironment

[zhijiangW]

What is the purpose of the change

The constructor of NetworkEnvironment could be refactored to only contain NetworkEnvironmentConfiguration, the other related components such as TaskEventDispatcher, ResultPartitionManager, NetworkBufferPool could be created internally.

We also refactor the process of generating NetworkEnvironmentConfiguration in TaskManagerServiceConfiguration to add numNetworkBuffers instead of previous networkBufFraction, networkBufMin, networkBufMax. This way seems more easy and direct to construct NetworkBufferPool later. isCreditBased field is also maintained in this component for considering the setting usage in tests.

Further we introduce the NetworkEnvironmentConfigurationBuilder for creating NetworkEnvironmentConfiguration easily especially for tests.

Brief change log

• Refactor the constructor of NetworkEnvironment to container only `NetworkEnvironmentConfiguration
• Refactor the constructor of NetworkEnvironmentConfiguration
• Introduce the NetworkEnvironmentConfigurationBuilder` for tests

Verifying this change

This change is already covered by existing tests, such as NetworkEnvironmentTest and NetworkBufferCalculationTest.

Does this pull request potentially affect one of the following parts:

• Dependencies (does it add or upgrade a dependency): (yes / no)
• The public API, i.e., is any changed class annotated with @Public(Evolving): (yes / no)
• The serializers: (yes / no / don't know)
• The runtime per-record code paths (performance sensitive): (yes / no / don't know)
• Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Yarn/Mesos, ZooKeeper: (yes / no / don't know)
• The S3 file system connector: (yes / no / don't know)

Documentation

• Does this pull request introduce a new feature? (yes / no)
• If yes, how is the feature documented? (not applicable / docs / JavaDocs / not documented)

[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.

Review Progress

• ❓ 1. The [description] looks good.
• ❓ 2. There is [consensus] that the contribution should go into to Flink.
• ❓ 3. Needs [attention] from.
• ❓ 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.

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

[azagrebin]

Thanks for opening this PR @zhijiangW! It should really simplify a lot of things :)
I left one comment.
Also, what do you think if parseNetworkEnvironmentConfiguration and all related methods are moved into NetworkEnvironmentConfiguration.fromConfiguration including parsing slots config?
Just thoughts: at the end, ShuffleService will be created directly from raw configuration in ShuffleManager, so TaskManagerServices.fromConfiguration might need to take raw configuration and ShuffleManager.

[zhijiangW]

Thanks for reviews and suggestions @azagrebin .
I agree to further move parseNetworkEnvironmentConfiguration into NetworkEnvironmentConfiguration and refactor more parts.

[zhijiangW]

@azagrebin I submitted two separate fixup commits for addressing above comments.

[azagrebin]

Thanks for more refactoring @zhijiangW!
I have left some smaller questions and suggestions.

[zhijiangW]

@azagrebin , thanks for review again. :)
I submitted the commit for rebasing master and addressing the above comments.

[zhijiangW]

@azagrebin thanks for review again. I submitted the commit for addressing left comments.

[azagrebin]

Thanks @zhijiangW ! LGTM 👍

[pnowojski]

I took a quite brief look and the change looks good. @zhijiangW could you check one my comment and rebase/resolve the conflicts before I merge it?

[zhijiangW]

@pnowojski , thanks for your review! I have rebased the master to solve the conflicts.

[pnowojski]

Thanks for the change @zhijiangW and review @azagrebin. Merging :)

[pnowojski]

One remark @zhijiangW, if you are already providing more detailed description of a change in the PR description, please remember about copying it to the commit message as well :) I copied it in this case.

[zhijiangW]

Thanks for merging and the kindly reminder. I would copy it to the commit next time. :)