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