Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hello!
At my work we run queries over websockets, to get around Herokus limit on 30s per request. When running queries, they are run in serial, so if we have one query taking 32s, every other query will have to wait in line.
In this PR, I try to start to handle this concurrently instead. I do this by running each query in its own Task. Instead of returning the
Next
message inrun_doc
, I runrun_doc
in aTask
, and send a message back to the "main" process.I use a little node script to test this, which is available on https://gist.github.com/spatrik/36a69161c793e5daa1894561ea7625f9#file-1-test-ws-concurrency-mjs.
In this gist, I also have a script that runs a Phoenix server with absinthe and graphql-ws-elixir. It exposes a query
foo
with thearg
sleep
, which specifies how long the query should sleep before returning.Running the node script with the current 0.3.6 version gives the output queries: 3.018 s. With my changes, it returns 2.012 s.
In the current state, this code is not ready for merge, but I'm curious if anyone have any thoughts if this is the right direction. I'm thinking to make it actually mergeable, I'll need to
Don't just
start
aTask
. Instead I'm thinking that there should be aTask.Supervisor
started when connecting. Maybe inSocket.__connect__
and then stored in theSocket
struct?Think about limiting the number of
Task
s started. If I just start a Task for each new query, isn't that a path for DoS-ing the server? Especially if you like in my case have a query that takes 30+s. It would be pretty easy to just do 100s or 1000s of queries.I'm not entirely sure what is needed here. A simple solution would be to just start the
Task.Supervisor
withmax_children
set to something. I this case, if there are too many requests done on a single WebSocket at the same time, that WebSocket process would crash.Another option would be pulling in poolboy or something similar to make sure there are a maximum number of processes. But this is another dependency...
Make sure this works with subscriptions as well.
So, any thoughts on this? Am I on a good path here or am I completely lost?