Skip to content

[FLINK-27944] Input channel metric will no longer be registered multiple times#21437

Merged
pnowojski merged 2 commits intoapache:masterfrom
reswqa:FLINK-27944-io-metrics-test
Dec 1, 2022
Merged

[FLINK-27944] Input channel metric will no longer be registered multiple times#21437
pnowojski merged 2 commits intoapache:masterfrom
reswqa:FLINK-27944-io-metrics-test

Conversation

@reswqa
Copy link
Copy Markdown
Member

@reswqa reswqa commented Nov 30, 2022

What is the purpose of the change

If a task has multiple input gates, It will create as many InputChannelMetrics as the number of gates, so the corresponding metrics are registered repeatedly. This pull request aimed to fix this problem.

Brief change log

  • Create InputChannelMetrics in NettyShuffleEnvironment#createInputGates.

Verifying this change

This change added unit test in NettyShuffleEnvironmentTest.

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, Kubernetes/Yarn, ZooKeeper: no
  • The S3 file system connector: no

Documentation

  • Does this pull request introduce a new feature? no

@flinkbot
Copy link
Copy Markdown
Collaborator

flinkbot commented Nov 30, 2022

CI report:

Bot commands The @flinkbot bot supports the following commands:
  • @flinkbot run azure re-run the last Azure build

@reswqa
Copy link
Copy Markdown
Member Author

reswqa commented Nov 30, 2022

Hi @pnowojski, Would you mind helping me to take a look this pull request when you have time? Thanks very much~

Copy link
Copy Markdown
Contributor

@pnowojski pnowojski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, change LGTM. However can you preserve the Xiaogang's authorship of the bug fix? I would recommend either having 3 commits here:

  1. the junit5 migration (as it is)
  2. cherry picking Xiaogang's commit
  3. your commit adding the test

or having the second commit having two authors.

…red multiple times

Co-authored-by: xiaogang <zhouxiaogang@shizhuang-inc.com>
@reswqa
Copy link
Copy Markdown
Member Author

reswqa commented Dec 1, 2022

@pnowojski Thank you very much for reminding me.
I'm sorry for forgetting this. I still keep two commits, but add Xiaogang as a co-author on the second one.

@pnowojski pnowojski merged commit 20808fd into apache:master Dec 1, 2022
FMX pushed a commit to apache/celeborn that referenced this pull request May 15, 2025
…nnelMetrics repeatedly

### What changes were proposed in this pull request?

`RemoteShuffleEnvironment` should not register `InputChannelMetrics` repeatedly, which only create once in `RemoteShuffleEnvironment#createInputGates`.

### Why are the changes needed?

If a task has multiple input gates, It will create as many `InputChannelMetrics` as the number of gates, so the corresponding metrics are registered repeatedly. Therefore, `RemoteShuffleEnvironment` should not register `InputChannelMetrics` repeatedly for `createInputGates`.

Backport apache/flink#21437.

### Does this PR introduce _any_ user-facing change?

No.

### How was this patch tested?

Manual.

Closes #3257 from SteNicholas/CELEBORN-1998.

Authored-by: SteNicholas <programgeek@163.com>
Signed-off-by: mingji <fengmingxiao.fmx@alibaba-inc.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants