Clojurians Log v2

Thank you 🙏 We have a workaround for this in production code where I'd rather avoid pointing to a snapshot version of a dependency. Besides, it's a minor issue in any case, so we'll most likely wait with upgrading until the next stable release.

Not sure when I'll cut a new release since that's the only change since the last release. Might be a month.

https://github.com/seancorfield/next-jdbc/issues/308

Thank you!

Okay, there is a new snapshot that should address this (or you can use git deps).

By default, next.jdbc allows nested TX so both of the above will work, I believe? What exactly did you get bitten by?

*nested-tx* is set to :allow by default. Or are you relying on active-tx? which is saying, yes, you have a nested TX here when you don't really?

(inside you ,,, code you would have active-tx? returning true anyway so I'm not sure what you're saying here)

IIRC, the problem here is that if the body of inner transaction throws, the outer transaction get rolled back, even though they’re running on two different datasources.

(for clarity, I work with @miro.bezjak 🙂

But you might want to wait for @miro.bezjak to explain this properly

But here is what he wrote on our internal slack:

Briefly, this snippet doesn’t work as I would expect

(jdbc/with-transaction [tx-in-a-different-db (:ardoq-ds (lib/ctx :pp))]
   (jdbc/execute! tx-in-a-different-db ["LOCK TABLE job;"])
   (jdbc/with-transaction [completely-unrelated-tx-to-above (:org-ds (lib/ctx :pp))]
     (jdbc/execute! completely-unrelated-tx-to-above ["delete from component__history"])
     (throw (Exception. "An exception that should rollback the delete!"))))

Since *nested-tx* should be :allow, the locking branch should be taken here. Are you binding *nested-tx* to something else?

Look what I found:

;; Reuse the outer transaction (i.e. inner tx are ignored and will succeed / fail in line with the
;; whole, outermost transaction) instead of allowing a nested tx to complete successfully even if
;; the outer one fails. Thus, it is always safe to wrap code in jdbc/with-transaction and being
;; able to roll everything back if there is any error.
(alter-var-root #'jdbc.tx/*nested-tx* (constantly :ignore))

(also, I’m just the messenger here, in particular a messenger who’s really scared and ignorant about the finer details of transactions)

Just a note: active-tx? is just a wrapper on top of a dynamic var *active-tx* which is set inside the with-transaction macro. Thus, it doesn't correlate 100% with what happens under the wire. You may open a transaction, then pass the tx value into another thread, and the binding context will be lost. The most reliable way to check if there is a transaction is to ask the connection. Say, in Postgres, there is a trick like that:

SELECT now() = statement_timestamp();

The real issue is that transactions don't really "exist" -- they are inherently just state inside the Connection object. next.jdbc currently does its best to kind of pretend that you can execute code "inside" a TX and by default it assumes you know what you are doing. If you get it wrong, next.jdbc can't help you -- but it does provide one "knob" you can tweak, if necessary, to get you out of some holes.

Sorry for resurrecting an old thread, but I think a bug might have been introduced in the https://github.com/seancorfield/next-jdbc/commit/b0a640a10150713313af4f9dd9782b4b6f0dbf78 that fixed transactions on distinct connections, and I figured it might be worth discussing it in the context of the original change. The problem is that *active-tx* always contains a connection unwrapped by raw-connection , but the new version of active-tx? just looks for con in *active-tx* without unwrapping it. One issue is that it's inconsistent with (contains? *active-tx* raw) checks in the implementation of Transactable , and from the library user's perspective it forces usage like for example (jdbc/active-tx? (#'jdbc-tx/raw-connection conn#)) because if the connection is wrapped, one cannot use the same arg for active-tx? and with-transaction. I think active-tx? should unwrap the connection, but I'm not 100% sure how the implementation should look like, especially around possibly moving or making raw-connection a public function. @seancorfield If you agree with the analysis, please advise if I should follow up with an issue or PR on GitHub, or if you'd rather look into it yourself 🙏

@mslupny You could be right. Can you create an issue on GitHub please and I'll take a look?

Apologies for not providing enough context. In my haste to meet a friend for beer, I forgot to make sure that the question makes sense. 😕 Let me try again. As Erik said above, we change the default of *nested-tx*.

(alter-var-root #'jdbc.tx/*nested-tx* (constantly :ignore))

(jdbc/with-transaction [tx1 data-source-1]
  (jdbc/execute! tx1 ,,,)
  (jdbc/with-transaction [tx2 data-source-2]
    (jdbc/execute! tx2 ,,,)
    (throw (Exception. "An exception happened, so rollback tx2"))))

(jdbc/with-transaction [tx1 data-source-1]
  (jdbc/execute! tx1 ,,,)
  (jdbc/execute! data-source-2 ,,,)
  (throw (Exception. "An exception happened, so it'll rollback tx1")))

(jdbc/with-transaction [tx data-source]
  (jdbc/execute! tx ,,,))

(jdbc/with-transaction [tx data-source]
  (jdbc/execute! data-source ,,,))

(jdbc/with-transaction [tx1 data-source-1]
  (jdbc/execute! tx1 ,,,)
  (jdbc/with-transaction [tx2 data-source-2]
    (jdbc/execute! tx2 ,,,)))

(jdbc/with-transaction [tx1 data-source-1]
  (jdbc/execute! tx1 ,,,)
  (jdbc/execute! data-source-2 ,,,))

Auto-commit isn't a useful indicator, unfortunately (see the closed tickets that led to some of this behavior -- pretty sure auto-commit was mentioned in one or more of them). *nested-tx* was exposed mainly as a migration aid for folks coming from c.j.j 😞 Feel free to create an issue on GH that references this Slack thread. Making the TX active/nesting checks specific to Connection objects sounds like a promising approach but it will need some analysis.

Issue created: https://github.com/seancorfield/next-jdbc/issues/282 I'll see tomorrow if I can come up with the solution that will work for us. That could be the basis for the PR.

Thank you!