Re: [Discuss][Flink-31326] Flink autoscaler code

[Maximilian Michels]

We discussed in the PR that it's actually a feature, but thanks Yang
for bringing it up and improving the docs around this piece of code!

-Max

On Tue, Jan 2, 2024 at 10:06 PM Yang LI  wrote:
>
> Hello Rui,
>
> Here is the jira ticket https://issues.apache.org/jira/browse/FLINK-33966, I 
> have pushed a tiny pr for this ticket.
>
> Regards,
> Yang
>
> On Tue, 2 Jan 2024 at 16:15, Rui Fan <1996fan...@gmail.com> wrote:
>>
>> Thanks Yang for reporting this issue!
>>
>> You are right, these 2 conditions are indeed the same. It's unexpected IIUC.
>> Would you like to fix it?
>>
>> Feel free to create a FLINK JIRA to fix it if you would like to, and I'm
>> happy to
>> review!
>>
>> And cc @Maximilian Michels 
>>
>> Best,
>> Rui
>>
>> On Tue, Jan 2, 2024 at 11:03 PM Yang LI  wrote:
>>
>> > Hello,
>> >
>> > I see we have 2 times the same condition check in the
>> > function getNumRecordsInPerSecond (L220
>> > <
>> > https://github.com/apache/flink-kubernetes-operator/blob/main/flink-autoscaler/src/main/java/org/apache/flink/autoscaler/metrics/ScalingMetrics.java#L220
>> > >
>> > and
>> > L224
>> > <
>> > https://github.com/apache/flink-kubernetes-operator/blob/main/flink-autoscaler/src/main/java/org/apache/flink/autoscaler/metrics/ScalingMetrics.java#L224
>> > >).
>> > I imagine you want to use SOURCE_TASK_NUM_RECORDS_OUT_PER_SEC when the
>> > operator is not the source. Can you confirm this and if we have a FIP
>> > ticket to fix this?
>> >
>> > Regards,
>> > Yang LI
>> >

Re: [Discuss][Flink-31326] Flink autoscaler code

[Yang LI]

Hello Rui,

Here is the jira ticket https://issues.apache.org/jira/browse/FLINK-33966,
I have pushed a tiny pr
 for this
ticket.

Regards,
Yang

On Tue, 2 Jan 2024 at 16:15, Rui Fan <1996fan...@gmail.com> wrote:

> Thanks Yang for reporting this issue!
>
> You are right, these 2 conditions are indeed the same. It's unexpected
> IIUC.
> Would you like to fix it?
>
> Feel free to create a FLINK JIRA to fix it if you would like to, and I'm
> happy to
> review!
>
> And cc @Maximilian Michels 
>
> Best,
> Rui
>
> On Tue, Jan 2, 2024 at 11:03 PM Yang LI  wrote:
>
> > Hello,
> >
> > I see we have 2 times the same condition check in the
> > function getNumRecordsInPerSecond (L220
> > <
> >
> https://github.com/apache/flink-kubernetes-operator/blob/main/flink-autoscaler/src/main/java/org/apache/flink/autoscaler/metrics/ScalingMetrics.java#L220
> > >
> > and
> > L224
> > <
> >
> https://github.com/apache/flink-kubernetes-operator/blob/main/flink-autoscaler/src/main/java/org/apache/flink/autoscaler/metrics/ScalingMetrics.java#L224
> > >).
> > I imagine you want to use SOURCE_TASK_NUM_RECORDS_OUT_PER_SEC when the
> > operator is not the source. Can you confirm this and if we have a FIP
> > ticket to fix this?
> >
> > Regards,
> > Yang LI
> >
>

Re: [Discuss][Flink-31326] Flink autoscaler code

[Rui Fan]

Thanks Yang for reporting this issue!

You are right, these 2 conditions are indeed the same. It's unexpected IIUC.
Would you like to fix it?

Feel free to create a FLINK JIRA to fix it if you would like to, and I'm
happy to
review!

And cc @Maximilian Michels 

Best,
Rui

On Tue, Jan 2, 2024 at 11:03 PM Yang LI  wrote:

> Hello,
>
> I see we have 2 times the same condition check in the
> function getNumRecordsInPerSecond (L220
> <
> https://github.com/apache/flink-kubernetes-operator/blob/main/flink-autoscaler/src/main/java/org/apache/flink/autoscaler/metrics/ScalingMetrics.java#L220
> >
> and
> L224
> <
> https://github.com/apache/flink-kubernetes-operator/blob/main/flink-autoscaler/src/main/java/org/apache/flink/autoscaler/metrics/ScalingMetrics.java#L224
> >).
> I imagine you want to use SOURCE_TASK_NUM_RECORDS_OUT_PER_SEC when the
> operator is not the source. Can you confirm this and if we have a FIP
> ticket to fix this?
>
> Regards,
> Yang LI
>