Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

What happens when a new pending activation interrupts long poll? #91

Closed
Sushisource opened this issue Apr 12, 2021 · 9 comments
Closed
Labels
bug Something isn't working

Comments

@Sushisource
Copy link
Member

See original discussion here: #89

We need to test and determine best course of action when a new PA interrupts a poll. Ideally it should cancel the request, but there are many potential weird corner cases here like what to do if things race and we somehow get a new task back but don't acknowledge it.

@Sushisource Sushisource added the bug Something isn't working label Apr 12, 2021
@bergundy
Copy link
Member

Does this happen when we receive a workflow task that triggers multiple "sequential" activations and haven't sent them all out to lang while lang polls for the next workflow task?
Perhaps it would make more sense to split polling into 2 methods:
poll_workflow_task_for_specific_workflow
poll_any_workflow_task

Or maybe when a workflow activation completes core responds with next pending activation if one is available.

We should think about it.

@bergundy
Copy link
Member

Maybe buffering the poll response from the server is a better way to go here.
It keeps the interface simple and the only downside I see is that lang might not pull the buffered task fast enough to avoid WFT timeout.
This might not be such a bad thing considering that lang controls the WFT poll limits.

@Sushisource
Copy link
Member Author

I'm worried buffering just opens up too many corner cases. You have to deal with the timeout you mentioned, but also that the buffered poll could be for a different task queue than the one you just asked to poll, etc etc.

But.....

Vitaly and I had talked before about potentially responding to completions with new pending activations. We decided against it initially because it felt a little unpleasant to have to have a whole new control flow for that. But, it does solve this problem handily, and the "different task queues" problem actually already exists for Pending Activations and it would solve that too. So, that might be a nice way to kill two birds with one stone. It does make things way more clear in that "poll" now always means "yes, actually poll the server" and frankly it'd probably make a bunch of what I am doing in #93 easier. @vitarb What do you think?

@bergundy
Copy link
Member

What if we just respond with all pending activation to the poll request? Is there a reason not to do that?

@Sushisource
Copy link
Member Author

What if we just respond with all pending activation to the poll request? Is there a reason not to do that?

Per discussion in call: Not possible. We need lang to reply with appropriate commands during replay to check against history.

@mfateev
Copy link
Member

mfateev commented Apr 14, 2021

Note that synchronous reply can take a long time if the core has to retrieve the next page of a history from the service.

@bergundy
Copy link
Member

bergundy commented Apr 14, 2021 via email

@Sushisource
Copy link
Member Author

The exported functions are already defined as async anyway - so any Rust glue with whatever language would turn it into that language's async mechanism or if one does not exist, into a callback or something.

@mfateev
Copy link
Member

mfateev commented Apr 15, 2021

OK. If this mechanism can handle such delays I don't have a strong opinion. If you feel that it simplifies Lang SDK development I would not oppose it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants