denodrivers/postgres

Unnecessary warning: there is no transaction in progress

baoshan opened this issue · 7 comments

When committing a transaction, certain errors (e.g., serialization errors) may be cached by postgres driver. In the catch clause, postgres issues another commit. Issuing COMMIT when not inside a transaction does no harm, but it will provoke a warning message.

This troubles me because my test report is filled with random WARNING: there is no transaction in progress, which compromises the readability.

I think muting warning messages is suboptimal. I’ll draft an experimental PR. Please let me know if there are better ways to deal with this. Thanks.

A system I'm yet to implement is the logging system in the driver. There are many more warnings than just this one, ideally they could all be muted together since their purpose is for debugging only

I don't think the solution to this is to add another method on the commit function though, that much I have clear

Use a logging system to mute this specific no transaction in progress warning is suboptimal because the (unnecessary) COMMIT statement adds an extra server round trip, which is probably 20~400ms (a transactional query usually targets the primary cluster in a cross-geo clusters).

I’m leaving this issue open because I think logging is not its root cause. Feel free to close it.

Thanks.

You are right, I should hold the state of the transaction to avoid issuing the commit to the database if the transaction has already been aborted

However as I said, the solution is not to add another option to the commit method

I’ve updated the PR to track if a transaction has been committed.

This doesn’t handle the situation that the commit is not submitted via transaction.commit(). Please let me know what do you think.

This doesn’t handle the situation that the commit is not submitted via transaction.commit()

Could you elaborate on this please?

I mean transaction.queryArray("COMMIT;"). Forget about it, that’s an abuse obviously.

Closed in #393