Thanks to everyone who discussed here, I really appreciate it. I've updated the FLIP to change all occurrences of "location" to "endpoint" instead. Given that we've reached consensus on the topic, I'll restart the voting.
Best, Zhanghao Chen ________________________________ 发件人: Rui Fan <fan...@apache.org> 发送时间: 2023年9月19日 12:16 收件人: Chen Zhanghao <zhanghao.c...@outlook.com>; dev <dev@flink.apache.org> 主题: Re: [DISCUSS] FLIP-363: Unify the Representation of TaskManager Location in REST API and Web UI +1, thanks everyone who discussed here. Best, Rui On Tue, 19 Sep 2023 at 11:41, Chen Zhanghao <zhanghao.c...@outlook.com<mailto:zhanghao.c...@outlook.com>> wrote: Hi Jing, Thanks for the clarification, I now see the point. +1 for using endpoint now. @fan...@apache.org<mailto:fan...@apache.org> WDYT? Best, Zhanghao Chen ________________________________ 发件人: Yangze Guo <karma...@gmail.com<mailto:karma...@gmail.com>> 发送时间: 2023年9月19日 11:18 收件人: dev@flink.apache.org<mailto:dev@flink.apache.org> <dev@flink.apache.org<mailto:dev@flink.apache.org>> 主题: Re: [DISCUSS] FLIP-363: Unify the Representation of TaskManager Location in REST API and Web UI Thanks for the clarification, Jing. I agree that using another term like endpoint can help us to distinguish it from the existing concept of "location". +1 for using the term endpoint and introducing TaskManagerLocation.getEndpoint(). Best, Yangze Guo On Mon, Sep 18, 2023 at 11:52 PM Jing Ge <j...@ververica.com.invalid> wrote: > > Hi Zhanghao, > > That is exactly the reason why location should not be used, because there > is a clear definition of location in Flink, e.g. TaskManagerLocation which > contains more information than hostname+port. If you think endpoint is too > generic, how about locationEndpoint? But if we build that format logic into > Location classes, it will look like > TaskManagerLocation.getLocationEndpoint() with redundant "location". > TaskManagerLocation.getEndpoint() is better. > TaskManagerLocation.getLocation(), > TaskManagerLocation.getLocationAsString(), or similar names in that > direction are even worse. > > Best regards, > Jing > > On Wed, Sep 13, 2023 at 2:52 PM Chen Zhanghao > <zhanghao.c...@outlook.com<mailto: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<mailto:dev@flink.apache.org> > > <dev@flink.apache.org<mailto: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<mailto: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<mailto: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<mailto:1996fan...@gmail.com>> > > > > 发送时间: 2023年9月11日 11:18 > > > > 收件人: dev@flink.apache.org<mailto:dev@flink.apache.org> > > > > <dev@flink.apache.org<mailto: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<mailto: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<mailto: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<mailto:zjur...@gmail.com>> > > > > > > 发送时间: 2023年9月11日 10:22 > > > > > > 收件人: dev@flink.apache.org<mailto:dev@flink.apache.org> > > > > > > <dev@flink.apache.org<mailto: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<mailto: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 > > > > > > > > > > > > > > > > > > > > > > > > > > >
