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
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
>
