Hi Qingsheng, Do you have any idea what has happened in the process here? Do we know why they were changed? I was under the impression that these metric names were newly introduced due to the new interfaces and because it still depends on each connector implementing these.
Sidenote: metric names are not mentioned in the FLIP process as a public API. Might make sense to have a separate follow-up to add that to the list (I do think we should list them there). +1 for reverting this and make this change in Flink 1.16 I'm not in favour of releasing a Flink 1.15.3 with this change: I think the impact is too big for a patch version, especially given how long Flink 1.15 is already out there. Best regards, Martijn On Mon, Oct 10, 2022 at 11:13 AM Leonard Xu <xbjt...@gmail.com> wrote: > Thanks Qingsheng for starting this thread. > > +1 on reverting sink metric name and releasing 1.15.3 to fix this > inconsistent behavior. > > > Best, > Leonard > > > > > > 2022年10月10日 下午3:06，Jark Wu <imj...@gmail.com> 写道： > > Thanks for discovering this problem, Qingsheng! > > I'm also +1 for reverting the breaking changes. > > IIUC, currently, the behavior of "numXXXOut" metrics of the new and old > sink is inconsistent. > We have to break one of them to have consistent behavior. Sink V2 is an > evolving API which is just introduced in 1.15. > I think it makes sense to break the unstable API instead of the stable API > which many connectors and users depend on. > > Best, > Jark > > > > On Mon, 10 Oct 2022 at 11:36, Jingsong Li <jingsongl...@gmail.com> wrote: > >> Thanks for driving, Qingsheng. >> >> +1 for reverting sink metric name. >> >> We often forget that metric is also one of the important APIs. >> >> +1 for releasing 1.15.3 to fix this. >> >> Best, >> Jingsong >> >> On Sun, Oct 9, 2022 at 11:35 PM Becket Qin <becket....@gmail.com> wrote: >> > >> > Thanks for raising the discussion, Qingsheng, >> > >> > +1 on reverting the breaking changes. >> > >> > In addition, we might want to release a 1.15.3 to fix this and update >> the previous release docs with this known issue, so that users can upgrade >> to 1.15.3 when they hit it. It would also be good to add some backwards >> compatibility tests on metrics to avoid unintended breaking changes like >> this in the future. >> > >> > Thanks, >> > >> > Jiangjie (Becket) Qin >> > >> > On Sun, Oct 9, 2022 at 10:35 AM Qingsheng Ren <re...@apache.org> wrote: >> >> >> >> Hi devs and users, >> >> >> >> I’d like to start a discussion about reverting a breaking change about >> sink metrics made in 1.15 by FLINK-26126 [1] and FLINK-26492 [2]. >> >> >> >> TL;DR >> >> >> >> All sink metrics with name “numXXXOut” defined in FLIP-33 are replace >> by “numXXXSend” in FLINK-26126 and FLINK-26492. Considering metric names >> are public APIs, this is a breaking change to end users and not backward >> compatible. Also unfortunately this breaking change was not discussed in >> the mailing list before. >> >> >> >> Background >> >> >> >> As defined previously in FLIP-33 (the FLIP page has been changed so >> please refer to the old version [3] ), metric “numRecordsOut” is used for >> reporting the total number of output records since the sink started (number >> of records written to the external system), and similarly for >> “numRecordsOutPerSecond”, “numBytesOut”, “numBytesOutPerSecond” and >> “numRecordsOutError”. Most sinks are following this naming and definition. >> However, these metrics are ambiguous in the new Sink API as “numXXXOut” >> could be used by the output of SinkWriterOperator for reporting number of >> Committables delivered to SinkCommitterOperator. In order to resolve the >> conflict, FLINK-26126 and FLINK-26492 changed names of these metrics with >> “numXXXSend”. >> >> >> >> Necessity of reverting this change >> >> >> >> - Metric names are actually public API, as end users need to configure >> metric collecting and alerting system with metric names. Users have to >> reset all configurations related to affected metrics. >> >> - This could also affect custom and external sinks not maintained by >> Flink, which might have implemented with numXXXOut metrics. >> >> - The number of records sent to external system is way more important >> than the number of Committables sent to SinkCommitterOperator, as the >> latter one is just an internal implementation of sink. We could have a new >> metric name for the latter one instead. >> >> - We could avoid splitting the project by version (like “plz use >> numXXXOut before 1.15 and use numXXXSend after”) if we revert it ASAP, >> cosidering 1.16 is still not released for now. >> >> >> >> As a consequence, I’d like to hear from devs and users about your >> opinion on changing these metrics back to “numXXXOut”. >> >> >> >> Looking forward to your reply! >> >> >> >> [1] https://issues.apache.org/jira/browse/FLINK-26126 >> >> [2] https://issues.apache.org/jira/browse/FLINK-26492 >> >> [1] FLIP-33, version 18: >> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=211883136 >> >> >> >> Best, >> >> Qingsheng >> > >
