Re: Dynamic timers now supported!

Thanks for figuring this out! I didn't know about the UsesUnboundedPCollections category. On Fri, Feb 14, 2020 at 12:57 PM Ismaël Mejía wrote: > Exact, since the new tests use Unbounded PCollections we have to add the > UsesUnboundedPCollections category. > Also the Flink runner was not

Re: Dynamic timers now supported!

Exact, since the new tests use Unbounded PCollections we have to add the UsesUnboundedPCollections category. Also the Flink runner was not excluding this category for the batch (bounded) tests. I opened this one to fix it, PTAL Reuven https://github.com/apache/beam/pull/10871 On Fri, Feb 14, 2020

Re: Dynamic timers now supported!

This is running as part of the validatesRunnerBatch test, but it is executing a streaming test. Maybe that's why it's failing? On Fri, Feb 14, 2020 at 9:42 AM Reuven Lax wrote: > Ismael, > > As part of that fix I added some new tests to make sure to run these tests > on both batch and streaming

Re: Dynamic timers now supported!

Ismael, As part of that fix I added some new tests to make sure to run these tests on both batch and streaming runners, as I realized that the test was running only on batch runners before. I did this by explicitly setting the isBounded attribute on the output of the Create transform. Somehow

Re: Dynamic timers now supported!

+1 for portable implementation and design. Having features only developed using the non-portable implementation in mind will mean that the portability effort gets bogged down with filling in features that were partially completed. On Fri, Feb 14, 2020 at 8:52 AM Ismaël Mejía wrote: > Apparently

Re: Dynamic timers now supported!

Apparently the fix of Dynamic Timers for Dataflow broke the ValidatesRunner tests for Flink in batch mode that were passing before. Can you please take a look Reuven or Rehman. Tests are failing since the exact commit for the fix: 7719708a04d5d0eff3048dbd58ac1337889f8ba5 For details on the

Re: Dynamic timers now supported!

Great to know you get it working on Dataflow easily Reuven. As a new feature it looks great! Agree with Kenn maybe worth to open a new thread to discuss the changes still needed to support this in portable runners. On Mon, Feb 10, 2020 at 8:25 PM Kenneth Knowles wrote: > I think the (lack of)

Re: Dynamic timers now supported!

I think the (lack of) portability bit may have been buried in this thread. Maybe a new thread about the design for that? Kenn On Sun, Feb 9, 2020 at 11:36 AM Reuven Lax wrote: > FYI, this is now fixed for Dataflow. I also added better rejection so that > runners that don't support this feature

Re: Dynamic timers now supported!

FYI, this is now fixed for Dataflow. I also added better rejection so that runners that don't support this feature will reject the pipeline. On Sat, Feb 8, 2020 at 12:10 AM Reuven Lax wrote: > I took a look, and I think this was a simple bug. Testing a fix now. > > A larger question is how to

Re: Dynamic timers now supported!

I took a look, and I think this was a simple bug. Testing a fix now. A larger question is how to support this in the portability layer. Right now portability assumes that each timer id corresponds to a logical input PCollection, but that assumption no longer works as we now support a dynamic set

Re: Dynamic timers now supported!

Thanks for finding this. Hopefully the bug is easy .to fix. The tests indeed never ran on any runner except for the DirectRunner, which is something I should've noticed in the code review. Reuven On Mon, Feb 3, 2020 at 12:50 AM Ismaël Mejía wrote: > I had a discussion with Rehman last week and

Re: Dynamic timers now supported!

I had a discussion with Rehman last week and we discovered that the TimersMap related tests were not running for all runners because they were not tagged as part of the ValidatesRunner category. I opened a PR [1] to enable this, so please someone help me with the review/merge. I took a look just

Re: Dynamic timers now supported!

Yes. For now we exclude the flink runner, but fixing this should be fairly trivial. On Fri, Jan 24, 2020 at 3:35 PM Maximilian Michels wrote: > The Flink Runner was allowing to set a timer multiple times before we > made it comply with the Beam semantics of overwriting past invocations. > I

Re: Dynamic timers now supported!

The Flink Runner was allowing to set a timer multiple times before we made it comply with the Beam semantics of overwriting past invocations. I wouldn't be surprised if the Spark Runner never addressed this. Flink and Spark itself allow for a timer to be set to multiple times. In order to fix

Re: Dynamic timers now supported!

The new timer family is in the portability protos. I think TimerReceiver needs to be updated to set it though (I think a 1-line change). The TimerInternals class that runners implement today already handles dynamic timers, so most of the work was in the Beam SDK to provide an API that allows

Re: Dynamic timers now supported!

This looks great, thanks for the contribution Rehman! I have some questions (note I have not looked at the code at all). - Is this working for both portable and non portable runners? - What do other runners need to implement to support this (e.g. Spark)? Maybe worth to add this to the website

Re: Dynamic timers now supported!

Thank you Reuven for the guidance throughout the development process. I am delighted to contribute my two cents to the Beam project. Looking forward to more active contributions. *Thanks & Regards* *Rehman Murad Ali* Software Engineer Mobile: +92 3452076766 Skype: rehman.muradali On Thu,

Re: Dynamic timers now supported!

Very cool ! Thank you Rehman! On Fri, 24 Jan 2020, 06:12 Maximilian Michels, wrote: > Great work! That makes timers so much easier to use and also adds new > use cases. Thank you Rehman. > > On 23.01.20 22:54, Robert Burke wrote: > > Fascinating and great work! *makes notes for an eventual Go

Re: Dynamic timers now supported!

Great work! That makes timers so much easier to use and also adds new use cases. Thank you Rehman. On 23.01.20 22:54, Robert Burke wrote: Fascinating and great work! *makes notes for an eventual Go SDK implementation* On Thu, Jan 23, 2020, 1:51 PM Luke Cwik > wrote:

Re: Dynamic timers now supported!

Fascinating and great work! *makes notes for an eventual Go SDK implementation* On Thu, Jan 23, 2020, 1:51 PM Luke Cwik wrote: > This is great. Thanks for the contribution Rehman. > > On Thu, Jan 23, 2020 at 10:09 AM Reuven Lax wrote: > >> Thanks to a lot of hard work by Rehman, Beam now

Re: Dynamic timers now supported!

This is great. Thanks for the contribution Rehman. On Thu, Jan 23, 2020 at 10:09 AM Reuven Lax wrote: > Thanks to a lot of hard work by Rehman, Beam now supports dynamic timers. > As a reminder, this was discussed on the dev list some time back. > > As background, previously one had to

Dynamic timers now supported!

Thanks to a lot of hard work by Rehman, Beam now supports dynamic timers. As a reminder, this was discussed on the dev list some time back. As background, previously one had to statically declare all timers in your code. So if you wanted to have two timers, you needed to create two timer