[FLINK-23842][coordination] Add logging statements in SourceCoordinators for reader registration and split requests.

[StephanEwen]

What is the purpose of the change

Previously, there were no log statements when source enumerators get reader registration events, or when they receive split requests.

While some specific source implementations log this in their implementation, for the general case, this information were missing, even though it is super valuable when debugging and understanding the work assignment behavior.

This PR adds these log statements.

Main question to consider during the review: Would the log volume be too high?

• For reader registration logging: Upon full failover in streaming, up to as many log lines as parallelism on recovery when readers register again.
• For split request: For streaming, one line per initial split request, typically few requests after that. For batch, one line per each split request over time.

[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 70b7b49 (Tue Aug 17 16:57:13 UTC 2021)

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.
• ❓ 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

[flinkbot]

CI report:

• 70b7b49 Azure: SUCCESS

Bot commands

The @flinkbot bot supports the following commands:

• @flinkbot run azure re-run the last Azure build

[StephanEwen]

From some offline discussions, this looks like a good improvement.

Would merge this unless there are objections...

[tillrohrmann]

Thanks for creating this PR @StephanEwen. I am wondering whether the added information shouldn't rather be logged on debug level or do you think that this information will be helpful for a Flink user?

Somewhat related: We seem to output duplicate log information when debug log level is enabled.

[tillrohrmann]

I am wondering whether this information is really helpful for a user of Flink. My spontaneous gut feeling is that it could also be logged on debug log level. This, however, already happens a couple of lines above.

Hence, I guess the first question is whether this is information we want to expose to every user. If yes, then we should probably change the LOG.debug line above to not log duplicate information on debug log level.

[StephanEwen]

Let me try and deduplicate these lines.

I think in general, this is good information. Like in batch mode, when a source processes multiple splits, you don't see any progress being logged on the JM side.

[StephanEwen]

Addressed the comments and will merge this now...