-
Type: Improvement
-
Resolution: Fixed
-
Priority: Major - P3
-
Affects Version/s: None
-
Component/s: Transactions
-
Storage Engines
-
5
-
2024-02-20_A_near-death_puffin
As part of many API calls, we do a SESSION_API_CALL.. That looks like this:
#define SESSION_API_CALL(s, n, config, cfg) \ SESSION_API_PREPARE_CHECK(s, WT_SESSION, n); \ API_CALL(s, WT_SESSION, n, NULL, config, cfg)
SESSION_API_PREPARE_CHECK, in turn, is defined like this:
#define SESSION_API_PREPARE_CHECK(s, h, n) \ do { \ if ((s)->api_call_counter == 0) { \ int __prepare_ret; \ API_SESSION_PUSH(s, WT_SESSION, n, NULL); \ __prepare_ret = __wt_txn_context_prepare_check(s); \ API_SESSION_POP(s); \ WT_RET(__prepare_ret); \ } \ } while (0)
The __wt_txn_context_prepare_check(s) is an inline function that doesn't do much:
static inline int __wt_txn_context_prepare_check(WT_SESSION_IMPL *session) { if (F_ISSET(session->txn, WT_TXN_PREPARE_IGNORE_API_CHECK)) return (0); if (F_ISSET(session->txn, WT_TXN_PREPARE)) WT_RET_MSG(session, EINVAL, "not permitted in a prepared transaction"); return (0); }
Given that, I'm wondering why we need the overhead of a temporary API_SESSION_PUSH and API_SESSION_POP (within SESSION_API_PREPARE_CHECK)? Each of these macros do a few assignments. Is this so WT_RET_MSG can give a more meaningful message?
Some possibilities are to remove the PUSH/POP, and live with potential inferior messages for this case. Or maybe we get the best of both worlds if we switch the ordering:
#define SESSION_API_CALL(s, n, config, cfg) \ API_CALL(s, WT_SESSION, n, NULL, config, cfg); \ SESSION_API_PREPARE_CHECK(s, WT_SESSION, n)
Then I think we can remove PUSH/POP from the prepare check, knowing that API_CALL does a push, which should set things up for good error messages.