Re: [DISCUSS] FLIP-443: Interruptible watermark processing

Hi Piotrek, It looks good to me. Thanks for the update! Best, Zakelly On Thu, May 23, 2024 at 7:04 PM Piotr Nowojski wrote: > Hi Zakelly, > > I've thought about it a bit more, and I think only `#execute()` methods > make the most sense to be used when implementing operators (and > interruptib

Re: [DISCUSS] FLIP-443: Interruptible watermark processing

Hi Zakelly, I've thought about it a bit more, and I think only `#execute()` methods make the most sense to be used when implementing operators (and interruptible mails), so I will just add `MailOptions` parameters only to them. If necessary, we can add more in the future. I have updated the FLIP.

Re: [DISCUSS] FLIP-443: Interruptible watermark processing

Hi Piotrek, Well, compared to this plan, I prefer your previous one, which is more in line with the intuition for executors' API, by calling `execute` directly. Before the variants get too much, I'd suggest we only do minimum change for only "interruptible". My original thinking is, doubling each

Re: [DISCUSS] FLIP-443: Interruptible watermark processing

Hi Zakelly, > I suggest not doubling the existing methods. Only providing the following one is enough In that case I would prefer to have a complete set of the methods for the sake of completeness. If the number of variants is/would be getting too much, we could convert the class into a builder?

Re: [DISCUSS] FLIP-443: Interruptible watermark processing

Hi Piotrek, `MailOptions` looks good to me. I suggest not doubling the existing methods. Only providing the following one is enough: void execute( > MailOptions mailOptions, > ThrowingRunnable command, > String descriptionFormat, > Object... descriptionArgs); WDYT? Best, Zakel

Re: [DISCUSS] FLIP-443: Interruptible watermark processing

Hi Zakelly and others, > 1. I'd suggest also providing `isInterruptable()` in `Mail`, and the > continuation mail will return true. The FLIP-425 will leverage this queue > to execute some state requests, and when the cp arrives, the operator may > call `yield()` to drain. It may happen that the co

Re: [DISCUSS] FLIP-443: Interruptible watermark processing

Hi Piotr, > we checked in the firing timers benchmark [1] and we didn't observe any > performance regression. Thanks for the feedback, it's good news to hear that. I didn't notice we already have fireProcessingTimers benchmark. If so, we can follow it after this FLIP is merged. +1 for this FLIP

Re: [DISCUSS] FLIP-443: Interruptible watermark processing

Hi Zakelly, > I'm suggesting skipping the continuation mail during draining of async state access. I see. That makes sense to me now. I will later update the FLIP. > the code path will become more complex after this FLIP due to the addition of shouldIntterupt() checks, right? Yes, that's correc

Re: [DISCUSS] FLIP-443: Interruptible watermark processing

Hi Piotr, Overall this FLIP is fine for me. I have a minor concern: IIUC, the code path will become more complex after this FLIP due to the addition of shouldIntterupt() checks, right? If so, it's better to add a benchmark to check whether the job performance regresses when one job has a lot of t

Re: [DISCUSS] FLIP-443: Interruptible watermark processing

Hi Piotr, I'm saying the scenario where things happen in the following order: 1. advance watermark and process timers. 2. the cp arrives and interrupts the timer processing, after this the continuation mail is in the mailbox queue. 3. `snapshotState` is called, where the async state access respons

Re: [DISCUSS] FLIP-443: Interruptible watermark processing

Hi Zakelly, Can you elaborate a bit more on what you have in mind? How marking mails as interruptible helps with something? If an incoming async state access response comes, it could just request to interrupt any currently ongoing computations, regardless the currently executed mail is or is not i

Re: [DISCUSS] FLIP-443: Interruptible watermark processing

Hi Piotr, Thanks for the improvement, overall +1 for this. I'd leave a minor comment: 1. I'd suggest also providing `isInterruptable()` in `Mail`, and the continuation mail will return true. The FLIP-425 will leverage this queue to execute some state requests, and when the cp arrives, the operato

Re: [DISCUSS] FLIP-443: Interruptible watermark processing

Thanks for your answers, Piotrek. I got it now. +1 for this improvement. Best, Yanfei Stefan Richter 于2024年4月30日周二 21:30写道： > > > Thanks for the improvement proposal, I’m +1 for the change! > > Best, > Stefan > > > > > On 30. Apr 2024, at 15:23, Roman Khachatryan wrote: > > > > Thanks for the

Re: [DISCUSS] FLIP-443: Interruptible watermark processing

Thanks for the improvement proposal, I’m +1 for the change! Best, Stefan > On 30. Apr 2024, at 15:23, Roman Khachatryan wrote: > > Thanks for the proposal, I definitely see the need for this improvement, +1. > > Regards, > Roman > > > On Tue, Apr 30, 2024 at 3:11 PM Piotr Nowojski

Re: [DISCUSS] FLIP-443: Interruptible watermark processing

Thanks for the proposal, I definitely see the need for this improvement, +1. Regards, Roman On Tue, Apr 30, 2024 at 3:11 PM Piotr Nowojski wrote: > Hi Yanfei, > > Thanks for the feedback! > > > 1. Currently when AbstractStreamOperator or AbstractStreamOperatorV2 > > processes a watermark, the

Re: [DISCUSS] FLIP-443: Interruptible watermark processing

Hi Yanfei, Thanks for the feedback! > 1. Currently when AbstractStreamOperator or AbstractStreamOperatorV2 > processes a watermark, the watermark will be sent to downstream, if > the `InternalTimerServiceImpl#advanceWatermark` is interrupted, when > is the watermark sent downstream? The watermar

Re: [DISCUSS] FLIP-443: Interruptible watermark processing

Hi Piotrek, Thanks for this proposal. It looks like it will shorten the checkpoint duration, especially in the case of back pressure. +1 for it! I'd like to ask some questions to understand your thoughts more precisely. 1. Currently when AbstractStreamOperator or AbstractStreamOperatorV2 process