Hello there, Zhu: agree with the config option, great suggestion Hong: global timeout is also interesting and a good addition -- only downside I see is just another config option
If everyone is happy, I suggest we keep the discussion open until Friday and start a Vote shortly after. Cheers, Panagiotis On Tue, Apr 11, 2023 at 12:58 AM Teoh, Hong <lian...@amazon.co.uk.invalid> wrote: > Hi Panagiotis, > > Thank you for the update. Looks great! Just one suggestion below: > > 1. We seem to be waiting for the future(s) to complete before restarting > the job - should we add a configurable timeout for the enrichment? Since > each failure enricher are run in parallel, we could probably settle for 1 > timeout for all failure handlers. > 2. +1 to Zhu’s comment on adding a comma separated list of FailureHandlers > instead of boolean toggle! > > Other than the above, the FLIP looks great! Thank you for your efforts. > > Regards, > Hong > > > > > On 11 Apr 2023, at 08:01, Zhu Zhu <reed...@gmail.com> wrote: > > > > CAUTION: This email originated from outside of the organization. Do not > click links or open attachments unless you can confirm the sender and know > the content is safe. > > > > > > > > Hi Panagiotis, > > > > Thanks for updating the FLIP. > > > >> Regarding the config option > `jobmanager.failure-enricher-plugins.enabled` > > I think a config option `jobmanager.failure-enrichers`, which accepts > > the names of enrichers to use, may be better. It allows the users to > > deploy and use the plugins in a more flexible way. The default value > > of the config can be none, which means failure enrichment will be > > disabled by default. > > A reference can be the config option `metrics.reporters` which helps > > to load metric reporter plugins. > > > > Thanks, > > Zhu > > > > Panagiotis Garefalakis <pga...@apache.org> 于2023年4月10日周一 03:47写道： > >> > >> Hello again everyone, > >> > >> FLIP is now updated based on our discussion! > >> In short, FLIP-304 [1] proposes the addition of a pluggable interface > that > >> will allow users to add custom logic and enrich failures with custom > >> metadata labels. > >> While as discussed, custom restart strategies will be part of a > different > >> effort. Every pluggable FaulireEnricher: > >> > >> - Is triggered on every global/non-global failure > >> - Receives a Throwable cause and an immutable Context > >> - Performs asynchronous execution (separate IoExecutor) to avoid > >> blocking the main thread for RPCs > >> - Is completely independent from other Enrichers > >> - Emits failure labels/tags for its unique, pre-defined keys (defined > at > >> startup time) > >> > >> > >> Check the link for implementation details and please let me know what > you > >> think :) > >> > >> > >> [1] > >> > https://cwiki.apache.org/confluence/display/FLINK/FLIP-304%3A+Pluggable+Failure+Enrichers > >> > >> > >> Panagiotis > >> > >> > >> On Tue, Mar 28, 2023 at 5:01 AM Zhu Zhu <reed...@gmail.com> wrote: > >> > >>> Hi Panagiotis, > >>> > >>> How about to introduce a config option to control which error handling > >>> plugins should be used? It is more flexible for deployments. > Additionally, > >>> it can also enable users to explicitly specify the order that the > plugins > >>> take effects. > >>> > >>> Thanks, > >>> Zhu > >>> > >>> Gen Luo <luogen...@gmail.com> 于2023年3月27日周一 15:02写道： > >>>> > >>>> Thanks for the summary! > >>>> > >>>> Also +1 to support custom restart strategies in a different FLIP, > >>>> as long as we can make sure that the plugin interface won't be > >>>> changed when the restart strategy interface is introduced. > >>>> > >>>> To achieve this, maybe we should think well how the handler > >>>> would cooperate with the restart strategy, like would it executes b > >>>> efore the strategy (e.g. some strategy may use the tag), or after > >>>> it (e.g. some metric reporting handler may use the handling result). > >>>> Though we can implement in one way, and extend if the other is > >>>> really necessary by someone. > >>>> > >>>> Besides, instead of using either of the names, shall we just make > >>>> them two subclasses named FailureEnricher and FailureListener? > >>>> The former executes synchronously and can modify the context, > >>>> while the latter executes asynchronously and has a read-only view > >>>> of context. In this way we can make sure a handler behaves in > >>>> the expected way. > >>>> > >>>> > >>>> On Thu, Mar 23, 2023 at 5:19 PM Zhu Zhu <reed...@gmail.com> wrote: > >>>> > >>>>> +1 to support custom restart strategies in a different FLIP. > >>>>> > >>>>> It's fine to have a different plugin for custom restart strategy. > >>>>> If so, since we do not treat the FLIP-304 plugin as a common failure > >>>>> handler, but instead mainly targets to add labels to errors, I would > >>>>> +1 for the name `FailureEnricher`. > >>>>> > >>>>> Thanks, > >>>>> Zhu > >>>>> > >>>>> David Morávek <d...@apache.org> 于2023年3月23日周四 15:51写道： > >>>>>> > >>>>>>> > >>>>>>> One additional remark on introducing it as an async operation: We > >>> would > >>>>>>> need a new configuration parameter to define the timeout for such a > >>>>>>> listener call, wouldn't we? > >>>>>>> > >>>>>> > >>>>>> This could be left up to the implementor to handle. > >>>>>> > >>>>>> What about adding an extra method getNamespace() to the Listener > >>>>> interface > >>>>>>> which returns an Optional<String>. > >>>>>>> > >>>>>> > >>>>>> I'd avoid mixing an additional concept into this. We can simply have > >>> a > >>>>> new > >>>>>> method that returns a set of keys the listener can output. We can > >>>>> validate > >>>>>> this at the JM startup time and fail fast (since it's a > configuration > >>>>>> error) if there is an overlap. If the listener outputs the key that > >>> is > >>>>> not > >>>>>> allowed to, I wouldn't be afraid to call into a fatal error handler > >>> since > >>>>>> it's an invalid implementation. > >>>>>> > >>>>>> Best, > >>>>>> D. > >>>>>> > >>>>>> On Thu, Mar 23, 2023 at 8:34 AM Matthias Pohl > >>>>>> <matthias.p...@aiven.io.invalid> wrote: > >>>>>> > >>>>>>> Sounds good. Two points I want to add: > >>>>>>> > >>>>>>> - Listener execution should be independent — however we need a > >>> way > >>>>> to > >>>>>>>> enforce a Label key/key-prefix is only assigned to a single > >>> Listener, > >>>>>>>> thinking of a validation step both at Listener init and runtime > >>>>> stages > >>>>>>>> > >>>>>>> What about adding an extra method getNamespace() to the Listener > >>>>> interface > >>>>>>> which returns an Optional<String>. Therefore, the > >>> implementation/the > >>>>> user > >>>>>>> can decide depending on the use case whether it's necessary to have > >>>>>>> separate namespaces for the key/value pairs or not. On the Flink > >>> side, > >>>>> we > >>>>>>> would just merge the different maps considering their namespaces. > >>>>>>> > >>>>>>> A flaw of this approach is that if a user decides to use the same > >>>>> namespace > >>>>>>> for multiple listeners, how is an error in one of the listeners > >>>>> represented > >>>>>>> in the outcome? We would have to overwrite either the successful > >>>>> listener's > >>>>>>> result or the failed ones. I wanted to share it, anyway. > >>>>>>> > >>>>>>> One additional remark on introducing it as an async operation: We > >>> would > >>>>>>> need a new configuration parameter to define the timeout for such a > >>>>>>> listener call, wouldn't we? > >>>>>>> > >>>>>>> Matthias > >>>>>>> > >>>>>>> On Wed, Mar 22, 2023 at 4:56 PM Panagiotis Garefalakis < > >>>>> pga...@apache.org> > >>>>>>> wrote: > >>>>>>> > >>>>>>>> Hi everyone, > >>>>>>>> > >>>>>>>> > >>>>>>>> Thanks for the valuable comments! > >>>>>>>> Excited to see this is an area of interest for the community! > >>>>>>>> > >>>>>>>> Summarizing some of the main points raised along with my > >>> thoughts: > >>>>>>>> > >>>>>>>> - Labels (Key/Value) pairs are more expressive than Tags > >>>>> (Strings) so > >>>>>>>> using the former is a good idea — I am also debating if we > >>> want to > >>>>>>>> return > >>>>>>>> multiple KV pairs per Listener (one could argue that we could > >>>>> split > >>>>>>> the > >>>>>>>> logic in multiple Listeners to support that) > >>>>>>>> - An immutable context along with data returned using the > >>>>> interface > >>>>>>>> method implementations is a better approach than a mutable > >>>>> Collection > >>>>>>>> - Listener execution should be independent — however we need a > >>>>> way to > >>>>>>>> enforce a Label key/key-prefix is only assigned to a single > >>>>> Listener, > >>>>>>>> thinking of a validation step both at Listener init and > >>> runtime > >>>>> stages > >>>>>>>> - We want to perform async Listener operations as sync could > >>>>> block the > >>>>>>>> main thread — exposing an ioExecutor pool through the context > >>>>> could be > >>>>>>>> an > >>>>>>>> elegant solution here > >>>>>>>> - Make sure Listener errors are not failing jobs — make sure > >>> to > >>>>> log > >>>>>>> and > >>>>>>>> keep the job alive > >>>>>>>> - We need better naming / public interface > >>> separation/description > >>>>>>>> > >>>>>>>> - Even though custom restart strategies share some > >>>>> properties > >>>>>>> with > >>>>>>>> Listeners, they would probably need a separate interface with a > >>>>> different > >>>>>>>> return type anyway (restart strategy not labels) and in general > >>> they > >>>>> are > >>>>>>>> different and complex enough to justify their own FLIP (that can > >>>>> also be > >>>>>>> a > >>>>>>>> follow-up). > >>>>>>>> > >>>>>>>> > >>>>>>>> What do people think? I am planning to modify the FLIP to reflect > >>>>> these > >>>>>>>> changes if they make sense to everyone. > >>>>>>>> > >>>>>>>> Cheers, > >>>>>>>> Panagiotis > >>>>>>>> > >>>>>>>> On Wed, Mar 22, 2023 at 6:28 AM Hong Teoh <hlteo...@gmail.com> > >>>>> wrote: > >>>>>>>> > >>>>>>>>> Hi all, > >>>>>>>>> > >>>>>>>>> Thank you Panagiotis for proposing this. From the size of the > >>>>> thread, > >>>>>>>> this > >>>>>>>>> is a much needed feature in Flink! > >>>>>>>>> Some thoughts, to extend those already adeptly summarised by > >>> Piotr, > >>>>>>>>> Matthias and Jing. > >>>>>>>>> > >>>>>>>>> - scope of FLIP: +1 to scoping this FLIP to observability > >>> around a > >>>>>>>>> restart. That would include adding metadata + exposing > >>> metadata to > >>>>>>>> external > >>>>>>>>> systems. IMO, introducing a new restart strategy solves > >>> different > >>>>>>>> problems, > >>>>>>>>> is much larger scope and should be covered in a separate FLIP. > >>>>>>>>> > >>>>>>>>> - failure handling: At the moment, we propose transitioning the > >>>>> Flink > >>>>>>> job > >>>>>>>>> to a terminal FAILED state when JobListener fails, when the job > >>>>> could > >>>>>>>> have > >>>>>>>>> transitioned to RESTARTING->RUNNING. If we are keeping in line > >>>>> with the > >>>>>>>>> scope to add metadata/observability around job restarts, we > >>> should > >>>>> not > >>>>>>> be > >>>>>>>>> affecting the running of the Flink job itself. Could I propose > >>> we > >>>>>>> instead > >>>>>>>>> log WARN/ERROR. > >>>>>>>>> > >>>>>>>>> - immutable context: +1 to keeping the contract clear via > >>> return > >>>>> types. > >>>>>>>>> - async operation: +1 to adding ioexecutor to context, however, > >>>>> given > >>>>>>> we > >>>>>>>>> don’t want to block the actual job restart on adding metadata / > >>>>> calling > >>>>>>>>> external services, should we consider returning and letting > >>> futures > >>>>>>>>> complete independently? > >>>>>>>>> > >>>>>>>>> - independent vs ordered execution: Should we consider making > >>> the > >>>>> order > >>>>>>>> of > >>>>>>>>> execution deterministic (use a List instead of Set)? > >>>>>>>>> > >>>>>>>>> > >>>>>>>>> Once again, thank you for working on this. > >>>>>>>>> > >>>>>>>>> Regards, > >>>>>>>>> Hong > >>>>>>>>> > >>>>>>>>> > >>>>>>>>>> On 21 Mar 2023, at 21:07, Jing Ge <j...@ververica.com.INVALID > >>>> > >>>>>>> wrote: > >>>>>>>>>> > >>>>>>>>>> Hi, > >>>>>>>>>> > >>>>>>>>>> Thanks Panagiotis for this FLIP and thanks for all valuable > >>>>>>>> discussions. > >>>>>>>>>> I'd like to share my two cents: > >>>>>>>>>> > >>>>>>>>>> - FailureListenerContext#addTag and > >>>>> FailureListenerContext#getTags. > >>>>>>> It > >>>>>>>>>> seems that we have to call getTags() and then do remove > >>>>> activities if > >>>>>>>> we > >>>>>>>>>> want to delete any tags (according to the javadoc in the > >>> FLIP). > >>>>> It > >>>>>>> is > >>>>>>>>>> inconsistent for me too. Either offer addTag(), deleteTag(), > >>> and > >>>>> let > >>>>>>>>>> getTags() return immutable collection, or offer getTags() > >>> only to > >>>>>>>> return > >>>>>>>>>> mutable collection. > >>>>>>>>>> > >>>>>>>>>> - label vs tag. Label is a great idea +1. AFAIC, tag could > >>> be a > >>>>>>> special > >>>>>>>>>> case of label, i.e. key="tag". It is convenient to offer the > >>>>> xxxTag() > >>>>>>>>>> method if the user only needs one label. I would love to have > >>>>> both of > >>>>>>>>> them. > >>>>>>>>>> Another thought is that tag implicitly contains the meaning > >>> of > >>>>>>>>> "immutable". > >>>>>>>>>> > >>>>>>>>>> - +1 for a separate FLIP of customized restart strategy. > >>>>> Attention > >>>>>>>> should > >>>>>>>>>> be taken to make sure it works well with Flink built-in > >>>>>>> restartStrategy > >>>>>>>>> in > >>>>>>>>>> order to have the single source of truth. > >>>>>>>>>> > >>>>>>>>>> - execution order. The default independent execution should > >>> be > >>>>> fine. > >>>>>>>>>> According to the FailureListener interface definition in the > >>>>> FLIP, > >>>>>>>> users > >>>>>>>>>> should be able to easily build a listener chain[1] to offer > >>>>>>> sequential > >>>>>>>>>> execution, e.g. public FailureListener(FailureListener > >>>>> nextListener). > >>>>>>>>>> Another option is to modify the interface or provide another > >>>>>>> interface > >>>>>>>>>> alongside the current one to extend the method to support > >>>>>>>> ListenerChain, > >>>>>>>>>> i.e. void onFailure(Throwable cause, FailureListenerContext > >>>>> context, > >>>>>>>>>> ListenerChain listenerChain). Users can also mix them up. > >>>>>>>>>> > >>>>>>>>>> - naming. Afaiu, the pluggable extension is not limited to > >>>>> failure > >>>>>>>>>> enrichment. Conceptually it can do everything for the given > >>>>> failure, > >>>>>>>> e.g. > >>>>>>>>>> start counting metric as the FLIP described, calling an > >>> external > >>>>>>>> system, > >>>>>>>>>> sending notification to slack channel, etc. you name it. It > >>>>> sounds to > >>>>>>>> me > >>>>>>>>>> more like a FailureActionListener - it can trigger actions > >>> based > >>>>> on > >>>>>>>>>> failure. Failure enrichment is one type of action. > >>>>>>>>>> > >>>>>>>>>> Best regards, > >>>>>>>>>> Jing > >>>>>>>>>> > >>>>>>>>>> [1] > >>>>> https://en.wikipedia.org/wiki/Chain-of-responsibility_pattern > >>>>>>>>>> > >>>>>>>>>> On Tue, Mar 21, 2023 at 3:39 PM Matthias Pohl > >>>>>>>>>> <matthias.p...@aiven.io.invalid> wrote: > >>>>>>>>>> > >>>>>>>>>>> Thanks for the proposal, Panagiotis. A lot of good points > >>> have > >>>>> been > >>>>>>>>> already > >>>>>>>>>>> shared. I just want to add my view on some of the items: > >>>>>>>>>>> > >>>>>>>>>>> - independent execution vs ordered execution: I prefer the > >>>>> listeners > >>>>>>>>> being > >>>>>>>>>>> processed independently from each other because it adds less > >>>>>>>> complexity > >>>>>>>>>>> code-wise. The use case Piotr described (where you want to > >>> reuse > >>>>>>> some > >>>>>>>>> other > >>>>>>>>>>> classifier) is the only one I can think of where we actually > >>>>> need > >>>>>>>>>>> classifiers depending on each other. Supporting such a use > >>> case > >>>>>>> right > >>>>>>>>> from > >>>>>>>>>>> the start feels a bit over-engineered and could be covered > >>> in a > >>>>>>>>> follow-up > >>>>>>>>>>> FLIP if we really come to that point where such a feature is > >>>>>>> requested > >>>>>>>>> by > >>>>>>>>>>> users. > >>>>>>>>>>> > >>>>>>>>>>> - key/value pairs instead of plain labels: I think that's a > >>> good > >>>>>>> idea. > >>>>>>>>>>> key/value pairs are more expressive. +1 > >>>>>>>>>>> > >>>>>>>>>>> - extending the FLIP to cover restart strategy: I understand > >>>>> Gen's > >>>>>>>>> concern > >>>>>>>>>>> about introducing too many different types of plugins. But I > >>>>> would > >>>>>>>> still > >>>>>>>>>>> favor not extending the FLIP in this regard. A pluggable > >>> restart > >>>>>>>>> strategy > >>>>>>>>>>> sounds reasonable. But an error classifier and a restart > >>>>> strategy > >>>>>>> are > >>>>>>>>> still > >>>>>>>>>>> different enough to justify separate plugins, IMHO. And > >>>>> therefore, I > >>>>>>>>> would > >>>>>>>>>>> think that covering the restart strategy in a separate FLIP > >>> is > >>>>> the > >>>>>>>>> better > >>>>>>>>>>> option for the sake of simplicity. > >>>>>>>>>>> > >>>>>>>>>>> - immutable context: Passing in an immutable context and > >>>>> returning > >>>>>>>> data > >>>>>>>>>>> through the interface method's return value sounds like a > >>> better > >>>>>>>>> approach > >>>>>>>>>>> to harden the contract of the interface. +1 for that > >>> proposal > >>>>>>>>>>> > >>>>>>>>>>> - async operation: I think David is right. An async > >>> interface > >>>>> makes > >>>>>>>> the > >>>>>>>>>>> listener implementations more robust when it comes to heavy > >>> IO > >>>>>>>>> operations. > >>>>>>>>>>> The ioExecutor can be passed through the context object. +1 > >>>>>>>>>>> > >>>>>>>>>>> Matthias > >>>>>>>>>>> > >>>>>>>>>>> On Tue, Mar 21, 2023 at 2:09 PM David Morávek < > >>>>>>>> david.mora...@gmail.com> > >>>>>>>>>>> wrote: > >>>>>>>>>>> > >>>>>>>>>>>> *@Piotr* > >>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>>>>>> I was thinking about actually defining the order of the > >>>>>>>>>>>>> classifiers/handlers and not allowing them to be > >>> asynchronous. > >>>>>>>>>>>>> Asynchronousity would create some problems: when to > >>> actually > >>>>>>> return > >>>>>>>>> the > >>>>>>>>>>>>> error to the user? After all async responses will get > >>> back? > >>>>>>> Before, > >>>>>>>>> but > >>>>>>>>>>>>> without classified exception? It would also add > >>> implementation > >>>>>>>>>>> complexity > >>>>>>>>>>>>> and I think we can always expand the API with async > >>> version > >>>>> in the > >>>>>>>>>>> future > >>>>>>>>>>>>> if needed. > >>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>>>>> As long as the classifiers need to talk to an external > >>> system, > >>>>> we > >>>>>>> by > >>>>>>>>>>>> definition need to allow them to be asynchronous to > >>> unblock the > >>>>>>> main > >>>>>>>>>>> thread > >>>>>>>>>>>> for handling other RPCs. Exposing ioExecutor via the > >>> context > >>>>>>> proposed > >>>>>>>>>>> above > >>>>>>>>>>>> would be great. > >>>>>>>>>>>> > >>>>>>>>>>>> After all async responses will get back > >>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>>>>> This would be the same if we trigger them synchronously > >>> one by > >>>>> one, > >>>>>>>>> with > >>>>>>>>>>> a > >>>>>>>>>>>> caveat that synchronous execution might take significantly > >>>>> longer > >>>>>>> and > >>>>>>>>>>>> introduce unnecessary downtime to a job. > >>>>>>>>>>>> > >>>>>>>>>>>> D. > >>>>>>>>>>>> > >>>>>>>>>>>> On Tue, Mar 21, 2023 at 1:12 PM Zhu Zhu <reed...@gmail.com > >>>> > >>>>> wrote: > >>>>>>>>>>>> > >>>>>>>>>>>>> Hi Piotr, > >>>>>>>>>>>>> > >>>>>>>>>>>>> It's fine to me to have a separate FLIP to extend this > >>>>>>>>>>> `FailureListener` > >>>>>>>>>>>>> to support custom restart strategy. > >>>>>>>>>>>>> > >>>>>>>>>>>>> What I was a bit concerned is that if we just treat the > >>>>>>>>>>> `FailureListener` > >>>>>>>>>>>>> as an error classifier which is not crucial to Flink > >>> framework > >>>>>>>>> process, > >>>>>>>>>>>>> we may design it to run asynchronously and not trigger > >>> Flink > >>>>>>>> failures. > >>>>>>>>>>>>> This may be a blocker if later we want to enable it to > >>> support > >>>>>>>> custom > >>>>>>>>>>>>> restart strategy. > >>>>>>>>>>>>> > >>>>>>>>>>>>> Thanks, > >>>>>>>>>>>>> Zhu > >>>>>>>>>>>>> > >>>>>>>>>>>>> Dian Fu <dian0511...@gmail.com> 于2023年3月21日周二 19:53写道： > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> Hi Panagiotis, > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> Thanks for the proposal. This is a very valuable feature > >>> and > >>>>> will > >>>>>>>> be > >>>>>>>>>>> a > >>>>>>>>>>>>> good > >>>>>>>>>>>>>> add-on for Flink. > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> I also think that it will be great if we can consider > >>> how to > >>>>> make > >>>>>>>> it > >>>>>>>>>>>>>> possible for users to customize the failure handling in > >>> this > >>>>>>> FLIP. > >>>>>>>>>>> It's > >>>>>>>>>>>>>> highly related to the problem we want to address in this > >>>>> FLIP and > >>>>>>>>>>> could > >>>>>>>>>>>>>> avoid refactoring the interfaces proposed in this FLIP > >>> too > >>>>>>> quickly. > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> Currently it treats all kinds of exceptions the same. > >>>>> However, > >>>>>>> some > >>>>>>>>>>>> kinds > >>>>>>>>>>>>>> of exceptions are actually not recoverable at all. It > >>> could > >>>>> let > >>>>>>>> users > >>>>>>>>>>>> to > >>>>>>>>>>>>>> customize the failure handling logic to fail fast for > >>> certain > >>>>>>> known > >>>>>>>>>>>>>> unrecoverable exceptions and finally make these kinds of > >>>>> jobs get > >>>>>>>>>>>> noticed > >>>>>>>>>>>>>> and recoveried more quickly. > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> Regards, > >>>>>>>>>>>>>> Dian > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> On Tue, Mar 21, 2023 at 4:36 PM Gen Luo < > >>> luogen...@gmail.com > >>>>>> > >>>>>>>> wrote: > >>>>>>>>>>>>>> > >>>>>>>>>>>>>>> Hi Panagiotis, > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> Thanks for the proposal. > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> It's useful to enrich the information so that users can > >>> be > >>>>> more > >>>>>>>>>>>>>>> clear why the job is failing, especially platform > >>>>> developers who > >>>>>>>>>>>>>>> need to provide the information to their end users. > >>>>>>>>>>>>>>> And for the very FLIP, I'd prefer the naming > >>>>> `FailureEnricher` > >>>>>>>>>>>>>>> proposed by David, as the plugin doesn't really handle > >>> the > >>>>>>>> failure. > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> However, like Zhu and Lijie said, I also joined a > >>> discussion > >>>>>>>>>>>>>>> recently about customized failure handling, e.g. > >>> counting > >>>>> the > >>>>>>>>>>>>>>> failure rate of pipeline regions separately, and failing > >>>>> the job > >>>>>>>>>>>>>>> when a specific error occurs, and so on. > >>>>>>>>>>>>>>> I suppose a custom restart strategy, or I'd call it a > >>> custom > >>>>>>>>>>>>>>> failure "handler", is indeed necessary. It can also > >>> enrich > >>>>> the > >>>>>>>>>>>>>>> information as the current proposed handler does. > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> To avoid adding too many plugin interfaces which may > >>> confuse > >>>>>>> users > >>>>>>>>>>>>>>> and make the ExecutionFailureHandler more complex, > >>>>>>>>>>>>>>> I think it'd be better to consider the requirements at > >>> the > >>>>> same > >>>>>>>>>>> time. > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> IMO, we can add a handler interface, then make the > >>> current > >>>>>>> restart > >>>>>>>>>>>>>>> strategy and the enricher both types of the handler. The > >>>>>>> handlers > >>>>>>>>>>>>>>> execute in sequence, and the failure is considered > >>>>> unrecoverable > >>>>>>>> if > >>>>>>>>>>>>>>> any of the handlers decides. > >>>>>>>>>>>>>>> In this way, users can also implement a handler using > >>> the > >>>>>>> enriched > >>>>>>>>>>>>>>> information provided by the previous handlers, e.g. fail > >>>>> the job > >>>>>>>>>>> and > >>>>>>>>>>>>>>> send a notification if too many failures are caused by > >>> the > >>>>> end > >>>>>>>>>>> users. > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> Best, > >>>>>>>>>>>>>>> Gen > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> On Tue, Mar 21, 2023 at 11:38 AM Weihua Hu < > >>>>>>>> huweihua....@gmail.com > >>>>>>>>>>>> > >>>>>>>>>>>>> wrote: > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> Hi Panagiotis, > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> Thanks for your proposal. It is valuable to analyze the > >>>>> reason > >>>>>>>>>>> for > >>>>>>>>>>>>>>>> failure with the user plug-in. > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> Making the context immutable could make the contract > >>>>> stronger. > >>>>>>>>>>>>>>>> Letting the listener return an enriching result may be > >>> a > >>>>> better > >>>>>>>>>>>> way. > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> IIUC, listeners could do two things, enrich more > >>>>> information > >>>>>>>>>>>>>>> (tags/labels) > >>>>>>>>>>>>>>>> to FailureHandlingResult, and push data out of Flink > >>>>> (metrics > >>>>>>> or > >>>>>>>>>>>>>>>> something). > >>>>>>>>>>>>>>>> IMO, we could split these two types into Listener and > >>>>> Advisor > >>>>>>>>>>>> (maybe > >>>>>>>>>>>>>>>> other names). The Listener just pushes the data out and > >>>>> returns > >>>>>>>>>>>>> nothing > >>>>>>>>>>>>>>> to > >>>>>>>>>>>>>>>> Flink, so we can run these async and don't have to > >>> wait for > >>>>>>>>>>>>> Listener's > >>>>>>>>>>>>>>>> result. > >>>>>>>>>>>>>>>> The Advisor returns rich information to the > >>>>>>> FailureHadingResult, > >>>>>>>>>>>>> and it > >>>>>>>>>>>>>>>> should > >>>>>>>>>>>>>>>> have a lighter logic. > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> Supporting a custom restart strategy is also valuable. > >>> In > >>>>> this > >>>>>>>>>>>>> design, we > >>>>>>>>>>>>>>>> use > >>>>>>>>>>>>>>>> RestartStrategy to construct a FailureHandingResult, > >>> and > >>>>> then > >>>>>>>>>>> pass > >>>>>>>>>>>>> it to > >>>>>>>>>>>>>>>> Listener. > >>>>>>>>>>>>>>>> My question is, should we change the restart strategy > >>>>> interface > >>>>>>>>>>> to > >>>>>>>>>>>>>>> support > >>>>>>>>>>>>>>>> the > >>>>>>>>>>>>>>>> custom restart strategy, or keep the current restart > >>>>> strategy > >>>>>>> and > >>>>>>>>>>>>> let the > >>>>>>>>>>>>>>>> later > >>>>>>>>>>>>>>>> Listener enrich the restartable information to > >>>>>>>>>>>> FailureHandingResult? > >>>>>>>>>>>>> The > >>>>>>>>>>>>>>>> latter > >>>>>>>>>>>>>>>> may cause some confusion when we use a custom restart > >>>>> strategy. > >>>>>>>>>>>>>>>> The default flink restart strategy also runs but does > >>> not > >>>>> take > >>>>>>>>>>>>> effect. > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> Best, > >>>>>>>>>>>>>>>> Weihua > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> On Mon, Mar 20, 2023 at 11:42 PM Lijie Wang < > >>>>>>>>>>>>> wangdachui9...@gmail.com> > >>>>>>>>>>>>>>>> wrote: > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> Hi Panagiotis, > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> Thanks for driving this. > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> +1 for supporting custom restart strategy, we did > >>> receive > >>>>> such > >>>>>>>>>>>>> requests > >>>>>>>>>>>>>>>>> from the user mailing list [1][2]. > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> Besides, in current design, the plugin will only do > >>> some > >>>>>>>>>>>>> statistical > >>>>>>>>>>>>>>> and > >>>>>>>>>>>>>>>>> classification work, and will not affect the > >>>>>>>>>>>>> *FailureHandlingResult*. > >>>>>>>>>>>>>>>> Just > >>>>>>>>>>>>>>>>> listening, no handling, it doesn't quite match the > >>> title. > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> [1] > >>>>>>>>>>>>> > >>>>> https://lists.apache.org/thread/ch3s4jhh09wnff3tscqnb6btp2zlp2r1 > >>>>>>>>>>>>>>>>> [2] > >>>>>>>>>>>>> > >>>>> https://lists.apache.org/thread/lwjfdr7c1ypo77r4rwojdk7kxx2sw4sx > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> Best, > >>>>>>>>>>>>>>>>> Lijie > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> Zhu Zhu <reed...@gmail.com> 于2023年3月20日周一 21:39写道： > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>> Hi Panagiotis, > >>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>> Thanks for creating this proposal! It's good to > >>> enable > >>>>> Flink > >>>>>>>>>>> to > >>>>>>>>>>>>>>> handle > >>>>>>>>>>>>>>>>>> different errors in different ways, through a > >>> pluggable > >>>>> way. > >>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>> There are requests for flexible restart strategies > >>> from > >>>>> time > >>>>>>>>>>> to > >>>>>>>>>>>>> time, > >>>>>>>>>>>>>>>> for > >>>>>>>>>>>>>>>>>> different strategies of restart backoff time, or to > >>>>> suppress > >>>>>>>>>>>>>>> restarting > >>>>>>>>>>>>>>>>>> on certain errors. Therefore, I think it's better > >>> that > >>>>> the > >>>>>>>>>>>>> proposed > >>>>>>>>>>>>>>>>>> failure handling plugin can also support custom > >>> restart > >>>>>>>>>>>>> strategies. > >>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>> Maybe we can call it FailureHandlingAdvisor which > >>>>> provides > >>>>>>>>>>> more > >>>>>>>>>>>>>>>>>> information (labels) and gives advice (restart > >>> backoff > >>>>> time, > >>>>>>>>>>>>> whether > >>>>>>>>>>>>>>>>>> to restart)? I do not have a strong opinion though, > >>> any > >>>>>>>>>>>>> explanatory > >>>>>>>>>>>>>>>>>> name would be good. > >>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>> To avoid unexpected mutation, how about to make the > >>>>> context > >>>>>>>>>>>>> immutable > >>>>>>>>>>>>>>>>>> and let the plugin return an immutable result? i.e. > >>>>> remove > >>>>>>>>>>> the > >>>>>>>>>>>>>>> setters > >>>>>>>>>>>>>>>>>> from the context, and let the plugin method return a > >>>>> result > >>>>>>>>>>>> which > >>>>>>>>>>>>>>>>>> contains `labels`, `canRestart` and > >>> `restartBackoffTime`. > >>>>>>>>>>> Flink > >>>>>>>>>>>>>>> should > >>>>>>>>>>>>>>>>>> apply the result to the context before invoking the > >>> next > >>>>>>>>>>>> plugin, > >>>>>>>>>>>>> so > >>>>>>>>>>>>>>>>>> that the next plugin will see the updated context. > >>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>> The plugin should avoid taking too much time to > >>> return > >>>>> the > >>>>>>>>>>>>> result, > >>>>>>>>>>>>>>>>> because > >>>>>>>>>>>>>>>>>> it will block the RPC and result in instability. > >>>>> However, it > >>>>>>>>>>>> can > >>>>>>>>>>>>>>> still > >>>>>>>>>>>>>>>>>> perform heavy actions in a different thread. The > >>> context > >>>>> can > >>>>>>>>>>>>> provide > >>>>>>>>>>>>>>> an > >>>>>>>>>>>>>>>>>> `ioExecutor` to the plugins for reuse. > >>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>> Thanks, > >>>>>>>>>>>>>>>>>> Zhu > >>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>> Shammon FY <zjur...@gmail.com> 于2023年3月20日周一 > >>> 20:21写道： > >>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>> Hi Panagiotis > >>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>> Thank you for your answer. I agree that > >>>>> `FailureListener` > >>>>>>>>>>>>> could be > >>>>>>>>>>>>>>>>>>> stateless, then I have some thoughts as follows > >>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>> 1. I see that listeners and tag collections are > >>>>> associated. > >>>>>>>>>>>>> When > >>>>>>>>>>>>>>>>>> JobManager > >>>>>>>>>>>>>>>>>>> fails and restarts, how can the new listener be > >>>>> associated > >>>>>>>>>>>>> with the > >>>>>>>>>>>>>>>> tag > >>>>>>>>>>>>>>>>>>> collection before failover? Is the listener loading > >>>>> order? > >>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>> 2. The tag collection may be too large, resulting > >>> in the > >>>>>>>>>>>>> JobManager > >>>>>>>>>>>>>>>>> OOM, > >>>>>>>>>>>>>>>>>> do > >>>>>>>>>>>>>>>>>>> we need to provide a management class that supports > >>> some > >>>>>>>>>>>>>>> obsolescence > >>>>>>>>>>>>>>>>>>> strategies instead of a direct Collection? > >>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>> 3. Is it possible to provide a more complex data > >>>>> structure > >>>>>>>>>>>>> than a > >>>>>>>>>>>>>>>>> simple > >>>>>>>>>>>>>>>>>>> string collection for tags in listeners, such as > >>>>> key-value? > >>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>> Best, > >>>>>>>>>>>>>>>>>>> Shammon FY > >>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>> On Mon, Mar 20, 2023 at 7:48 PM Leonard Xu < > >>>>>>>>>>>> xbjt...@gmail.com> > >>>>>>>>>>>>>>>> wrote: > >>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>> Hi，Panagiotis > >>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>> Thank you for kicking off this discussion. > >>> Overall, the > >>>>>>>>>>>>> proposed > >>>>>>>>>>>>>>>>>> feature of > >>>>>>>>>>>>>>>>>>>> this FLIP makes sense to me. We have also discussed > >>>>>>>>>>> similar > >>>>>>>>>>>>>>>>>> requirements > >>>>>>>>>>>>>>>>>>>> with our users and developers, and I believe it > >>> will > >>>>> help > >>>>>>>>>>>>> many > >>>>>>>>>>>>>>>> users. > >>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>> In terms of FLIP content, I have some thoughts: > >>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>> (1) For the FailureListenerContextget interface, > >>> the > >>>>>>>>>>>> methods > >>>>>>>>>>>>>>>>>>>> FailureListenerContext#addTag and > >>>>>>>>>>>>> FailureListenerContextgetTags > >>>>>>>>>>>>>>>> looks > >>>>>>>>>>>>>>>>>> very > >>>>>>>>>>>>>>>>>>>> inconsistent because they imply specific > >>> implementation > >>>>>>>>>>>>> details, > >>>>>>>>>>>>>>>> and > >>>>>>>>>>>>>>>>>> not > >>>>>>>>>>>>>>>>>>>> all FailureListeners need to handle them, we > >>> shouldn't > >>>>>>>>>>> put > >>>>>>>>>>>>> them > >>>>>>>>>>>>>>> in > >>>>>>>>>>>>>>>>> the > >>>>>>>>>>>>>>>>>>>> interface. Minor: The comment "UDF loading" in the > >>>>>>>>>>>>>>>>> getUserClassLoader() > >>>>>>>>>>>>>>>>>>>> method looks like a typo, IIUC it should return the > >>>>>>>>>>>>> classloader > >>>>>>>>>>>>>>> of > >>>>>>>>>>>>>>>>> the > >>>>>>>>>>>>>>>>>>>> current job. > >>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>> (2) Regarding the implementation in > >>>>>>>>>>>>>>>>>> ExecutionFailureHandler#handleFailure, > >>>>>>>>>>>>>>>>>>>> some custom listeners may have heavy IO operations, > >>>>> such > >>>>>>>>>>> as > >>>>>>>>>>>>>>>> reporting > >>>>>>>>>>>>>>>>>> to > >>>>>>>>>>>>>>>>>>>> their monitoring system. The current logic appears > >>> to > >>>>> be > >>>>>>>>>>>>>>> processing > >>>>>>>>>>>>>>>>> in > >>>>>>>>>>>>>>>>>> the > >>>>>>>>>>>>>>>>>>>> JobMaster's main thread, and it is recommended not > >>> to > >>>>> do > >>>>>>>>>>>> this > >>>>>>>>>>>>>>> kind > >>>>>>>>>>>>>>>> of > >>>>>>>>>>>>>>>>>>>> processing in the main thread. > >>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>> (3) The results of FailureListener's processing > >>> and the > >>>>>>>>>>>>>>>>>>>> FailureHandlingResult returned by > >>>>> ExecutionFailureHandler > >>>>>>>>>>>>> are not > >>>>>>>>>>>>>>>>>> related. > >>>>>>>>>>>>>>>>>>>> I think these two are closely related, the > >>> motivation > >>>>> of > >>>>>>>>>>>> this > >>>>>>>>>>>>>>> FLIP > >>>>>>>>>>>>>>>> is > >>>>>>>>>>>>>>>>>> to > >>>>>>>>>>>>>>>>>>>> make current failure handling more flexible. From > >>> this > >>>>>>>>>>>>>>> perspective, > >>>>>>>>>>>>>>>>>>>> different listeners should have the opportunity to > >>>>> affect > >>>>>>>>>>>> the > >>>>>>>>>>>>>>> job's > >>>>>>>>>>>>>>>>>> failure > >>>>>>>>>>>>>>>>>>>> handling flow. For example, a Flink job is > >>> configured > >>>>>>>>>>> with > >>>>>>>>>>>> a > >>>>>>>>>>>>>>>>>>>> RestartStrategy with huge numbers retry , but the > >>> Kafka > >>>>>>>>>>>>> topic of > >>>>>>>>>>>>>>>>>> Source has > >>>>>>>>>>>>>>>>>>>> been deleted, the job will failover continuously. > >>> In > >>>>> this > >>>>>>>>>>>>> case, > >>>>>>>>>>>>>>> the > >>>>>>>>>>>>>>>>>> user > >>>>>>>>>>>>>>>>>>>> should have their listener to determine whether > >>> this > >>>>>>>>>>>> failure > >>>>>>>>>>>>> is > >>>>>>>>>>>>>>>>>> recoverable > >>>>>>>>>>>>>>>>>>>> or unrecoverable, and then wrap the processing > >>> result > >>>>>>>>>>> into > >>>>>>>>>>>>>>>>>>>> FailureHandlingResult.unrecoverable(xx) and pass > >>> it to > >>>>>>>>>>>>> JobMaster, > >>>>>>>>>>>>>>>>> this > >>>>>>>>>>>>>>>>>>>> approach will be more flexible. > >>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>> (4) All FLIPs have an important section named > >>> Public > >>>>>>>>>>>>> Interfaces. > >>>>>>>>>>>>>>>>>> Current > >>>>>>>>>>>>>>>>>>>> FLIP mixes the interface section and the > >>> implementation > >>>>>>>>>>>>> section > >>>>>>>>>>>>>>>>>> together. > >>>>>>>>>>>>>>>>>>>> It is better for us to refer to the FLIP > >>> template[1] > >>>>> and > >>>>>>>>>>>>> separate > >>>>>>>>>>>>>>>>> them, > >>>>>>>>>>>>>>>>>>>> this will make the entire FLIP clearer. > >>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>> In addition, regarding the FLIP process, there is a > >>>>> small > >>>>>>>>>>>>>>>> suggestion: > >>>>>>>>>>>>>>>>>> The > >>>>>>>>>>>>>>>>>>>> community generally creates a JIRA issue after the > >>> FLIP > >>>>>>>>>>>> vote > >>>>>>>>>>>>> is > >>>>>>>>>>>>>>>>> passed, > >>>>>>>>>>>>>>>>>>>> instead of during the FLIP preparation phase > >>> because > >>>>> the > >>>>>>>>>>>>> FLIP may > >>>>>>>>>>>>>>>> be > >>>>>>>>>>>>>>>>>>>> rejected. Although this FLIP is very reasonable, > >>> it's > >>>>>>>>>>>> better > >>>>>>>>>>>>> to > >>>>>>>>>>>>>>>>> follow > >>>>>>>>>>>>>>>>>> the > >>>>>>>>>>>>>>>>>>>> process. > >>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>> Best, > >>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>> Leonard > >>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>> [1] > >>>>>>>>>>>>>>>> > >>>>>>> https://cwiki.apache.org/confluence/display/FLINK/FLIP+Template > >>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>> On Mon, Mar 20, 2023 at 7:04 PM David Morávek < > >>>>>>>>>>>>> d...@apache.org> > >>>>>>>>>>>>>>>>> wrote: > >>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>> however listeners can use previous state > >>>>>>>>>>> (tags/labels) > >>>>>>>>>>>> to > >>>>>>>>>>>>>>> make > >>>>>>>>>>>>>>>>>>>> decisions > >>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>> That sounds like a very fragile contract. We > >>> should > >>>>>>>>>>>> either > >>>>>>>>>>>>>>> allow > >>>>>>>>>>>>>>>>>> passing > >>>>>>>>>>>>>>>>>>>>> tags between listeners and then need to define > >>>>> ordering > >>>>>>>>>>>> or > >>>>>>>>>>>>> make > >>>>>>>>>>>>>>>> all > >>>>>>>>>>>>>>>>>> of > >>>>>>>>>>>>>>>>>>>> them > >>>>>>>>>>>>>>>>>>>>> independent. I prefer the latter because it > >>> allows us > >>>>>>>>>>> to > >>>>>>>>>>>>>>>>> parallelize > >>>>>>>>>>>>>>>>>>>> things > >>>>>>>>>>>>>>>>>>>>> if needed (if all listeners trigger an RCP to the > >>>>>>>>>>>> external > >>>>>>>>>>>>>>>> system, > >>>>>>>>>>>>>>>>>> for > >>>>>>>>>>>>>>>>>>>>> example). > >>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>> Can you expand on why we need more than one > >>> classifier > >>>>>>>>>>> to > >>>>>>>>>>>>> be > >>>>>>>>>>>>>>> able > >>>>>>>>>>>>>>>>> to > >>>>>>>>>>>>>>>>>>>> output > >>>>>>>>>>>>>>>>>>>>> the same tag? > >>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>> system ones come first and then the ones loaded > >>> from > >>>>>>>>>>> the > >>>>>>>>>>>>> plugin > >>>>>>>>>>>>>>>>>> manager > >>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>> Since they're returned as a Set, the order is > >>>>>>>>>>> completely > >>>>>>>>>>>>>>>>>>>> non-deterministic, > >>>>>>>>>>>>>>>>>>>>> no matter in which order they're loaded. > >>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>> just communicating with external > >>> monitoring/alerting > >>>>>>>>>>>>> systems > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>> That makes the need for pushing things out of the > >>> main > >>>>>>>>>>>>> thread > >>>>>>>>>>>>>>>> even > >>>>>>>>>>>>>>>>>>>>> stronger. This almost sounds like we need to > >>> return a > >>>>>>>>>>>>>>>>>> CompletableFuture > >>>>>>>>>>>>>>>>>>>> for > >>>>>>>>>>>>>>>>>>>>> the per-throwable classification because an > >>> external > >>>>>>>>>>>> system > >>>>>>>>>>>>>>> might > >>>>>>>>>>>>>>>>>> take a > >>>>>>>>>>>>>>>>>>>>> significant time to respond. We need to unblock > >>> the > >>>>>>>>>>> main > >>>>>>>>>>>>> thread > >>>>>>>>>>>>>>>> for > >>>>>>>>>>>>>>>>>> other > >>>>>>>>>>>>>>>>>>>>> RPCs. > >>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>> Also, in the proposal, this happens in the failure > >>>>>>>>>>>>> handler. If > >>>>>>>>>>>>>>>>>> that's the > >>>>>>>>>>>>>>>>>>>>> case, this might block the job from being > >>> restarted > >>>>> (if > >>>>>>>>>>>> the > >>>>>>>>>>>>>>>> restart > >>>>>>>>>>>>>>>>>>>>> strategy allows for another restart), which would > >>> be > >>>>>>>>>>>> great > >>>>>>>>>>>>> to > >>>>>>>>>>>>>>>> avoid > >>>>>>>>>>>>>>>>>>>> because > >>>>>>>>>>>>>>>>>>>>> it can introduce extra downtime. > >>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>> This raises another question: what should happen > >>> if > >>>>> the > >>>>>>>>>>>>>>>>>> classification > >>>>>>>>>>>>>>>>>>>>> fails? Crashing the job (which is what's currently > >>>>>>>>>>>>> proposed) > >>>>>>>>>>>>>>>> seems > >>>>>>>>>>>>>>>>>> very > >>>>>>>>>>>>>>>>>>>>> dangerous if this might depend on an external > >>> system. > >>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>> Thats a valid point, passing the JobGraph > >>> containing > >>>>>>>>>>> all > >>>>>>>>>>>>> the > >>>>>>>>>>>>>>>> above > >>>>>>>>>>>>>>>>>>>>>> information is also something to consider > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>> We should avoid passing JG around because it's > >>> mutable > >>>>>>>>>>>>> (which > >>>>>>>>>>>>>>> we > >>>>>>>>>>>>>>>>>> must fix > >>>>>>>>>>>>>>>>>>>>> in the long term), and letting users change it > >>> might > >>>>>>>>>>> have > >>>>>>>>>>>>>>>>>> consequences. > >>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>> Best, > >>>>>>>>>>>>>>>>>>>>> D. > >>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>> On Mon, Mar 20, 2023 at 7:23 AM Panagiotis > >>> Garefalakis > >>>>>>>>>>> < > >>>>>>>>>>>>>>>>>>>> pga...@apache.org> > >>>>>>>>>>>>>>>>>>>>> wrote: > >>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>> Hey David, Shammon, > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>> Thanks for the valuable comments! > >>>>>>>>>>>>>>>>>>>>>> I am glad you find this proposal useful, some > >>>>>>>>>>> thoughts: > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>> @Shammon > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>> 1. How about adding more job information in > >>>>>>>>>>>>>>>>>> FailureListenerContext? For > >>>>>>>>>>>>>>>>>>>>>>> example, job vertext, subtask, taskmanager > >>>>>>>>>>> location. > >>>>>>>>>>>>> And > >>>>>>>>>>>>>>> then > >>>>>>>>>>>>>>>>>> user > >>>>>>>>>>>>>>>>>>>> can > >>>>>>>>>>>>>>>>>>>>> do > >>>>>>>>>>>>>>>>>>>>>>> more statistics according to different > >>> dimensions. > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>> Thats a valid point, passing the JobGraph > >>> containing > >>>>>>>>>>>> all > >>>>>>>>>>>>> the > >>>>>>>>>>>>>>>>> above > >>>>>>>>>>>>>>>>>>>>>> information > >>>>>>>>>>>>>>>>>>>>>> is also something to consider, I was mostly > >>> trying to > >>>>>>>>>>>> be > >>>>>>>>>>>>>>>>>> conservative: > >>>>>>>>>>>>>>>>>>>>>> i.e., passingly only the information we need, and > >>>>>>>>>>>> extend > >>>>>>>>>>>>> as > >>>>>>>>>>>>>>> we > >>>>>>>>>>>>>>>>> see > >>>>>>>>>>>>>>>>>> fit > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>> 2. Users may want to save results in listener, > >>> and > >>>>>>>>>>> then > >>>>>>>>>>>>> they > >>>>>>>>>>>>>>>> can > >>>>>>>>>>>>>>>>>> get > >>>>>>>>>>>>>>>>>>>> the > >>>>>>>>>>>>>>>>>>>>>>> historical results even jabmanager failover. > >>> Can we > >>>>>>>>>>>>>>> provide a > >>>>>>>>>>>>>>>>>> unified > >>>>>>>>>>>>>>>>>>>>>>> implementation for data storage requirements? > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>> The idea is to store only the output of the > >>> Listeners > >>>>>>>>>>>>> (tags) > >>>>>>>>>>>>>>>> and > >>>>>>>>>>>>>>>>>> treat > >>>>>>>>>>>>>>>>>>>>> them > >>>>>>>>>>>>>>>>>>>>>> as stateless. > >>>>>>>>>>>>>>>>>>>>>> Tags are be stored along with HistoryEntries, and > >>>>>>>>>>> will > >>>>>>>>>>>> be > >>>>>>>>>>>>>>>>> available > >>>>>>>>>>>>>>>>>>>>> through > >>>>>>>>>>>>>>>>>>>>>> the HistoryServer > >>>>>>>>>>>>>>>>>>>>>> even after a JM dies. > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>> @David > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>> 1) Should we also consider adding labels? The > >>>>>>>>>>>>> combination of > >>>>>>>>>>>>>>>> tags > >>>>>>>>>>>>>>>>>> and > >>>>>>>>>>>>>>>>>>>>>>> labels seems to be what most systems offer; > >>>>>>>>>>>> sometimes, > >>>>>>>>>>>>> they > >>>>>>>>>>>>>>>>> offer > >>>>>>>>>>>>>>>>>>>>> labels > >>>>>>>>>>>>>>>>>>>>>>> only (key=value pairs) because tags can be > >>>>>>>>>>>> implemented > >>>>>>>>>>>>>>> using > >>>>>>>>>>>>>>>>>> those, > >>>>>>>>>>>>>>>>>>>> but > >>>>>>>>>>>>>>>>>>>>>> not > >>>>>>>>>>>>>>>>>>>>>>> the other way around. > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>> Indeed changing tags to k:v labels could be more > >>>>>>>>>>>>> expressive, > >>>>>>>>>>>>>>> I > >>>>>>>>>>>>>>>>>> like it! > >>>>>>>>>>>>>>>>>>>>>> Let's see what others think. > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>> 2) Since we can not predict how heavy > >>> user-defined > >>>>>>>>>>>> models > >>>>>>>>>>>>>>>>>> ("listeners") > >>>>>>>>>>>>>>>>>>>>> are > >>>>>>>>>>>>>>>>>>>>>>> going to be, it would be great to keep the > >>>>>>>>>>>>> interfaces/data > >>>>>>>>>>>>>>>>>> structures > >>>>>>>>>>>>>>>>>>>>>>> immutable so we can push things over to the I/O > >>>>>>>>>>>>> threads. > >>>>>>>>>>>>>>>> Also, > >>>>>>>>>>>>>>>>> it > >>>>>>>>>>>>>>>>>>>>> sounds > >>>>>>>>>>>>>>>>>>>>>>> off to call the main interface a Listener since > >>>>>>>>>>> it's > >>>>>>>>>>>>>>> supposed > >>>>>>>>>>>>>>>>> to > >>>>>>>>>>>>>>>>>>>>> enhance > >>>>>>>>>>>>>>>>>>>>>>> the original throwable with additional metadata. > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>> The idea was for the name to be generic as there > >>>>>>>>>>> could > >>>>>>>>>>>> be > >>>>>>>>>>>>>>>>> Listener > >>>>>>>>>>>>>>>>>>>>>> implementations > >>>>>>>>>>>>>>>>>>>>>> just communicating with external > >>> monitoring/alerting > >>>>>>>>>>>>> systems > >>>>>>>>>>>>>>>> and > >>>>>>>>>>>>>>>>> no > >>>>>>>>>>>>>>>>>>>>>> metadata output > >>>>>>>>>>>>>>>>>>>>>> -- but lets rethink that. For immutability, see > >>>>>>>>>>> below: > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>> 3) You're proposing to support a set of > >>> listeners. > >>>>>>>>>>>> Since > >>>>>>>>>>>>>>> you're > >>>>>>>>>>>>>>>>>> passing > >>>>>>>>>>>>>>>>>>>>> the > >>>>>>>>>>>>>>>>>>>>>>> mutable context around, which includes tags set > >>> by > >>>>>>>>>>>> the > >>>>>>>>>>>>>>>> previous > >>>>>>>>>>>>>>>>>>>>> listener, > >>>>>>>>>>>>>>>>>>>>>>> do you expect users to make any assumptions > >>> about > >>>>>>>>>>> the > >>>>>>>>>>>>> order > >>>>>>>>>>>>>>>> in > >>>>>>>>>>>>>>>>>> which > >>>>>>>>>>>>>>>>>>>>>>> listeners are executed? > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>> In the existing proposal we are not making any > >>>>>>>>>>>>> assumptions > >>>>>>>>>>>>>>>> about > >>>>>>>>>>>>>>>>>> the > >>>>>>>>>>>>>>>>>>>>> order > >>>>>>>>>>>>>>>>>>>>>> of listeners, > >>>>>>>>>>>>>>>>>>>>>> (system ones come first and then the ones loaded > >>> from > >>>>>>>>>>>> the > >>>>>>>>>>>>>>>> plugin > >>>>>>>>>>>>>>>>>>>> manager) > >>>>>>>>>>>>>>>>>>>>>> however listeners can use previous state > >>>>>>>>>>> (tags/labels) > >>>>>>>>>>>> to > >>>>>>>>>>>>>>> make > >>>>>>>>>>>>>>>>>>>> decisions: > >>>>>>>>>>>>>>>>>>>>>> e.g., wont assign *UNKNOWN* failureType when we > >>> have > >>>>>>>>>>>>> already > >>>>>>>>>>>>>>>> seen > >>>>>>>>>>>>>>>>>> *USER > >>>>>>>>>>>>>>>>>>>>> *or > >>>>>>>>>>>>>>>>>>>>>> the other way around -- when we have seen > >>> *UNKNOWN* > >>>>>>>>>>>>> remove in > >>>>>>>>>>>>>>>>>> favor of > >>>>>>>>>>>>>>>>>>>>>> *USER* > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>> Cheers, > >>>>>>>>>>>>>>>>>>>>>> Panagiotis > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>> On Sun, Mar 19, 2023 at 10:42 AM David Morávek < > >>>>>>>>>>>>>>>> d...@apache.org> > >>>>>>>>>>>>>>>>>>>> wrote: > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>> Hi Panagiotis, > >>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>> This is an excellent proposal and something > >>>>>>>>>>> everyone > >>>>>>>>>>>>> trying > >>>>>>>>>>>>>>>> to > >>>>>>>>>>>>>>>>>>>> provide > >>>>>>>>>>>>>>>>>>>>>>> "Flink as a service" needs to solve at some > >>> point. > >>>>>>>>>>> I > >>>>>>>>>>>>> have a > >>>>>>>>>>>>>>>>>> couple of > >>>>>>>>>>>>>>>>>>>>>>> questions: > >>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>> If I understand the proposal correctly, this is > >>>>>>>>>>> just > >>>>>>>>>>>>> about > >>>>>>>>>>>>>>>>> adding > >>>>>>>>>>>>>>>>>>>> tags > >>>>>>>>>>>>>>>>>>>>> to > >>>>>>>>>>>>>>>>>>>>>>> the Throwable by running a tuple of (Throwable, > >>>>>>>>>>>>>>>> FailureContext) > >>>>>>>>>>>>>>>>>>>>> through a > >>>>>>>>>>>>>>>>>>>>>>> user-defined model. > >>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>> 1) Should we also consider adding labels? The > >>>>>>>>>>>>> combination > >>>>>>>>>>>>>>> of > >>>>>>>>>>>>>>>>>> tags and > >>>>>>>>>>>>>>>>>>>>>>> labels seems to be what most systems offer; > >>>>>>>>>>>> sometimes, > >>>>>>>>>>>>> they > >>>>>>>>>>>>>>>>> offer > >>>>>>>>>>>>>>>>>>>>> labels > >>>>>>>>>>>>>>>>>>>>>>> only (key=value pairs) because tags can be > >>>>>>>>>>>> implemented > >>>>>>>>>>>>>>> using > >>>>>>>>>>>>>>>>>> those, > >>>>>>>>>>>>>>>>>>>> but > >>>>>>>>>>>>>>>>>>>>>> not > >>>>>>>>>>>>>>>>>>>>>>> the other way around. > >>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>> 2) Since we can not predict how heavy > >>> user-defined > >>>>>>>>>>>>> models > >>>>>>>>>>>>>>>>>>>> ("listeners") > >>>>>>>>>>>>>>>>>>>>>> are > >>>>>>>>>>>>>>>>>>>>>>> going to be, it would be great to keep the > >>>>>>>>>>>>> interfaces/data > >>>>>>>>>>>>>>>>>> structures > >>>>>>>>>>>>>>>>>>>>>>> immutable so we can push things over to the I/O > >>>>>>>>>>>>> threads. > >>>>>>>>>>>>>>>> Also, > >>>>>>>>>>>>>>>>> it > >>>>>>>>>>>>>>>>>>>>> sounds > >>>>>>>>>>>>>>>>>>>>>>> off to call the main interface a Listener since > >>>>>>>>>>> it's > >>>>>>>>>>>>>>> supposed > >>>>>>>>>>>>>>>>> to > >>>>>>>>>>>>>>>>>>>>> enhance > >>>>>>>>>>>>>>>>>>>>>>> the original throwable with additional metadata. > >>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>> I'd propose something along the lines of (we > >>> should > >>>>>>>>>>>>> have > >>>>>>>>>>>>>>>> better > >>>>>>>>>>>>>>>>>>>> names, > >>>>>>>>>>>>>>>>>>>>>> this > >>>>>>>>>>>>>>>>>>>>>>> is just to outline the idea): > >>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>> interface FailureEnricher { > >>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>> ThrowableWithTagsAndLabels > >>>>>>>>>>> enrichFailure(Throwable > >>>>>>>>>>>>> cause, > >>>>>>>>>>>>>>>>>>>>>>> ImmutableContextualMetadataAboutTheThrowable > >>>>>>>>>>>> context); > >>>>>>>>>>>>>>>>>>>>>>> } > >>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>> The names should change; this is just to outline > >>>>>>>>>>> the > >>>>>>>>>>>>> idea. > >>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>> 3) You're proposing to support a set of > >>> listeners. > >>>>>>>>>>>>> Since > >>>>>>>>>>>>>>>> you're > >>>>>>>>>>>>>>>>>>>> passing > >>>>>>>>>>>>>>>>>>>>>> the > >>>>>>>>>>>>>>>>>>>>>>> mutable context around, which includes tags set > >>> by > >>>>>>>>>>>> the > >>>>>>>>>>>>>>>> previous > >>>>>>>>>>>>>>>>>>>>> listener, > >>>>>>>>>>>>>>>>>>>>>>> do you expect users to make any assumptions > >>> about > >>>>>>>>>>> the > >>>>>>>>>>>>> order > >>>>>>>>>>>>>>>> in > >>>>>>>>>>>>>>>>>> which > >>>>>>>>>>>>>>>>>>>>>>> listeners are executed? > >>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>> *@Shammon* > >>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>> Users may want to save results in listener, and > >>>>>>>>>>> then > >>>>>>>>>>>>> they > >>>>>>>>>>>>>>> can > >>>>>>>>>>>>>>>>>> get the > >>>>>>>>>>>>>>>>>>>>>>>> historical results even jabmanager failover. > >>> Can > >>>>>>>>>>> we > >>>>>>>>>>>>>>>> provide a > >>>>>>>>>>>>>>>>>>>> unified > >>>>>>>>>>>>>>>>>>>>>>>> implementation for data storage requirements? > >>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>> I think we should explicitly state that all > >>>>>>>>>>>>> "listeners" are > >>>>>>>>>>>>>>>>>> treated > >>>>>>>>>>>>>>>>>>>> as > >>>>>>>>>>>>>>>>>>>>>>> stateless. I don't see any strong reason for > >>>>>>>>>>>>> snapshotting > >>>>>>>>>>>>>>>> them. > >>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>> Best, > >>>>>>>>>>>>>>>>>>>>>>> D. > >>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>> On Sat, Mar 18, 2023 at 1:00 AM Shammon FY < > >>>>>>>>>>>>>>>> zjur...@gmail.com> > >>>>>>>>>>>>>>>>>>>> wrote: > >>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>> Hi Panagiotis > >>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>> Thank you for starting this discussion. I think > >>>>>>>>>>>> this > >>>>>>>>>>>>> FLIP > >>>>>>>>>>>>>>>> is > >>>>>>>>>>>>>>>>>>>> valuable > >>>>>>>>>>>>>>>>>>>>>> and > >>>>>>>>>>>>>>>>>>>>>>>> can help user to analyze the causes of job > >>>>>>>>>>> failover > >>>>>>>>>>>>>>> better! > >>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>> I have two comments as follows > >>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>> 1. How about adding more job information in > >>>>>>>>>>>>>>>>>> FailureListenerContext? > >>>>>>>>>>>>>>>>>>>>> For > >>>>>>>>>>>>>>>>>>>>>>>> example, job vertext, subtask, taskmanager > >>>>>>>>>>>> location. > >>>>>>>>>>>>> And > >>>>>>>>>>>>>>>> then > >>>>>>>>>>>>>>>>>> user > >>>>>>>>>>>>>>>>>>>>> can > >>>>>>>>>>>>>>>>>>>>>> do > >>>>>>>>>>>>>>>>>>>>>>>> more statistics according to different > >>>>>>>>>>> dimensions. > >>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>> 2. Users may want to save results in listener, > >>>>>>>>>>> and > >>>>>>>>>>>>> then > >>>>>>>>>>>>>>>> they > >>>>>>>>>>>>>>>>>> can > >>>>>>>>>>>>>>>>>>>> get > >>>>>>>>>>>>>>>>>>>>>> the > >>>>>>>>>>>>>>>>>>>>>>>> historical results even jabmanager failover. > >>> Can > >>>>>>>>>>> we > >>>>>>>>>>>>>>>> provide a > >>>>>>>>>>>>>>>>>>>> unified > >>>>>>>>>>>>>>>>>>>>>>>> implementation for data storage requirements? > >>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>> Best, > >>>>>>>>>>>>>>>>>>>>>>>> shammon FY > >>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>> On Saturday, March 18, 2023, Panagiotis > >>>>>>>>>>>> Garefalakis < > >>>>>>>>>>>>>>>>>>>>> pga...@apache.org > >>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>> wrote: > >>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>> Hi everyone, > >>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>> This FLIP [1] proposes a pluggable interface > >>>>>>>>>>> for > >>>>>>>>>>>>>>> failure > >>>>>>>>>>>>>>>>>> handling > >>>>>>>>>>>>>>>>>>>>>>>> allowing > >>>>>>>>>>>>>>>>>>>>>>>>> users to implement custom failure logic using > >>>>>>>>>>> the > >>>>>>>>>>>>>>> plugin > >>>>>>>>>>>>>>>>>>>> framework. > >>>>>>>>>>>>>>>>>>>>>>>>> Motivated by existing proposals [2] and > >>> tickets > >>>>>>>>>>>>> [3], > >>>>>>>>>>>>>>> this > >>>>>>>>>>>>>>>>>> enables > >>>>>>>>>>>>>>>>>>>>>>>> use-cases > >>>>>>>>>>>>>>>>>>>>>>>>> like: assigning particular types to failures > >>>>>>>>>>>> (e.g., > >>>>>>>>>>>>>>> User > >>>>>>>>>>>>>>>> or > >>>>>>>>>>>>>>>>>>>>> System), > >>>>>>>>>>>>>>>>>>>>>>>>> emitting custom metrics per type (e.g., > >>>>>>>>>>>>> application or > >>>>>>>>>>>>>>>>>> platform), > >>>>>>>>>>>>>>>>>>>>>> even > >>>>>>>>>>>>>>>>>>>>>>>>> exposing errors to downstream consumers (e.g., > >>>>>>>>>>>>>>>> notification > >>>>>>>>>>>>>>>>>>>>> systems). > >>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>> Thanks to Piotr and Anton for the initial > >>>>>>>>>>> reviews > >>>>>>>>>>>>> and > >>>>>>>>>>>>>>>>>>>> discussions! > >>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>> For anyone interested, the starting point > >>> would > >>>>>>>>>>>> be > >>>>>>>>>>>>> the > >>>>>>>>>>>>>>>> FLIP > >>>>>>>>>>>>>>>>>> [1] > >>>>>>>>>>>>>>>>>>>>> that > >>>>>>>>>>>>>>>>>>>>>> I > >>>>>>>>>>>>>>>>>>>>>>>>> created, > >>>>>>>>>>>>>>>>>>>>>>>>> describing the motivation and the proposed > >>>>>>>>>>>> changes > >>>>>>>>>>>>>>> (part > >>>>>>>>>>>>>>>> of > >>>>>>>>>>>>>>>>>> the > >>>>>>>>>>>>>>>>>>>>> core, > >>>>>>>>>>>>>>>>>>>>>>>>> runtime and web). > >>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>> The intuition behind this FLIP is being able > >>> to > >>>>>>>>>>>>> execute > >>>>>>>>>>>>>>>>>> custom > >>>>>>>>>>>>>>>>>>>>> logic > >>>>>>>>>>>>>>>>>>>>>> on > >>>>>>>>>>>>>>>>>>>>>>>>> failures by exposing a FailureListener > >>>>>>>>>>> interface. > >>>>>>>>>>>>>>>>>> Implementation > >>>>>>>>>>>>>>>>>>>> by > >>>>>>>>>>>>>>>>>>>>>>> users > >>>>>>>>>>>>>>>>>>>>>>>>> can be simply loaded to the system as Jar > >>>>>>>>>>> files. > >>>>>>>>>>>>>>>>>> FailureListeners > >>>>>>>>>>>>>>>>>>>>> may > >>>>>>>>>>>>>>>>>>>>>>>> also > >>>>>>>>>>>>>>>>>>>>>>>>> decide to assign failure tags to errors > >>>>>>>>>>>> (expressed > >>>>>>>>>>>>> as > >>>>>>>>>>>>>>>>>> strings), > >>>>>>>>>>>>>>>>>>>>>>>>> that will then be exposed as metadata by the > >>>>>>>>>>>>> UI/Rest > >>>>>>>>>>>>>>>>>> interfaces. > >>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>> Feedback is always appreciated! Looking > >>> forward > >>>>>>>>>>>> to > >>>>>>>>>>>>> your > >>>>>>>>>>>>>>>>>> thoughts! > >>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>> [1] > >>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> > >>>>> https://cwiki.apache.org/confluence/display/FLINK/FLIP-304% > >>>>>>>>>>>>>>>>>>>>>>>>> 3A+Pluggable+failure+handling+for+Apache+Flink > >>>>>>>>>>>>>>>>>>>>>>>>> [2] > >>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> > >>>>> https://docs.google.com/document/d/1pcHg9F3GoDDeVD5GIIo2wO67 > >>>>>>>>>>>>>>>>>>>>>>>>> Hmjgy0-hRDeuFnrMgT4 > >>>>>>>>>>>>>>>>>>>>>>>>> [3] > >>>>>>>>>>>>> https://issues.apache.org/jira/browse/FLINK-20833 > >>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>>> Cheers, > >>>>>>>>>>>>>>>>>>>>>>>>> Panagiotis > >>>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> > >>>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>> > >>>>>>>>> > >>>>>>>> > >>>>>>> > >>>>> > >>>>> > >>> > >
