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
