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

Reply via email to