This page is not created by, affiliated with, or supported by Slack Technologies, Inc.
2021-07-16
Channels
- # announcements (3)
- # babashka (25)
- # beginners (71)
- # calva (18)
- # clj-kondo (52)
- # cljs-dev (94)
- # cljsrn (12)
- # clojure (33)
- # clojure-europe (52)
- # clojure-nl (2)
- # clojure-uk (27)
- # clojurescript (18)
- # clojureverse-ops (4)
- # datomic (64)
- # deps-new (27)
- # depstar (5)
- # events (5)
- # fulcro (5)
- # graalvm (12)
- # graalvm-mobile (82)
- # helix (2)
- # introduce-yourself (1)
- # juxt (5)
- # lsp (10)
- # malli (7)
- # missionary (1)
- # off-topic (41)
- # pathom (69)
- # pedestal (6)
- # re-frame (4)
- # reagent (8)
- # releases (9)
- # remote-jobs (8)
- # shadow-cljs (3)
- # sql (46)
- # tools-deps (44)
- # uncomplicate (1)
- # vim (83)
Triggered by a gnarly issue (yet unresolved), I am trying to migrate to next.jdbc
starting with a macro that is used to wrap forms that need to run in a transaction. For various practical reasons, the migration cannot be done in one shot.
I did go through the migration bits in the documentation and about the with-transaction
in https://cljdoc.org/d/com.github.seancorfield/next.jdbc/1.2.674/doc/migration-from-clojure-java-jdbc#further-minor-differences and also looked at the source for both for the old and new libraries, but I am still not able figure this out.
The issue is that the new with-transaction
does not rollback if there is an exception in the body (the block tagged as :new
in the comment). But the :old
and the :almost-ideal
work as expected.
Am I missing something obvious? Should I not be doing this? It would be nice if someone can shed some light on what marks the fact that a connection has an open transaction in the realm of JDBC.
The current version of clojure.java.jdbc
is 0.3.5
.
P.S. A lot of the things in the new macro are there for backward compatibility with the existing code, example the binding of the dynamic var. I understand that next.jdbc
does this internally(?).
(def ^:dynamic *db-conn* nil)
(defn db-spec []
(or *db-conn*
(db-spec*)))
(defmacro with-transaction-old
[[name & args :as binding] & body]
`(clojure.java.jdbc/with-db-transaction ~binding
(binding [*db-conn* ~name]
~@body)))
(defmacro with-transaction
[[name db] & body]
`(let [db# ~db
transactable# (or (:connection db#)
(:datasource db#))]
(next.jdbc/with-transaction [txn# transactable#]
(let [~name {:connection txn#}]
(binding [*db-conn* ~name]
~@body)))))
(comment
:almost-ideal
(do
(try
(with-transaction [txn (db-spec)]
(next.jdbc/execute! txn ["update member set first_name = 'Foo 3' where id = 1"])
(throw (Exception.)))
(catch Exception e
nil))
(clojure.java.jdbc/query (db-spec) ["select first_name from member where id = 1"]))
:new
(do
(try
(with-transaction [txn (db-spec)]
(clojure.java.jdbc/execute! txn ["update member set first_name = 'Foo 1' where id = 1"])
(throw (Exception.)))
(catch Exception e
nil))
(clojure.java.jdbc/query (db-spec) ["select first_name from member where id = 1"]))
:old
(do
(try
(with-transaction-old [txn (db-spec)]
(clojure.java.jdbc/execute! txn ["update member set first_name = 'Foo 2' where id = 1"])
(throw (Exception.)))
(catch Exception e
nil))
(clojure.java.jdbc/query (db-spec) ["select first_name from member where id = 1"])))
You absolutely cannot mix'n'match transactions between c.j.j and next.jdbc
.
There's a lot to unpack here but your problems are self-inflicted due to over-complicating things and/or misunderstanding the migration guide. I'll explain what we do at work, since we have a lot of code that mixes c.j.j and next.jdbc
.
We pretty much always traffic in connection pooled datasources (we use c3p0 but hikaricp is also a good choice). The "db" thing that we pass around is {:datasource c3p0-pool}
. That's directly compatible with c.j.j as a "db-spec" and you can use it with next.jdbc
via (:datasource db)
.
That allows us to swap any section of code that currently uses c.j.j over to next.jdbc
without touching any of the surrounding code -- as long as transactions are not in play higher up the call chain (we only ever use transactions for small blocks of code where it actually matters -- partly so we can be sure we never try to accidentally "nest" transaction calls... because transactions do not nest).
When you do with-db-transaction
in c.j.j, what it binds is still a "db-spec" but it has a connection inside it, that you can call c.j.j/`db-find-connection` on as I recall to get an actual Connection
that you could use as-is with next.jdbc
calls inside the c.j.j transaction.
With you do with-transaction
in next.jdbc
, what it binds is an actual Connection
object and you could pass that to a c.j.j call if you wrap it in {:connection con}
.
Except for transaction-based c.j.j code. That's like "mixing the streams". First off, the two libraries have very different behavior by default: c.j.j actually ignores any nested transaction calls, by keeping track of the "transaction level" in the "db-spec" hash map; next.jdbc
just assumes you know what you're doing and will (try to) setup a new transaction on the existing Connection
-- which means the nested call will commit/rollback to the beginning of the outermost transaction call (and this is why you should use save points instead). next.jdbc
has a mode where it will prohibit nested transaction calls -- i.e., if it thinks there's already a tx in progress when you call with-transaction
, it will throw an exception -- and it has a mode where it will ignore nested transaction calls (to mimic the c.j.j behavior). But the two mechanisms are not compatible with each other.
Thanks Sean, except for the last paragraph, I have understood it the same way as described. And I also read the next.jdbc.transaction/*nested-tx*
bit which is used to toggle the modes.
If it was the binding + dynamic-var and the overall use of a macro that wraps the libraries' with-transaction
that make it seem complicated, then that is because any calls to (db-spec)
inside the body of the transaction were also intended to return the same transaction hashmap (with the connection, level, and the rollback atom). This is code we inherited and there are a lot of places that use them now.
If I set next.jdbc.transaction/*nested-tx*
to :ignore
, will I not be able to mimic the c.j.j behaviour?
next.jdbc
will not know about the context of tx started using with-db-transaction
from c.j.j -- that's what I mean about not mixing the two types of TX.
next.jdbc
functions expect either a Connection
, a Datasource
, or something they can call get-datasource
on to get a Datasource
(and then they call get-connection
on that). So what with-transaction
binds and what with-db-transaction
bind are very different.
Understood and that is why I'm doing (or (:connection db#) (:datasource db#))
in the new macro.
I will never be starting a transaction with c.j.j's with-db-transaction
going forward. Will only be starting them with the new one.
(defmacro with-transaction
[[name db] & body]
`(let [db# ~db
transactable# (or (:connection db#)
(:datasource db#))]
(next.jdbc/with-transaction [txn# transactable#]
(let [~name {:connection txn#}]
(binding [*db-conn* ~name]
~@body)))))
And all the downstream existing code will not break because I'm doing a (let [~name {:connection txn#}...)
. Or at least that's what I expect. I mean the new macro reads and updates the DB, the issue is that it doesn't rollback when the body throws.
This is all a temporary measure so that we can migrate gradually to next.jdbc while checking if changing this one possible culprit function to use the latest library solves a burning issue we are currently tackling.You're binding ~name
to a hash map containing :connection
so I would not expect next.jdbc/execute!
to be able to use that -- it'll think it's a db-spec hash map and it'll try to call get-datasource
on it and then call get-connection
on that... which won't work...
You'd need (next.jdbc/execute! (:connection txn) ...)
in those nested calls I think?
Oops sorry that was a copy paste mistake, I did mean (:connection txn)
in the almost-ideal case. I'm calling it almost ideal because in the ideal case and our eventual goal is not have these macros and just use next.jdbc through and through. The almost ideal case was also pasted for comparison sake, it is the new
case where rollback doesn't happen.
But thanks for your help Sean, I'll scan through the sources. And dig around a little more.
At the end of the day, from what I understand at least, the way with-transaction
and with-db-transaction
work is by setting setAutoCommit to false temporarily, run the body and call commit (or rollback in case the body throws) on the connection. Since I'm working with the connection correctly, I'm not sure why the rollback doesn't happen.
I suspect c.j.j functions check whether they're in a TX and if they think they're not, they wrap each operation in a TX.
So you'll need to fake whatever context c.j.j checks for that case.
(this is why I said mixing TX styles "doesn't work": you can't just put c.j.j code inside a next.jdbc
TX and nor can you put next.jdbc
code inside a c.j.j TX)
Ah, it's worse than that... c.j.j operations can be passed a :transaction? false
option (in the hash map after the sql-params vector) to prevent that happening, but otherwise try to run a c.j.j with-db-transaction
internally so the {:connection txn#}
hash map you build also needs to have a fake c.j.j TX context in it, so that any attempts to run with-db-transaction
inside with-transaction
will think there's a c.j.j TX already in progress and not actually create another one.
(clojure.java.jdbc/execute! txn ["update member set first_name = 'Foo 5' where id = 5"] :transaction? false)
I guess it creates a new connection(?) if I pass :transaction?
as true
which is the default.
So, short of horrible hacking (which is the path you're taking), you can't nest c.j.j operations inside a next.jdbc/with-transaction
call at all.
I will make a note of that in the migration docs.
And, no, it won't create a new connection but it will build an (overlapping) TX on the current open connection -- and will commit it at the end of the c.j.j operation.
@UMPJRJU9E I've updated the next.jdbc
migration docs to talk about transactions: https://github.com/seancorfield/next-jdbc/blob/develop/doc/migration-from-clojure-java-jdbc.md#transactions -- LMK if that helps and if there's anything else you can think of that could be added.
Thanks Sean for taking the time. I understood the updated bits about transaction pretty well since I faced the issues first hand. But for someone new they might find,
However, if you have migrated that with-db-transaction call over to next.jdbc/with-transaction then any clojure.java.jdbc operations invoked inside the body of that migrated transaction will still try to create their own transactions and with-db-transaction won't know about the outer with-transaction call so you will effectively get the "overlapping" behavior of next.jdbc since the clojure.java.jdbc operation will cause the outermost transaction to be committed or rolled back.
a bit of a mouthful. Maybe pause and break at "overlapping" part?
Also, thought it'll be worthwhile pointing again that c.j.j's with-db-transaction is implicit in this part, "...that migrated transaction will still try to create their own transactions, and this implicit with-db-transaction
won't know about the outer with-transaction..."Also, fwiw, I spent the weekend migrating from c.j.j to next.jdbc for one of our microservices, all the tests are passing 🙂 The docs and the source of next.jdbc (especially that a lot of it is based on protocols), along with the fact that we had wrapped all the jdbc library calls made it easier than expected to migrate.
The plan
API is super neat compared to what was being done with db-query-with-resultset
in our code base.
Thanks. I'll have another run at that tomorrow!
@UMPJRJU9E I've split that long sentence in two:
> However, if you have migrated that with-db-transaction
call over to next.jdbc/with-transaction
then any clojure.java.jdbc
operations invoked inside the body of that migrated transaction will still try to create their own transactions and with-db-transaction
won't know about the outer with-transaction
call.
> That means you will effectively get the "overlapping" behavior of next.jdbc
since the clojure.java.jdbc
operation will cause the outermost transaction to be committed or rolled back.
I hope that's a bit easier to read. Thanks for the feedback, and I'm glad your migration is going well!
So it will be "as if" you tried to nest with-transaction
calls in next.jdbc
(default behavior).
Hahaha, I really would love to do this properly, if not for the burning issue we are facing lately and I thought it could be done because we are after all dealing with connections at the end of the day. But I guess this is depending on very internal implementation details of the libraries to make this work. I should just talk to the team and prio a proper bump of the library.
c.j.j really just had a very poor design in the area of TX 😞 That's why next.jdbc
does things so very differently.
c.j.j made it look like TX nest and then it built castles on top of that sand 😕
Sorry you're in such a terrible situation to even be attempting this sort of mix'n'match migration 😞