datascript icon indicating copy to clipboard operation
datascript copied to clipboard

Feat/timeout for `q` & `pull`

Open baibhavbista opened this issue 9 months ago • 2 comments

Adds a timeout to q and pull

Mostly inspired from the timeout approach in datalevin: https://github.com/juji-io/datalevin/blob/master/src/datalevin/timeout.clj (& original PR which adds it)

Is this approach correct, @tonsky ? (seems to work, based on my tests)

baibhavbista avatar Mar 06 '25 07:03 baibhavbista

Interesting! Can you talk a little when is this useful (I am not saying that it isn’t, just want to understand use-cases)

Implementation-wise:

  • timeout ns should be probably merged into util
  • I don’t understand why parse-timeout is needed. It looks like it supports some legacy formats but we don’t have any legacy? We can make any API we want
  • relation! shouldn’t have exclamation mark in its name (there are no side effects)
  • I don’t think (timeout/assert-time-left) should be part of relation!. When you do this, it just scatters checks in more or less random places. I’d prefer more deliberate placing. E.g. in -resolve-clause, -collect, aggregate?
  • (binding [timeout/*deadline* (timeout/to-deadline should probably be one macro (with-deadline?) that does binding and wraps the body

tonsky avatar Mar 07 '25 14:03 tonsky

Sorry just seeing your reply now @tonsky !

The main usecase is when allowing end users to be able to run their own queries. Queries not properly written can cause combinatorial explosion of candidates, and lead either to OOM or to the application freezing for a non-allowably-long period of time (common use case of datascript running in the main thread in browser). In these cases, a timeout allows one to stop the query partway through if the time taken exceeds a value we deem reasonable.

Re: your implementation notes, I will go through them properly next week. BTW, as I mentioned in the PR description, most of your "why"s could be answered by "it was done that way in datalevin, and the relevant parts of the two codebases looked similar enough that I just recreated the same changes here in datascript". I will more properly think about your notes and reply/fix soon though

baibhavbista avatar Mar 12 '25 11:03 baibhavbista