xtdb icon indicating copy to clipboard operation
xtdb copied to clipboard

Chained `with-tx` call failure

Open refset opened this issue 4 years ago • 8 comments

We don't necessarily need to support this, but it was unexpected.

(-> (crux/db (crux/start-node {}))
    (crux/with-tx [[:crux.tx/put {:crux.db/id 1 :a 2}]])
    (crux/with-tx [[:crux.tx/put {:crux.db/id 2 :a 2}]]))
1. Unhandled java.lang.IllegalArgumentException
   No implementation of method: :latest-completed-tx of protocol:
   #'crux.db/IndexStore found for class: crux.kv.index_store.KvIndexStoreTx

          core_deftype.clj:  583  clojure.core/-cache-protocol-fn
          core_deftype.clj:  575  clojure.core/-cache-protocol-fn
                    db.clj:   26  crux.db/eval23094/fn/G
                 query.clj: 1902  crux.query.QueryDatasource/with_tx
                      REPL:  102  dev/eval32924
                      REPL:  100  dev/eval32924
             Compiler.java: 7177  clojure.lang.Compiler/eval
             Compiler.java: 7132  clojure.lang.Compiler/eval
                  core.clj: 3214  clojure.core/eval
                  core.clj: 3210  clojure.core/eval
    interruptible_eval.clj:   87  nrepl.middleware.interruptible-eval/evaluate/fn/fn
                  AFn.java:  152  clojure.lang.AFn/applyToHelper
                  AFn.java:  144  clojure.lang.AFn/applyTo
                  core.clj:  665  clojure.core/apply
                  core.clj: 1973  clojure.core/with-bindings*
                  core.clj: 1973  clojure.core/with-bindings*
               RestFn.java:  425  clojure.lang.RestFn/invoke
    interruptible_eval.clj:   87  nrepl.middleware.interruptible-eval/evaluate/fn
                  main.clj:  437  clojure.main/repl/read-eval-print/fn
                  main.clj:  437  clojure.main/repl/read-eval-print
                  main.clj:  458  clojure.main/repl/fn
                  main.clj:  458  clojure.main/repl
                  main.clj:  368  clojure.main/repl
               RestFn.java:  137  clojure.lang.RestFn/applyTo
                  core.clj:  665  clojure.core/apply
                  core.clj:  660  clojure.core/apply
                regrow.clj:   18  refactor-nrepl.ns.slam.hound.regrow/wrap-clojure-repl/fn
               RestFn.java: 1523  clojure.lang.RestFn/invoke
    interruptible_eval.clj:   84  nrepl.middleware.interruptible-eval/evaluate
    interruptible_eval.clj:   56  nrepl.middleware.interruptible-eval/evaluate
    interruptible_eval.clj:  152  nrepl.middleware.interruptible-eval/interruptible-eval/fn/fn
                  AFn.java:   22  clojure.lang.AFn/run
               session.clj:  202  nrepl.middleware.session/session-exec/main-loop/fn
               session.clj:  201  nrepl.middleware.session/session-exec/main-loop
                  AFn.java:   22  clojure.lang.AFn/run
               Thread.java:  834  java.lang.Thread/run

refset avatar Feb 24 '21 15:02 refset

Should this be called "chained with-tx call failure"? It's not actually the case that successive calls fail in some circumstance — just that you can't treat the return value of with-tx as a db, right? I could see why someone might try this but the existing behaviour doesn't seem surprising to me, tbh.

deobald avatar Feb 24 '21 15:02 deobald

I've clearly spent too much time with DataScript, which definitely supports it, e.g. https://github.com/tonsky/datascript/blob/def78a2079230b2d58dd2555ab100a03233f3e49/test/datascript/test/db.cljc#L53

refset avatar Feb 24 '21 16:02 refset

At first glace this looks doable - the problem is that we're using KvIndexStoreTx in place of the index store, which works for the majority of the query engine (because it only needs open-index-snapshot) but opening up a chained with-tx also requires a decent amount of the rest of the IndexStore protocol.

Reckon this is a 2-4h bash with a good wind, but possible that there's something on IndexStore that's hard to also implement for KvIndexStoreTx (particularly related to the latter having 2 kv-stores to deal with).

jarohen avatar Mar 05 '21 13:03 jarohen

Just got bit by this. Workaround is to combine all your tx-ops into one call, but it's annoying if you want to pass the results of a prospective query to with-tx and query against the resulting DB.

Note that Datomic supports chained calls to datomic.api/with for any datomic.Db.

Edit: is this related to not being able to change the valid time of a prospective DB, only a node? I.e. crux.api/db only takes node, not a prospective DB.

theronic avatar Jul 21 '21 10:07 theronic

I extensively use prospective transactions in my Datomic-based personal tax accounting system (via datomic.api/with) and am hoping to move to Crux, but without chained x/with-tx, the import and reconciliation logic will be more painful.

theronic avatar Jul 26 '21 07:07 theronic

Thanks for the added context @theronic - it's useful to hear :) I can't promise that this will get worked on organically by the team in the immediate future, but I would be very happy to help anyone interested in figuring out a PR. If you want to discuss more tactical options for getting this resolved soon, please feel free to email me ([email protected]) or message me on Slack / the Crux Zulip - and I'm always happy to chat!

refset avatar Jul 26 '21 18:07 refset

I've also run into this problem. In my application, users can transact triples (datoms as EAV) to their own speculative database 0 or more times, which will eventually be "merged" with the actual database, which I sadly can't implement without speculative transactions.

kyleerhabor avatar Dec 19 '21 01:12 kyleerhabor

I ran into what I think is this same issue when attempting to use with-tx inside the body of a tx function. Details here.

lgessler avatar Feb 09 '22 17:02 lgessler

Just ran into this too. In the context it's being used we have a previous tx's so can reconstruct a new dad as suggested by @theronic, but certainly limits where we can use this. Would really love to see this implemented, is there appetite within Juxt?

alexisvincent avatar Oct 14 '22 22:10 alexisvincent

Probably not using this for the right reasons but with-tx is very helpful for setting up testing

djtango avatar May 09 '23 11:05 djtango