On Thu, Dec 1, 2016 at 3:46 PM, Tom Lane <[email protected]> wrote:
> Robert Haas <[email protected]> writes:
>> On Thu, Dec 1, 2016 at 2:32 PM, Tom Lane <[email protected]> wrote:
>>> ... well, they would be if we passed down xactStartTimestamp to parallel
>>> workers, but I can't find any code that does that. In view of the fact that
>>> transaction_timestamp() is marked as parallel-safe, this is a bug in 9.6.
>
>> Yeah. Do you think we should arrange to pass that down, or change the
>> marking?
>
> We can't fix the marking in existing 9.6 installations, so I think we
> have to pass it down. (Which would be a better response anyway.)
I happened across this thread today and took a look at what it would
take to fix this. I quickly ran up against the fact that
SerializeTransactionState() and RestoreTransactionState() are not
exactly brilliantly designed, relying on the notion that each
individual value that we want to serialize will be no wider than a
TransactionId, which won't be true for timestamps. Even apart from
that, the whole design of those functions is pretty lame, and I'm
pretty sure I wrote all of that code myself, so I have nobody to blame
but me. Anyway, here's a proposed patch to refactor that code into
something a little more reasonable. It doesn't fix the actual problem
here, but I think it's a good first step.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index d643216..9c13717 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -65,6 +65,23 @@
#include "utils/timestamp.h"
#include "pg_trace.h"
+/*
+ * Serialization format for transaction state information.
+ */
+typedef struct SerializedTransactionState
+{
+ int xactIsoLevel;
+ bool xactDeferrable;
+ TransactionId xactTopTransactionId;
+ TransactionId currentTransactionId;
+ CommandId currentCommandId;
+ int nCurrentXids;
+ TransactionId currentXids[FLEXIBLE_ARRAY_MEMBER];
+} SerializedTransactionState;
+
+#define SIZE_OF_SERIALIZED_TRANSACTION_STATE(n_current_xids) \
+ offsetof(SerializedTransactionState, currentXids) + \
+ (n_current_xids * sizeof(TransactionId))
/*
* User-tweakable parameters
@@ -4812,8 +4829,7 @@ Size
EstimateTransactionStateSpace(void)
{
TransactionState s;
- Size nxids = 6; /* iso level, deferrable, top & current XID,
- * command counter, XID count */
+ Size nxids = 0;
for (s = CurrentTransactionState; s != NULL; s = s->parent)
{
@@ -4823,7 +4839,7 @@ EstimateTransactionStateSpace(void)
}
nxids = add_size(nxids, nParallelCurrentXids);
- return mul_size(nxids, sizeof(TransactionId));
+ return SIZE_OF_SERIALIZED_TRANSACTION_STATE(nxids);
}
/*
@@ -4847,16 +4863,17 @@ SerializeTransactionState(Size maxsize, char *start_address)
TransactionState s;
Size nxids = 0;
Size i = 0;
- Size c = 0;
TransactionId *workspace;
- TransactionId *result = (TransactionId *) start_address;
+ SerializedTransactionState *result;
+
+ result = (SerializedTransactionState *) start_address;
- result[c++] = (TransactionId) XactIsoLevel;
- result[c++] = (TransactionId) XactDeferrable;
- result[c++] = XactTopTransactionId;
- result[c++] = CurrentTransactionState->transactionId;
- result[c++] = (TransactionId) currentCommandId;
- Assert(maxsize >= c * sizeof(TransactionId));
+ Assert(maxsize >= SIZE_OF_SERIALIZED_TRANSACTION_STATE(0));
+ result->xactIsoLevel = XactIsoLevel;
+ result->xactDeferrable = XactDeferrable;
+ result->xactTopTransactionId = XactTopTransactionId;
+ result->currentTransactionId = CurrentTransactionState->transactionId;
+ result->currentCommandId = currentCommandId;
/*
* If we're running in a parallel worker and launching a parallel worker
@@ -4865,9 +4882,10 @@ SerializeTransactionState(Size maxsize, char *start_address)
*/
if (nParallelCurrentXids > 0)
{
- result[c++] = nParallelCurrentXids;
- Assert(maxsize >= (nParallelCurrentXids + c) * sizeof(TransactionId));
- memcpy(&result[c], ParallelCurrentXids,
+ result->nCurrentXids = nParallelCurrentXids;
+ Assert(maxsize >=
+ SIZE_OF_SERIALIZED_TRANSACTION_STATE(nParallelCurrentXids));
+ memcpy(&result->currentXids, ParallelCurrentXids,
nParallelCurrentXids * sizeof(TransactionId));
return;
}
@@ -4882,7 +4900,7 @@ SerializeTransactionState(Size maxsize, char *start_address)
nxids = add_size(nxids, 1);
nxids = add_size(nxids, s->nChildXids);
}
- Assert((c + 1 + nxids) * sizeof(TransactionId) <= maxsize);
+ Assert(SIZE_OF_SERIALIZED_TRANSACTION_STATE(nxids) <= maxsize);
/* Copy them to our scratch space. */
workspace = palloc(nxids * sizeof(TransactionId));
@@ -4900,8 +4918,8 @@ SerializeTransactionState(Size maxsize, char *start_address)
qsort(workspace, nxids, sizeof(TransactionId), xidComparator);
/* Copy data into output area. */
- result[c++] = (TransactionId) nxids;
- memcpy(&result[c], workspace, nxids * sizeof(TransactionId));
+ result->nCurrentXids = nxids;
+ memcpy(&result->currentXids, workspace, nxids * sizeof(TransactionId));
}
/*
@@ -4912,18 +4930,20 @@ SerializeTransactionState(Size maxsize, char *start_address)
void
StartParallelWorkerTransaction(char *tstatespace)
{
- TransactionId *tstate = (TransactionId *) tstatespace;
+ SerializedTransactionState *tstate;
+
+ tstate = (SerializedTransactionState *) tstatespace;
Assert(CurrentTransactionState->blockState == TBLOCK_DEFAULT);
StartTransaction();
- XactIsoLevel = (int) tstate[0];
- XactDeferrable = (bool) tstate[1];
- XactTopTransactionId = tstate[2];
- CurrentTransactionState->transactionId = tstate[3];
- currentCommandId = tstate[4];
- nParallelCurrentXids = (int) tstate[5];
- ParallelCurrentXids = &tstate[6];
+ XactIsoLevel = tstate->xactIsoLevel;
+ XactDeferrable = tstate->xactDeferrable;
+ XactTopTransactionId = tstate->xactTopTransactionId;
+ CurrentTransactionState->transactionId = tstate->currentTransactionId;
+ currentCommandId = tstate->currentCommandId;
+ nParallelCurrentXids = tstate->nCurrentXids;
+ ParallelCurrentXids = tstate->currentXids;
CurrentTransactionState->blockState = TBLOCK_PARALLEL_INPROGRESS;
}
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
