Support for handle_continue #234

palm86 · 2019-05-16T18:49:19Z

This tries so address issue #227. But I need some help to figure it out. I've done what seems necesary to me, but many tests fail with a timeout now and I haven't given any though to additional tests yet.

josevalim · 2019-05-17T09:02:36Z

lib/gen_stage.ex

@@ -882,11 +882,17 @@ defmodule GenStage do

  @callback init(args :: term) ::
              {:producer, state}
+              | {:producer, state, :hibernate | {:continue, term}}


The current PR doesn't handle hiberrnate, does it? In any case, there are other discussions we need to have regarding hibernate, so probably better to focus on continue for now. :)

No, but the existing documentation for the init function mentions hibernate, which confused me. I'll remove the hibernate reference from the docs (around lines 833-835) and from this callback.

I'll remove the hibernate reference from the docs (around lines 833-835) and from this callback.

Fantastic, thank you!

Hmm, actually, the handle_info callback (and the others) also contains :hibernate. I guess that was a copy paste from the GenServer docs?

Good catch! Actually we do support hibernate there but not on init. So I guess we crossed this line anyway. So I would go ahead and support it on init too!

josevalim · 2019-05-17T09:03:09Z

lib/gen_stage.ex

@@ -1726,7 +1770,7 @@ defmodule GenStage do
    end
  end

-  defp init_producer(mod, opts, state) do
+  defp init_producer(mod, opts, state, continue \\ :no_continue) do


I don't like to use default arguments to private functions. We can directly pass :no_continue when calling this function. :)

josevalim · 2019-05-17T09:03:45Z

Looks great, can you please add some tests too? Thanks!

palm86 · 2019-05-17T20:51:54Z

Looks great, can you please add some tests too? Thanks!

I'd love to, but would welcome some pointers regarding why the current tests fail:

     test/gen_stage_test.exs:1584
     ** (EXIT from #PID<0.815.0>) an exception was raised:
         ** (ErlangError) Erlang error: :timeout_value
             (stdlib) gen_server.erl:411: :gen_server.loop/7
             (stdlib) proc_lib.erl:249: :proc_lib.init_p_do_apply/3

If I only update producers to handle continue in the init, all is well, but also updating the consumers and producer-consumers to handle continue in the init, the tests fail. I suspect it has something to do with subscriptions.

josevalim · 2019-05-17T21:13:06Z

So this failure is likely that a timeout value different from an integer or infinity is being given to “after” in a receive or or Process.send_after or similar. -- *José Valimwww.plataformatec.com.br <http://www.plataformatec.com.br/>Founder and Director of R&D*

josevalim · 2019-05-27T10:27:55Z

lib/gen_stage.ex

@@ -2407,7 +2407,7 @@ defmodule GenStage do

  ## Consumer helpers

-  defp consumer_init_subscribe(producers, stage, continue) do
+  defp consumer_init_subscribe(producers, stage, continue_or_hibernate) do
    fold_fun = fn
      to, {:ok, stage, :no_continue} ->


We don't need this clause if we have a case at the end.

You are right, I'll remove it.

josevalim · 2019-05-27T10:28:13Z

lib/gen_stage.ex

+    :lists.foldl(fold_fun, {:ok, stage, continue_or_hibernate}, producers)
+    |> case do
+      {:ok, stage, :no_continue} -> {:ok, stage}
+      {:ok, stage, continue_or_hibernate} -> {:ok, stage, continue_or_hibernate}


We don't need this clause either.

Right, the otherwise is enough. Will chang.e

josevalim · 2019-05-27T10:28:52Z

Looks good, I have added some comments. Next step is to add some tests too. :)

palm86 · 2019-05-27T10:37:40Z

Looks good, I have added some comments. Next step is to add some tests too. :)

Thanks. I think there is still some functionality missing though. Currently, I only handle continue as returned from the init. I should probably handle continue as returned from the other callback implementations as well.

josevalim · 2019-05-27T10:40:42Z

I have just realized something. Today, if you return subscribe_to in your init callback, handle_subscribe would be called BEFORE the handle_continue, and it probably should be called after. I think that, instead of return {:continue, ...} and let the underlying GenServer take care of it, we should probably handle and call continue ourselves, as we need to do before handle_subscribe.

palm86 · 2019-05-27T10:43:52Z

I think I understand what you mean. I'll see if I can figure out what needs to change.

…

On Mon, 27 May 2019 at 12:40, José Valim ***@***.***> wrote: I have just realized something. Today, if you return subscribe_to in your init callback, handle_subscribe would be called BEFORE the handle_continue, and it probably should be called after. I think that, instead of return {:continue, ...} and let the underlying GenServer take care of it, we should probably handle and call continue ourselves, as we need to do before handle_subscribe. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#234?email_source=notifications&email_token=AAXFLZOKTHDBSPBSP6JM3WTPXO3C3A5CNFSM4HNPHOWKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODWJO45I#issuecomment-496168565>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAXFLZNT2D3CQRZQ4XIMM63PXO3C3ANCNFSM4HNPHOWA> .

josevalim · 2019-07-15T11:31:25Z

Ping!

palm86 · 2019-07-15T12:17:17Z

Pong! I'm still intending to get this done.

…

On Mon, 15 Jul 2019, 1:31 PM José Valim, ***@***.***> wrote: Ping! — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#234?email_source=notifications&email_token=AAXFLZLC2SZBYJKVCUW4SELP7RNZDA5CNFSM4HNPHOWKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODZ5NHMI#issuecomment-511366065>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAXFLZNU7KM34DFWZWPCVDTP7RNZDANCNFSM4HNPHOWA> .

josevalim · 2020-02-03T10:57:32Z

Hi @palm86, it seems we both got busy with other things, so I will go ahead and close this for now. If you still have an interest on this feature, we will gladly reopen it. Thanks!

Danie Palm added 2 commits April 10, 2019 10:11

Handle continue on producer init

7a1ba10

support continue for consumers

67bba00

josevalim reviewed May 17, 2019

View reviewed changes

most tests pass again

299d17e

josevalim reviewed May 27, 2019

View reviewed changes

simplify consumer init subscribe

abb99b2

josevalim closed this Feb 3, 2020

palm86 mentioned this pull request Sep 18, 2020

Support handle_continue #227

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support for handle_continue #234

Support for handle_continue #234

palm86 commented May 16, 2019

josevalim May 17, 2019

palm86 May 17, 2019

josevalim May 17, 2019

palm86 May 17, 2019

josevalim May 17, 2019

josevalim May 17, 2019

josevalim commented May 17, 2019

palm86 commented May 17, 2019 •

edited

Loading

josevalim commented May 17, 2019 via email

josevalim May 27, 2019

palm86 May 27, 2019

josevalim May 27, 2019

palm86 May 27, 2019

josevalim commented May 27, 2019

palm86 commented May 27, 2019

josevalim commented May 27, 2019

palm86 commented May 27, 2019 via email

josevalim commented Jul 15, 2019

palm86 commented Jul 15, 2019 via email

josevalim commented Feb 3, 2020

Support for handle_continue #234

Support for handle_continue #234

Conversation

palm86 commented May 16, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

josevalim commented May 17, 2019

palm86 commented May 17, 2019 • edited Loading

josevalim commented May 17, 2019 via email

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

josevalim commented May 27, 2019

palm86 commented May 27, 2019

josevalim commented May 27, 2019

palm86 commented May 27, 2019 via email

josevalim commented Jul 15, 2019

palm86 commented Jul 15, 2019 via email

josevalim commented Feb 3, 2020

palm86 commented May 17, 2019 •

edited

Loading