Hi Yangze,
Thanks for your reply!
For a subtask, the taskManagerLocation is the physical location.
And the task id, operator id and subtask index may be the
logical locations of a subtask.
Actually, I don't think the taskManagerLocation is
a good name here.
Unlike task id, operator id, and subtask index, both location
and taskManagerLocation are ambiguous. subtask_index is
an unambiguous name in the Flink world.
That's why I think it's not a good name, especially we want to
unify the TaskManager Location in REST API and Web UI.
Of course, if we can't define a better name then location is fine
with me, thank you~
Best,
Rui
On Mon, Sep 18, 2023 at 9:06 PM Yangze Guo <karma...@gmail.com> wrote:
> Hi, Rui,
>
> I think the term "location" might be sufficiently clear in the
> specific context, e.g. SubtaskXXXInfo and TaskManagerXXXInfo. Could
> you elaborate more on what concept you think one might confuse it
> with?
>
> Best,
> Yangze Guo
>
> On Mon, Sep 18, 2023 at 12:07 PM Rui Fan <1996fan...@gmail.com> wrote:
> >
> > Hi Zhanghao,
> >
> > > Use a field named "location" (already used in
> JobExceptionsInfoWithHistory)
> > that represents TaskManager location using the newly added string
> formatter
> > method.
> >
> > How about using the taskManagerLocation instead of location
> > for all Rest response related classes?
> >
> > The taskManagerLocation may be clearer.
> >
> > Best,
> > Rui
> >
> >
> > On Mon, Sep 18, 2023 at 10:11 AM Chen Zhanghao <
> zhanghao.c...@outlook.com>
> > wrote:
> >
> > > Hi all,
> > >
> > > I've updated the FLIP to incorporate Yangze's advice:
> > >
> > > 1. Add a new string formatter method to TaskManagerLocation and
> > > ArchivedTaskManagerLocation that prints in the form of
> > > "${hostname}:${port}" to align the string formatter used by REST API.
> > > 2. Highlight that the old host field will be kept for at least 2 minor
> > > versions before removal.
> > >
> > > Best,
> > > Zhanghao Chen
> > > ________________________________
> > > 发件人: Yangze Guo <karma...@gmail.com>
> > > 发送时间: 2023年9月15日 17:26
> > > 收件人: dev@flink.apache.org <dev@flink.apache.org>
> > > 主题: Re: [DISCUSS] FLIP-363: Unify the Representation of TaskManager
> > > Location in REST API and Web UI
> > >
> > > Thanks for driving this, Zhanghao. +1 for the overall proposal.
> > >
> > > Some cents from my side:
> > >
> > > 1. Since most of the rest api get the location from
> > > TaskManagerLocation, we can align the string formatter in this class.
> > > E.g. add something like toHumanRealableString to this class.
> > >
> > > 2. From my understanding of FLIP-321, if we want to deprecate the host
> > > field, we should annotate the related field / getter / setter with
> > > @Deprecated in this version, and keep them at least 2 minor releases.
> > >
> > > Best,
> > > Yangze Guo
> > >
> > > On Wed, Sep 13, 2023 at 8:52 PM Chen Zhanghao <
> zhanghao.c...@outlook.com>
> > > wrote:
> > > >
> > > > Hi Jing,
> > > >
> > > > Thanks for the suggestion. Endpoint is indeed a more professional
> word
> > > in the networking world but I think location is more suited here for
> two
> > > reasons:
> > > >
> > > > 1. The term here is for uniquely identifying the TaskManager where
> > > the task is deployed while providing the host machine info as well to
> help
> > > identify taskmanager- and host-aggregative problems. So strictly
> speaking,
> > > it is not used in a pure networking context.
> > > > 2. The term "location" is already used widely in the codebase,
> e.g.
> > > TaskManagerLocation and JobExceptions-related classes.
> > > >
> > > > WDYT?
> > > >
> > > > Best,
> > > > Zhanghao Chen
> > > > ________________________________
> > > > 发件人: Jing Ge <j...@ververica.com.INVALID>
> > > > 发送时间: 2023年9月13日 4:52
> > > > 收件人: dev@flink.apache.org <dev@flink.apache.org>
> > > > 主题: Re: [DISCUSS] FLIP-363: Unify the Representation of TaskManager
> > > Location in REST API and Web UI
> > > >
> > > > Hi Zhanghao,
> > > >
> > > > Thanks for bringing this to our attention. It is a good proposal to
> > > improve
> > > > data consistency.
> > > >
> > > > Speaking of naming conventions of choosing location over host, how
> about
> > > > "endpoint" with the following thoughts:
> > > >
> > > > 1. endpoint is a more professional word than location in the network
> > > > context.
> > > > 2. I know commonly endpoints mean the URLs of services. Using
> > > Hostname:port
> > > > as the endpoint follows exactly the same rule, because TaskManager
> is the
> > > > top level service that aligns with the top level endpoint.
> > > >
> > > > WDYT?
> > > >
> > > > Best regards,
> > > > Jing
> > > >
> > > >
> > > > On Mon, Sep 11, 2023 at 6:01 AM Weihua Hu <huweihua....@gmail.com>
> > > wrote:
> > > >
> > > > > Hi, Zhanghao
> > > > >
> > > > > Since the meaning of "host" is not aligned, it seems good for me to
> > > remove
> > > > > it in the future release.
> > > > >
> > > > > Best,
> > > > > Weihua
> > > > >
> > > > >
> > > > > On Mon, Sep 11, 2023 at 11:48 AM Chen Zhanghao <
> > > zhanghao.c...@outlook.com>
> > > > > wrote:
> > > > >
> > > > > > Hi Fan Rui,
> > > > > >
> > > > > > Thanks for clarifying the definition of "public interfaces", that
> > > helps a
> > > > > > lot!
> > > > > >
> > > > > > Best,
> > > > > > Zhanghao Chen
> > > > > > ________________________________
> > > > > > 发件人: Rui Fan <1996fan...@gmail.com>
> > > > > > 发送时间: 2023年9月11日 11:18
> > > > > > 收件人: dev@flink.apache.org <dev@flink.apache.org>
> > > > > > 主题: Re: [DISCUSS] FLIP-363: Unify the Representation of
> TaskManager
> > > > > > Location in REST API and Web UI
> > > > > >
> > > > > > Thanks Zhanghao driving this FLIP, adding the port in Web UI
> > > > > > seems good to me.
> > > > > >
> > > > > > Hi Shammon and Zhanghao,
> > > > > >
> > > > > > I would like to clarify the difference between Public Interfaces
> > > > > > in FLIP and @Public in code.
> > > > > >
> > > > > > As I understand, the `Public Interfaces in FLIP` means these
> > > > > > changes will be used in user side, such as: @Public class,
> > > > > > Configuration settings, User-facing scripts/command-line tools,
> > > > > > and rest api, etc.
> > > > > >
> > > > > > You can refer to "What are the "public interfaces" of the
> project?"
> > > > > > part in Flink Improvement Proposals doc[1].
> > > > > >
> > > > > > @Public class means the user will use this class directly, and
> > > > > > these rest classes won't be depended on directly. So I think
> > > > > > these classes related to rest don't need to be marked @Public.
> > > > > >
> > > > > > Please correct me if anything is wrong, thanks~
> > > > > >
> > > > > > [1]
> > > > > >
> > > > > >
> > > > >
> > >
> https://cwiki.apache.org/confluence/display/FLINK/Flink+Improvement+Proposals
> > > > > >
> > > > > > Best,
> > > > > > Rui
> > > > > >
> > > > > > On Mon, Sep 11, 2023 at 11:09 AM Weihua Hu <
> huweihua....@gmail.com>
> > > > > wrote:
> > > > > >
> > > > > > > Hi, Zhanghao
> > > > > > >
> > > > > > > Thanks for bringing this proposal.
> > > > > > >
> > > > > > > I have a concern:
> > > > > > >
> > > > > > > I prefer to keep the "host" field and add a "location" field in
> > > future
> > > > > > > versions.
> > > > > > > Consider a scenario where a machine (host) with multiple
> > > TaskManagers
> > > > > has
> > > > > > > poor processing performance due to some problems.
> > > > > > > By using a host field aggregation, I can identify the problems
> with
> > > > > this
> > > > > > > machine and take it offline.
> > > > > > >
> > > > > > > Best,
> > > > > > > Weihua
> > > > > > >
> > > > > > >
> > > > > > > On Mon, Sep 11, 2023 at 10:34 AM Chen Zhanghao <
> > > > > > zhanghao.c...@outlook.com>
> > > > > > > wrote:
> > > > > > >
> > > > > > > > Hi Shammon,
> > > > > > > >
> > > > > > > > I think all REST API response messages (e.g.
> > > > > > > > SubtaskExecutionAttemptDetailsInfo) should be considered as
> part
> > > of
> > > > > the
> > > > > > > > public APIs and therefore be marked as @Public. It is true
> though
> > > > > none
> > > > > > of
> > > > > > > > them are marked as @public yet. Maybe we should do that.
> ccing
> > > > > > > > @chesnay<mailto:ches...@apache.org> for confirmation.
> > > > > > > >
> > > > > > > > Best,
> > > > > > > > Zhanghao Chen
> > > > > > > > ________________________________
> > > > > > > > 发件人: Shammon FY <zjur...@gmail.com>
> > > > > > > > 发送时间: 2023年9月11日 10:22
> > > > > > > > 收件人: dev@flink.apache.org <dev@flink.apache.org>
> > > > > > > > 主题: Re: [DISCUSS] FLIP-363: Unify the Representation of
> > > TaskManager
> > > > > > > > Location in REST API and Web UI
> > > > > > > >
> > > > > > > > Thanks Zhanghao for initialing this discussion, I have just
> one
> > > > > > comment:
> > > > > > > >
> > > > > > > > I checked the classes `SubtasksAllAccumulatorsHandler`,
> > > > > > > > `SubtasksTimesHandler`,
> `SubtaskCurrentAttemptDetailsHandler`,
> > > > > > > > `JobVertexTaskManagersHandler` and `JobExceptionsHandler` you
> > > > > mentioned
> > > > > > > in
> > > > > > > > `Public Interfaces` and they are not annotated as `Public`.
> So
> > > do you
> > > > > > > want
> > > > > > > > to annotate them as `Plublic`? If not, I think you may need
> to
> > > move
> > > > > > them
> > > > > > > > from `Public Interfaces` to `Proposed Changes`.
> > > > > > > >
> > > > > > > > Best,
> > > > > > > > Shammon FY
> > > > > > > >
> > > > > > > > On Sat, Sep 9, 2023 at 12:11 PM Chen Zhanghao <
> > > > > > zhanghao.c...@outlook.com
> > > > > > > >
> > > > > > > > wrote:
> > > > > > > >
> > > > > > > > > Hi Devs,
> > > > > > > > >
> > > > > > > > > I would like to start a discussion on FLIP-363: Unify the
> > > > > > > Representation
> > > > > > > > > of TaskManager Location in REST API and Web UI [1].
> > > > > > > > >
> > > > > > > > > The TaskManager location of subtasks is important for
> > > identifying
> > > > > > > > > TM-related problems. There are a number of places in REST
> API
> > > and
> > > > > Web
> > > > > > > UI
> > > > > > > > > where TaskManager location is returned/displayed.
> > > > > > > > >
> > > > > > > > > Problems:
> > > > > > > > >
> > > > > > > > > * Only hostname is provided to represent TaskManager
> > > location
> > > > > in
> > > > > > > some
> > > > > > > > > places (e.g. SubtaskCurrentAttemptDetailsHandler).
> However, in
> > > a
> > > > > > > > > containerized era, it is common to have multiple TMs on the
> > > same
> > > > > > host,
> > > > > > > > and
> > > > > > > > > port info is crucial to distinguish different TMs.
> > > > > > > > > * Inconsistent naming of the field to represent
> TaskManager
> > > > > > > location:
> > > > > > > > > "host" is used in most places but "location" is also used
> in
> > > > > > > > > JobExceptions-related places.
> > > > > > > > > * Inconsistent semantics of the "host" field: The
> > > semantics of
> > > > > > the
> > > > > > > > > host field are inconsistent, sometimes it denotes hostname
> only
> > > > > while
> > > > > > > in
> > > > > > > > > other times it denotes hostname + port (which is also
> > > inconsistent
> > > > > > with
> > > > > > > > the
> > > > > > > > > name of "host").
> > > > > > > > >
> > > > > > > > > We propose to improve the current situation by:
> > > > > > > > >
> > > > > > > > > * Use a field named "location" that represents
> TaskManager
> > > > > > location
> > > > > > > > in
> > > > > > > > > the form of "${hostname}:${port}" in a consistent manner
> across
> > > > > REST
> > > > > > > APIs
> > > > > > > > > and the front-end.
> > > > > > > > > * Rename the column name from "Host" to "Location" on
> the
> > > Web
> > > > > UI
> > > > > > to
> > > > > > > > > reflect the change that both hostname and port are
> displayed.
> > > > > > > > > * Keep the old "host" fields untouched for
> compatibility.
> > > They
> > > > > > can
> > > > > > > be
> > > > > > > > > removed in the next major version.
> > > > > > > > >
> > > > > > > > > Looking forward to your feedback.
> > > > > > > > >
> > > > > > > > > [1] FLIP-363: Unify the Representation of TaskManager
> Location
> > > in
> > > > > > REST
> > > > > > > > API
> > > > > > > > > and Web UI - Apache Flink - Apache Software Foundation<
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > >
> https://cwiki.apache.org/confluence/display/FLINK/FLIP-363%3A+Unify+the+Representation+of+TaskManager+Location+in+REST+API+and+Web+UI
> > > > > > > > > >
> > > > > > > > >
> > > > > > > > > Best,
> > > > > > > > > Zhanghao Chen
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > >
>
