-
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.