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

RPC Server: Make endpoints asynchronous #1706

Closed
drewbanin opened this issue Aug 27, 2019 · 2 comments · Fixed by #1735
Closed

RPC Server: Make endpoints asynchronous #1706

drewbanin opened this issue Aug 27, 2019 · 2 comments · Fixed by #1735
Labels
enhancement New feature or request rpc Issues related to dbt's RPC server

Comments

@drewbanin
Copy link
Contributor

Describe the feature

The non-administrative endpoints in the rpc server should be asynchronous. When a request is made, the rpc server should return immediately with an async token. Subsequent requests to the rpc server should be able to check the status of the running task via another endpoint, as well as retrieve task results when the request completes.

The following tasks should be asynchronous:

  • compile
  • run
  • compile_project
  • run_project
  • seed_project
  • test_project

The following tasks should be synchronous:

  • ps
  • kill
  • status
  • poll new

Async tokens
When an asynchronous request is made to the server, a request_token should be returned in the response. This should be something like a uuid.

The new /poll endpoint should accept a request_token either in the url (eg. /poll/:request_token) or as a query parameter (eg. /poll?request_token=abc123). My instinct is that this should be a POST request, but happy to discuss if something else makes more sense.

The /poll endpoint
The /poll endpoint should return a dictionary. The following examples show three different schemas for three different task states, but a single schema may be used if that is preferred in the implementation.

When the targeted request is running:

{
  "status": "running",
  "logs": [...]
}

When the targeted request has completed successfully:

{
  "status": "success",
  "data": {...},
  "logs": [...]
}

When the targeted request has completed with an error:

{
  "status": "error",
  "error": {...},
  "logs": [...]
}

Storing task results
The rpc server cannot store results indefinitely - it would eventually run out of memory! Instead, the rpc server could serialize results & logs.... somewhere. When the /poll endpoint is queried for a completed task, the rpc server could load these results from that persistent storage location. See Questions below for a discussion....

Questions:

  • The implementation described here includes logs in the /poll response. Is this a reasonable way to fetch logs from a running task?
  • Can we re-purpose the task_id used internally (and exposed in the ps/kill commands) as the request_token? That feels sensible to me, but I'm not sure if there are any caveats to be aware of.
  • Is persisting results to disk (or somewhere else) viable and reasonable in the rpc server? Should we instead hold these results in memory, then purge them after some amount of time? Holding the results in memory is a path of lesser resistance, and we can implement async tasks separately from data persistence - the two questions are closely related, but definitely distinct and separable IMO. Very happy to discuss what our options are here.

@cmcarthur, @beckjake, would love to get your thoughts on these!

Describe alternatives you've considered

we could ask users to keep their browsers open on a perfectly stable internet connection ;)

@drewbanin drewbanin added enhancement New feature or request rpc Issues related to dbt's RPC server labels Aug 27, 2019
@drewbanin drewbanin added this to the Louisa May Alcott milestone Aug 27, 2019
@beckjake
Copy link
Contributor

The implementation described here includes logs in the /poll response. Is this a reasonable way to fetch logs from a running task?

Probably for small things, but it could get slow to return the logs all the time... maybe a flag?

Can we re-purpose the task_id used internally (and exposed in the ps/kill commands) as the request_token? That feels sensible to me, but I'm not sure if there are any caveats to be aware of.

Yup, that's the plan! Async tasks work pretty nicely with a process-table model like ps/kill already use.

Is persisting results to disk (or somewhere else) viable and reasonable in the rpc server? Should we instead hold these results in memory, then purge them after some amount of time? Holding the results in memory is a path of lesser resistance, and we can implement async tasks separately from data persistence - the two questions are closely related, but definitely distinct and separable IMO. Very happy to discuss what our options are here.

For a first pass, I would just hold them in memory until they are requested and then delete them, but I'd longer-term like to "expire" completed results in the ps output after some length of time. In that case I'd tie the result storage into the process table behavior (see the point above about them being the same thing). In that world throwing them into a file (sqlite3 database?) probably makes a lot of sense.

@cmcarthur
Copy link
Member

cmcarthur commented Aug 28, 2019

Probably for small things, but it could get slow to return the logs all the time... maybe a flag?

👍 to a flag

Is persisting results to disk (or somewhere else) viable and reasonable in the rpc server? Should we instead hold these results in memory, then purge them after some amount of time? Holding the results in memory is a path of lesser resistance, and we can implement async tasks separately from data persistence - the two questions are closely related, but definitely distinct and separable IMO. Very happy to discuss what our options are here.

For a first pass, I would just hold them in memory until they are requested and then delete them, but I'd longer-term like to "expire" completed results in the ps output after some length of time. In that case I'd tie the result storage into the process table behavior (see the point above about them being the same thing). In that world throwing them into a file (sqlite3 database?) probably makes a lot of sense.

Agree all around. Let's store them in memory until that stops working.

we could ask users to keep their browsers open on a perfectly stable internet connection ;)

for the sake of cloud scalability, we time out open, unanswered requests after 60 seconds. we should avoid doing long polling all over the place if possible, IMO python isn't that good at handling that type of request load

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request rpc Issues related to dbt's RPC server
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants