[jira] [Commented] (FLINK-30454) Inconsistent class hierarchy in TaskIOMetricGroup

2022-12-22 Thread Robert Metzger (Jira)


[ 
https://issues.apache.org/jira/browse/FLINK-30454?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17651304#comment-17651304
 ] 

Robert Metzger commented on FLINK-30454:


Thanks for your contribution!

Merged to master (1.17) in f82be845f3f673264a13eaf29e11b19e00f37222

> Inconsistent class hierarchy in TaskIOMetricGroup
> -
>
> Key: FLINK-30454
> URL: https://issues.apache.org/jira/browse/FLINK-30454
> Project: Flink
>  Issue Type: Improvement
>  Components: Runtime / Metrics
>Reporter: Gunnar Morling
>Assignee: Gunnar Morling
>Priority: Major
>  Labels: pull-request-available
> Fix For: 1.17.0
>
>
> 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)}}
>  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:
> {code:java}
> The type TaskIOMetricGroup.SizeGauge from the descriptor computed for the 
> target context is not visible here.  
> {code}
> 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.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)


[jira] [Commented] (FLINK-30454) Inconsistent class hierarchy in TaskIOMetricGroup

2022-12-19 Thread Gunnar Morling (Jira)


[ 
https://issues.apache.org/jira/browse/FLINK-30454?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17649383#comment-17649383
 ] 

Gunnar Morling commented on FLINK-30454:


Ok, cool. Logged 
[https://github.com/apache/flink/pull/21531/.|https://github.com/apache/flink/pull/21531/]

> Inconsistent class hierarchy in TaskIOMetricGroup
> -
>
> Key: FLINK-30454
> URL: https://issues.apache.org/jira/browse/FLINK-30454
> Project: Flink
>  Issue Type: Improvement
>  Components: Runtime / Metrics
>Reporter: Gunnar Morling
>Priority: Major
>  Labels: pull-request-available
>
> 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)}}
>  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:
> {code:java}
> The type TaskIOMetricGroup.SizeGauge from the descriptor computed for the 
> target context is not visible here.  
> {code}
> 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.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)


[jira] [Commented] (FLINK-30454) Inconsistent class hierarchy in TaskIOMetricGroup

2022-12-19 Thread Robert Metzger (Jira)


[ 
https://issues.apache.org/jira/browse/FLINK-30454?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17649365#comment-17649365
 ] 

Robert Metzger commented on FLINK-30454:


I'm pretty sure these are not public classes. We use the @Public / 
@PublicEvolving / @Internal annotations to mark interface visibility in Flink.
The TaskIOMetricGroup-stuff is, in my understanding anyways internal. 

> Inconsistent class hierarchy in TaskIOMetricGroup
> -
>
> Key: FLINK-30454
> URL: https://issues.apache.org/jira/browse/FLINK-30454
> Project: Flink
>  Issue Type: Improvement
>  Components: Runtime / Metrics
>Reporter: Gunnar Morling
>Priority: Major
>
> 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)}}
>  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:
> {code:java}
> The type TaskIOMetricGroup.SizeGauge from the descriptor computed for the 
> target context is not visible here.  
> {code}
> 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.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)


[jira] [Commented] (FLINK-30454) Inconsistent class hierarchy in TaskIOMetricGroup

2022-12-19 Thread Gunnar Morling (Jira)


[ 
https://issues.apache.org/jira/browse/FLINK-30454?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17649345#comment-17649345
 ] 

Gunnar Morling commented on FLINK-30454:


[~rmetzger] , curious about your thoughts on that compatibility question.

> Inconsistent class hierarchy in TaskIOMetricGroup
> -
>
> Key: FLINK-30454
> URL: https://issues.apache.org/jira/browse/FLINK-30454
> Project: Flink
>  Issue Type: Improvement
>  Components: Runtime / Metrics
>Reporter: Gunnar Morling
>Priority: Major
>
> 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)}}
>  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:
> {code}
> The type TaskIOMetricGroup.SizeGauge from the descriptor computed for the 
> target context is not visible here.  
> {code}
> 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.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)