Fork me on GitHub
#xtdb
<
2021-04-06
>
nivekuil22:04:23

what's the situation with using open-q on an open-db? I remember this is ill-advised but can't recall why

refset22:04:29

I'm not sure what the concern might be here as my understanding is that they are orthogonal, i.e. open-q works the same whether inside a (with-open [db (c/open-db node)] ...) or not (asides from the transparent performance benefits of pooling the underlying caches & KV index iterator resources) That said, I might be missing something, and your fancy clojure.lang.ref.Cleaner machinery could well break those assumptions very easily. I'll double-check and confirm with you again here tomorrow 🙂

nivekuil22:04:05

it's just a vague memory.. I think you mentioned it actually

😅 3
refset11:04:53

Circling back to this after some discussion - I can confirm there should be no issue. That said, I will write a test for running multiple concurrent open-q's inside an open-db, since we don't actually have a test for that right now.

kevinmershon17:04:37

Just caught a bug with open-q where if I try to return a lazy sequence (e.g. a filter instead of a filterv) it actually crashed the whole JVM

nivekuil18:04:10

are you using with-open such that it's trying to realize the lazy seq after the open-q snapshot has been closed? laziness and RAII-semantics don't really mix, I've found

kevinmershon19:04:58

Yeah exactly. It's just crazy that it doesn't simply fail, but actually crashes the whole JVM

kevinmershon19:04:24

I fixed my code to realize within the with-open but I thought that was a pretty nasty edge case to point out

refset19:04:52

> I fixed my code to realize within the with-open I'm relieved to hear that worked 🙂 I agree it's a sharp edge though. Sadly there's not much we can do functionally to change the situation (due to how the native interop works), but do you think a strong warning in the docstring would help?

kevinmershon19:04:28

Yeah. Currently the docstring says attempting to iterate when closed is undefined. Perhaps it varies by operating system?

nivekuil19:04:33

I think crux could manage the lifetimes of resources opened through its api internally (e.g. expose a crux/close that safely schedules a .close, to be run only when all references to the db are gone) rather than push it on the user, though dubious whether it's worth the complexity/effort

kevinmershon19:04:03

or discourage the use of with-open in favor of a with-streaming-results type macro that wraps the form and forces data realization

kevinmershon19:04:55

I dunno if that's too hand-holdy though

refset11:04:27

both interesting ideas, thanks, I'll add notes to the board to discuss in due course. We'll at least improve the docstring for now!