Hi

In that case we could also close your ticket and the one you linked and
refer to this one as the selected approach :)

Gyula

On Fri, 24 Nov 2023 at 20:19, Surendra Singh Lilhore <
surendralilh...@gmail.com> wrote:

> Hi Gyula,
>
>
> Yes, I agree with this approach.
>
>
> Thanks
> Surendra
>
>
> On Sat, Nov 25, 2023, 12:28 AM Gyula Fóra <gyula.f...@gmail.com> wrote:
>
> > I think a better and more Kubernetes canonical approach would be what is
> > outlined in this ticket:
> >
> > https://issues.apache.org/jira/browse/FLINK-33548
> >
> > The upside is that it will work for all operator supported Flink versions
> > and is much simpler to use than the pod template.
> >
> > What do you think?
> >
> > Gyula
> >
> > On Fri, 24 Nov 2023 at 19:50, Surendra Singh Lilhore <
> > surendralilh...@apache.org> wrote:
> >
> > > Hi Gyula,
> > >
> > > Thank you for raising these valid concerns and sharing your
> perspective.
> > I
> > > agree that this new change will impact the existing pipelines.
> > >
> > > I brought up this issue because in Kubernetes environments, resource
> > > configuration is typically managed through Kubernetes objects like
> > > container templates or Custom Resources (CR). To align with this
> > > established practice in Kubernetes, I was planning to utilize pod
> > templates
> > > in the FlinkDeployment
> > > <
> > >
> >
> https://github.com/apache/flink-kubernetes-operator/blob/main/helm/flink-kubernetes-operator/crds/flinkdeployments.flink.apache.org-v1.yml#L61
> > > >
> > > CR instead of exclusively relying on Flink-specific configuration.
> > >
> > >
> > > There is similar type of issue raised : FLINK-24150
> > > <https://issues.apache.org/jira/browse/FLINK-24150>
> > >
> > > Thanks
> > > Surendra
> > >
> > > On Fri, Nov 24, 2023 at 4:16 PM Gyula Fóra <gyula.f...@gmail.com>
> wrote:
> > >
> > > > Hi Surendra!
> > > >
> > > > The resource configuration in Flink is pretty well established and
> > > > covers setting both memory requests and limits (through the limit
> > > factor.)
> > > >
> > > > Could you please elaborate why you think this change is a good
> > addition?
> > > >
> > > > I see a few downsides:
> > > >  - It complicates memory configuration by adding new options without
> > > > actually enabling anything new
> > > >  - Existing pipelines with podTemplates may suddenly start running
> with
> > > > different memory settings in prod after this change
> > > >
> > > > So at this point I am slightly against making this change, but I
> would
> > > like
> > > > to hear the thoughts of the community on this matter.
> > > >
> > > > Thanks!
> > > > Gyula
> > > >
> > > > On Thu, Nov 23, 2023 at 2:00 PM Surendra Singh Lilhore <
> > > > surendralilh...@apache.org> wrote:
> > > >
> > > > > Hello everyone,
> > > > >
> > > > > I've encountered an issue while using the flink open source
> > > > > kubernetes operator for Flink deployment. Despite setting resource
> > > limits
> > > > > in the pod template, it appears that these limits are not
> considered
> > > > during
> > > > > TaskManager (TM) pod deployment. Upon code investigation, it seems
> > the
> > > > > limits are being overridden by the default limit factor in
> > > > > KubernetesUtils#getResourceRequirements()
> > > > > <
> > > > >
> > > >
> > >
> >
> https://github.com/apache/flink/blob/master/flink-kubernetes/src/main/java/org/apache/flink/kubernetes/utils/KubernetesUtils.java#L372
> > > > > >
> > > > > .
> > > > >
> > > > > The current behavior of Flink only considers the limit from the
> > default
> > > > > factor, neglecting pod template resource limits. I propose Flink
> > should
> > > > > incorporate both the limit factor and pod template resource limits,
> > > > taking
> > > > > the maximum value.
> > > > >
> > > > > I've raised the issue and submitted a pull request:  FLINK-33609
> > > > > <https://github.com/apache/flink/pull/23768>
> > > > >
> > > > > During the review process, a valid concern was raised regarding the
> > > > > proposed changes. The suggestion is to initiate a quick discussion,
> > as
> > > > this
> > > > > modification will significantly alter the resource handling logic.
> > It's
> > > > > emphasized that maintaining consistency in the logic for both
> > resource
> > > > > requests and limits is crucial, rather than applying changes to
> only
> > > one
> > > > of
> > > > > them.
> > > > >
> > > > > I would appreciate any feedback on this.
> > > > >
> > > > > Thank you for your time and contributions to the Flink project.
> > > > >
> > > > > Thank you,
> > > > > Surendra
> > > > >
> > > >
> > >
> >
>

Reply via email to