Skip to content
This repository has been archived by the owner on Jan 6, 2021. It is now read-only.

Worker status pipeline for "alda parse" requests too #2

Closed
daveyarwood opened this issue Dec 3, 2016 · 8 comments
Closed

Worker status pipeline for "alda parse" requests too #2

daveyarwood opened this issue Dec 3, 2016 · 8 comments
Milestone

Comments

@daveyarwood
Copy link
Member

Since version 1.0.0-rc42, we've had a worker status pipeline for alda play requests only, in order to more robustly handle worker requests that take a long time from the client side.

The idea is that when a worker receives a request, it starts handling it on a separate thread and goes into a particular working status like "parsing" or "playing," and it also immediately responds to the request to acknowledge that it received the request and is working on it. Then, the client can repeatedly make status requests to the worker so it can report when the worker is playing the score. The key thing here is that the client knows when to keep waiting vs. report that the request failed and exit.

We should extend this functionality to the alda parse command, too. Currently, the client just has a timeout interval, after which time it will retry the request again. If you do something like alda parse -c "(Thread/sleep 10000)" -m, the client will get impatient and keep retrying the request until every available worker is busy parsing the same score! This effectively kills all of the available workers because they all stop what they're doing and evaluate this score that makes them pause for 10 seconds, causing the server to think the workers are dead and evict them from the worker queue. Yikes!

The solution here is the same thing we implement for the alda play command, only the client would not print any of the status updates, since the alda parse command is intended to print only the parsed output to STDOUT.

@daveyarwood
Copy link
Member Author

Should maybe rename the play-status command to worker-status.

@daveyarwood daveyarwood added this to the 1.0.0 milestone May 27, 2017
@daveyarwood
Copy link
Member Author

This is especially important for the new client/server implementation of the Alda REPL. Currently, long parse requests cause a worker to be blocked, which can hang the REPL unnecessarily (another available worker could be chosen instead of the blocked one).

@jgkamat
Copy link
Contributor

jgkamat commented Oct 15, 2017

There's a bug that's a symptom of this as well. If you parse a sufficiently large file it will return 'no workers found' instead of properly returning the parsed data (since it timesout)

@bernardo-amorim
Copy link

Is this still of interest? I'd love to give it try

@daveyarwood
Copy link
Member Author

@bernardo-amorim Absolutely, help would be much appreciated! Let us know (either here or in the #development channel in Slack) if you need any help figuring it out.

@daveyarwood
Copy link
Member Author

daveyarwood commented Mar 23, 2018

I think the first step is to extract the "job" logic from handle-code-play so that handle-code-parse can use it too:

(defn handle-code-play
[code {:keys [jobId] :as options}]
(-> (cond
(empty? jobId)
(error-response "Request missing a `jobId` option.")
(get @job-cache jobId)
(success-response "Already playing that score.")
:else
(do
(future (run-job! code options))
(success-response "Request received.")))
(assoc :jobId jobId)))
(defn handle-code-parse
[code {:keys [output] :as options}]
(try
(require '[alda.lisp :refer :all])
(let [output (case output
("data" nil) :score
"events" :events-or-error)]
(success-response (parse-input code :output output)))
(catch Throwable e
(log/error e e)
(error-response e))))

Then, run-job! will need to be modified to not be specific to playing, so it can handle parsing too:

(defn run-job!
[code {:keys [history from to jobId]}]
(try
(log/debugf "Starting job %s..." jobId)
(update-job! jobId (Job. :parsing nil nil nil))
(let [_ (log/debug "Parsing body...")
events (parse-input code :output :events-or-error)
_ (log/debug "Parsing history...")
history (when history (parse-input history))]
(log/debug "Playing score...")
(let [play-opts {:from from
:to to
:async? true
:one-off? false}
{:keys [score stop! wait]}
(if (empty? history)
(now/play-score! (score/score events) play-opts)
(now/with-score* (atom history)
(now/play-with-opts! play-opts events)))]
(update-job! jobId (Job. :playing score nil stop!))
(wait))
(log/debug "Done playing score.")
(update-job-status! jobId :success))
(catch Throwable e
(log/error e e)
(update-job! jobId (Job. :error nil e nil)))))

This could be as simple as taking an extra argument that is either :parse or :play and acting accordingly.

@bernardo-amorim
Copy link

OK, thanks for the response. I'll start coding it.

@daveyarwood
Copy link
Member Author

I think this issue can be closed. I'm in the process of redesigning Alda for an eventual 2.0 release, and part of the new design is that most of the work will be done in the client, which will make this a non-issue.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants