[FLINK-30454] Inconsistent class hierarchy in TaskIOMetricGroup

• Type: Improvement
• Status: Closed
• Priority: Major
• Resolution: Fixed
• Fix Version/s: 1.17.0
• Component/s: Runtime / Metrics
• Labels: pull-request-available
• Assignee: Gunnar Morling
• Reporter: Gunnar Morling
• Created: 19/Dec/22 14:15
• Updated: 05/Jan/23 08:07
• Resolved: 05/Jan/23 08:07

Description

I noticed an interesting issue when trying to compile the flink-runtime module with Eclipse (same for VSCode): the private inner class org.apache.flink.runtime.metrics.groups.TaskIOMetricGroup.SizeGauge has yet another public inner class, SizeSupplier. The public method org.apache.flink.runtime.metrics.groups.TaskIOMetricGroup.registerMailboxSizeSupplier(SizeSupplier<Integer>) has a parameter of that type.

The invocation of this method in org.apache.flink.streaming.runtime.tasks.StreamTask.StreamTask(Environment, TimerService, UncaughtExceptionHandler, StreamTaskActionExecutor, TaskMailbox) can be compiled with the javac compiler of the JDK, but fails to compile with ecj:

The type TaskIOMetricGroup.SizeGauge from the descriptor computed for the target context is not visible here.  

I tend to believe that the behavior of Eclipse's compiler is the correct one. After all, you couldn't explicitly reference the SizeSupplier type either.

One possible fix would be to promote SizeSupplier to the same level as SizeGauge. This would be source-compatible but not binary-compatible, though. I.e. code compiled against the earlier signature of registerMailboxSizeSupplier() would be broken with a NoSuchMethodError. Depending on whether registerMailboxSizeSupplier() are expected in client code or not, this may or may not be acceptable.

Another fix would be to make SizeGauge public. I think that's the change I'd do. Curious what other folks here think.

Issue Links

• links to: GitHub Pull Request #21531