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