Skip to content

async-await (wip) #3

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

Merged
merged 3 commits into from
Apr 9, 2021
Merged

async-await (wip) #3

merged 3 commits into from
Apr 9, 2021

Conversation

trevorturk
Copy link
Collaborator

I'm trying to use https://github.com/socketry/async-await instead of https://github.com/Shopify/graphql-batch following the example from https://github.com/socketry/async-await/blob/00052e12a80ff02b3821222c52a8aed5f1c99799/examples/sleep_sort.rb but running into some trouble.

When running the server, the Rack app returns [2,2,3,4,5] which doesn't include all of the numbers I would expect. The logs look like this:

Request start
Waiting at the barrier!
I've sorted 2 for you.
I've sorted 2 for you.
I've sorted 3 for you.
I've sorted 4 for you.
I've sorted 5 for you.
Returning the result
Request finish
I've sorted 5 for you.
I've sorted 7 for you.
I've sorted 8 for you.
I've sorted 9 for you.

So, you can see we're returning the result and finishing the request before we're done with the sorting.

I'm sure I'm misunderstanding how this is all supposed to work, but I'm not clear about a few things:

  • It seems sort returns a promise (Async::Task) but do we need to call barrier! in there and also result
  • Is it reasonable/possible to do this sync def call(env) method, or do we need to do something else
  • I believe I needed to use the class << self syntax instead of sync def self.call(env) which might be a gotcha
  • I would expect this to work, but I'm sure I'm doing something wrong, or something is going awry when trying to use Rack

@trevorturk trevorturk changed the title Trying to work with async-await async-await (wip) Mar 10, 2021
@ioquatix
Copy link
Member

ioquatix commented Apr 9, 2021

I'm taking a look now.

@ioquatix
Copy link
Member

ioquatix commented Apr 9, 2021

This issue was caused by a bug in which the list of children tasks got borked. We used a linked list and there were some cases where the data structure was not updated correctly. It was fixed in the latest release of async.

I've sorted 2 for you.
I've sorted 2 for you.
I've sorted 3 for you.
I've sorted 4 for you.
I've sorted 5 for you.
I've sorted 5 for you.
I've sorted 7 for you.
I've sorted 8 for you.
I've sorted 9 for you.
Returning the result
Request finish

I'll update the PR.

@trevorturk trevorturk merged commit f5ef642 into main Apr 9, 2021
@trevorturk trevorturk deleted the async-await branch April 9, 2021 22:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants