[PATCH 2/3] FIXUP 0002-session-variables
Julien Rouhaud <julien.rouhaud@free.fr>
From: Julien Rouhaud <julien.rouhaud@free.fr>
To:
Date: 2022-09-02T19:13:47Z
Lists: pgsql-hackers
---
src/backend/commands/session_variable.c | 137 +++++++++++++-----------
1 file changed, 73 insertions(+), 64 deletions(-)
diff --git a/src/backend/commands/session_variable.c b/src/backend/commands/session_variable.c
index d5febfef75..711ebce76e 100644
--- a/src/backend/commands/session_variable.c
+++ b/src/backend/commands/session_variable.c
@@ -45,25 +45,26 @@
#include "utils/typcache.h"
/*
- * Values of session variables are stored in local memory, in
- * sessionvars hash table in binary format and the life cycle of
- * session variable can be longer than transaction. Then we
- * need to solve following issues:
+ * Values of session variables are stored in the backend local memory,
+ * in sessionvars hash table in binary format, in a dedicated memory
+ * context SVariableMemoryContext. A session variable value can stay
+ * valid for longer than a transaction. To make sure that the
+ * underlying memory is freed, we need to solve following issues:
*
- * - We need to free local memory when variable is dropped (in current
- * transaction in current session or by other sessions (other
- * users). There is a request to protect content against
- * possibly aborted the DROP VARIABLE command. So the purging should
- * not be executed immediately. It should be postponed until the end
- * of the transaction.
+ * - We need to free local memory when the variable is droppedn,
+ * whether in the current transaction in the current session or by
+ * other sessions (other users). To protect the content against
+ * possibly rollbacked DROP VARIABLE commands, the memory shouldn't
+ * be freed immediately but be postponed until the end of the
+ * transaction.
*
- * - The session variable can be dropped explicitly (by DROP VARIABLE)
- * command or implicitly (by using ON COMMIT DROP clause). The
- * purging memory at transaction end can be requested implicitly
- * too (by using the ON TRANSACTION END clause).
+ * - The session variable can be dropped explicitly (by DROP VARIABLE
+ * command) or implicitly (using ON COMMIT DROP clause), and the
+ * value can be implicitly removed (using the ON TRANSACTION END
+ * clause). In all those cases the memory should be freed at the
+ * transaction end.
*
- * The values of session variables are stored in hash table
- * sessionvars. We maintain four queues (implemented as list
+ * To achieve that, we maintain 4 queues (implemented as list
* of variable oid, list of RecheckVariableRequests
* (requests are created in reaction on sinval) and lists
* of actions). List of actions are used whe we need to calculate
@@ -140,20 +141,15 @@ static List *xact_reset_varids = NIL;
/*
* When the session variable is dropped we need to free local memory. The
* session variable can be dropped by current session, but it can be
- * dropped by other's sessions too, so we have to watc sinval message.
+ * dropped by other's sessions too, so we have to watch sinval message.
* But because we don't want to free local memory immediately, we need to
* hold list of possibly dropped session variables and at the end of
* transaction, we check session variables from this list against system
* catalog. This check can be postponed into next transaction if
- * current transactions is in aborted state. When the check is postponed
- * to the next transaction, then have to be executed just once time.
- * Saved lxid in synced_lxid is safeguard against repeated useles
- * recheck inside one transaction.
+ * current transactions is in aborted state.
*/
static List *xact_recheck_varids = NIL;
-static LocalTransactionId synced_lxid = InvalidLocalTransactionId;
-
typedef struct SVariableData
{
Oid varid; /* pg_variable OID of this sequence (hash key) */
@@ -176,7 +172,8 @@ typedef struct SVariableData
* acceptable for debug purposes (and it is necessary, because
* sometimes we cannot to access to catalog.
*/
- char *name; /* session variable name (at time of initialization) */
+ char *name; /* session variable name (at time of
+ initialization) */
char *nsname; /* session variable schema name */
bool isnull;
@@ -193,6 +190,13 @@ typedef struct SVariableData
void *domain_check_extra;
LocalTransactionId domain_check_extra_lxid;
+ /*
+ * Top level local transaction id of the last transaction that dropped the
+ * variable if any. We need this information to avoid freeing memory for
+ * variable dropped by the local backend that may be rollbacked.
+ */
+ LocalTransactionId drop_lxid;
+
TupleDesc tupdesc; /* for row type we save actual tupdesc *
* at save time, for conversion to uptime *
* tupdesc in read time */
@@ -217,8 +221,6 @@ static HTAB *sessionvars_types = NULL; /* hash table for type fingerprints of se
static MemoryContext SVariableMemoryContext = NULL;
-static bool first_time = true;
-
static void init_session_variable(SVariable svar, Variable *var);
static void register_session_variable_xact_action(Oid varid, SVariableXActAction action);
@@ -360,7 +362,7 @@ pg_variable_cache_callback(Datum arg, int cacheid, uint32 hashvalue)
/*
* although it there is low probability, we have to iterate
- * over all actively used session variables, because hashvalue
+ * over all locally set session variables, because hashvalue
* is not unique identifier.
*/
}
@@ -397,9 +399,7 @@ is_session_variable_valid(SVariable svar)
/*
* The possible invalidated variables (in memory) are checked
* against system catalog. This routine is called before
- * any read or any write from/to session variable, but it
- * should be called only once per transaction. The synced_lxid
- * is used as protection against repeated call.
+ * any read or any write from/to session variable.
*/
static void
sync_sessionvars_all()
@@ -407,14 +407,8 @@ sync_sessionvars_all()
SVariable svar;
ListCell *l;
- if (synced_lxid == MyProc->lxid)
- return;
-
if (!xact_recheck_varids)
- {
- synced_lxid = MyProc->lxid;
return;
- }
/*
* sessionvars is null after DISCARD VARIABLES.
@@ -425,14 +419,13 @@ sync_sessionvars_all()
{
list_free(xact_recheck_varids);
xact_recheck_varids = NIL;
- synced_lxid = MyProc->lxid;
return;
}
elog(DEBUG1, "effective call of sync_sessionvars_all()");
/*
- * This routine is called before any reading. So the
+ * This routine is called before any reading, so the
* session should be in transaction state. This is required
* for access to system catalog.
*/
@@ -446,18 +439,27 @@ sync_sessionvars_all()
svar = (SVariable) hash_search(sessionvars, &varid,
HASH_FIND, &found);
+ /*
+ * Remove invalid variables, but don't touch variables that were
+ * dropped by the current top level local transaction, as there's no
+ * guarantee that the transaction will be committed. Such variables
+ * will be removed in the next transaction if needed.
+ */
if (found)
{
+ /*
+ * If this is a variable dropped by the current transaction, ignore
+ * it and keep the oid to recheck in the next transaction.
+ */
+ if (svar->drop_lxid == MyProc->lxid)
+ continue;
+
if (!is_session_variable_valid(svar))
remove_session_variable(svar);
}
- }
-
- list_free(xact_recheck_varids);
- xact_recheck_varids = NIL;
- /* Don't repeat this check in this transaction more time */
- synced_lxid = MyProc->lxid;
+ xact_recheck_varids = foreach_delete_current(xact_recheck_varids, l);
+ }
}
/*
@@ -468,20 +470,21 @@ create_sessionvars_hashtables(void)
{
HASHCTL vars_ctl;
+ Assert(!sessionvars);
+
/* set callbacks */
- if (first_time)
+ if (!SVariableMemoryContext)
{
/* Read sinval messages */
CacheRegisterSyscacheCallback(VARIABLEOID,
pg_variable_cache_callback,
(Datum) 0);
- /* needs its own long lived memory context */
+
+ /* We need our own long lived memory context */
SVariableMemoryContext =
AllocSetContextCreate(TopMemoryContext,
"session variables",
ALLOCSET_START_SMALL_SIZES);
-
- first_time = false;
}
Assert(SVariableMemoryContext);
@@ -946,7 +949,6 @@ SetSessionVariable(Oid varid, Datum value, bool isNull)
SVariable svar;
bool found;
-
if (!sessionvars)
create_sessionvars_hashtables();
@@ -1238,24 +1240,31 @@ RemoveSessionVariable(Oid varid)
svar = (SVariable) hash_search(sessionvars, &varid,
HASH_FIND, &found);
- /*
- * For variables that are not purged by default we need to
- * register SVAR_ON_COMMIT_RESET action. This action can
- * be reverted by ABORT of DROP VARIABLE command.
- */
- if (found && !svar->eox_reset)
+ if (found)
{
+ /* Save the current top level local transaction id to make sure we
+ * don't automatically remove the local variable storage in
+ * sync_sessionvars_all, as the DROP VARIABLE will send an
+ * invalidation message.
+ */
+ Assert(LocalTransactionIdIsValid(MyProc->lxid));
+ svar->drop_lxid = MyProc->lxid;
+
/*
- * And we want to enforce variable clearning when this transaction or
- * subtransaction will be committed (we don't need to wait for
- * sinval message). The cleaning action for one session variable
- * can be repeated in the action list without causing any problem,
- * so we don't need to ensure uniqueness. We need a different action
- * from RESET, because RESET is executed on any transaction end,
- * but we want to execute cleaning only when the current transaction
- * will be committed.
+ * For variables that are not purged by default we need to register
+ * an SVAR_ON_COMMIT_RESET action to free the local memory for this
+ * variable when this transaction or subtransaction is committed
+ * (we don't need to wait for sinval message). The cleaning action
+ * for one session variable can be repeated in the action list
+ * without causing any problem, so we don't need to ensure
+ * uniqueness. We need a different action from RESET, because
+ * RESET is executed on any transaction end, but we want to execute
+ * cleaning only when the current transaction will be committed.
+ * This action can be reverted by ABORT of DROP VARIABLE command.
*/
- register_session_variable_xact_action(varid, SVAR_ON_COMMIT_RESET);
+ if (!svar->eox_reset)
+ register_session_variable_xact_action(varid,
+ SVAR_ON_COMMIT_RESET);
}
}
}
--
2.37.0
--vuzhuyepzhyurc52
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment;
filename="0003-FIXUP-0009-regress-tests-for-session-variables.txt"