Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FLINK-26420][Connector] use numRecordsSendCounter #19120

Merged
merged 4 commits into from
Mar 18, 2022

Conversation

JingGe
Copy link
Contributor

@JingGe JingGe commented Mar 16, 2022

What is the purpose of the change

This is the follow-up PR of #18825. We found that the new sink v2 interface will have a wrong numRecordsOut metric for the sink writers. We send a fixed number of records to the source, but the numRecordsOut of the sink continues to increase by the time.

Brief change log

  • replace the records out counter with the records send counter in FileWriter
  • replace the records and byte out counter with the records and byte send counter in AsyncSinkWriter
  • migrate FileWriterTest and AsyncSinkWriterTest to AssertJ

Verifying this change

This change is already covered by existing tests, such as FileWriterTest and AsyncSinkWriterTest.

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, Kubernetes/Yarn, 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)

@JingGe JingGe force-pushed the FLINK-26420-metric-file-connector branch from bc326cc to d77dc2c Compare March 16, 2022 16:52
@flinkbot
Copy link
Collaborator

flinkbot commented Mar 16, 2022

CI report:

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

@JingGe
Copy link
Contributor Author

JingGe commented Mar 16, 2022

@flinkbot run azure

1 similar comment
@JingGe
Copy link
Contributor Author

JingGe commented Mar 17, 2022

@flinkbot run azure

@JingGe JingGe force-pushed the FLINK-26420-metric-file-connector branch from d77dc2c to 9d0f8c2 Compare March 17, 2022 09:28
Copy link
Contributor

@AHeise AHeise left a comment

Choose a reason for hiding this comment

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

LGTM. Also thank you for migrating the tests!

@AHeise AHeise merged commit a2df266 into apache:master Mar 18, 2022
@JingGe JingGe deleted the FLINK-26420-metric-file-connector branch March 18, 2022 12:38
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.

4 participants