Re: [HACKERS] logical decoding - GetOldestXmin
On Fri, Dec 14, 2012 at 7:19 PM, Andres Freund and...@2ndquadrant.com wrote: On 2012-12-14 14:01:30 -0500, Robert Haas wrote: On Fri, Dec 14, 2012 at 6:46 AM, Andres Freund and...@2ndquadrant.com wrote: Just moving that tidbit inside the lock seems to be the pragmatic choice. GetOldestXmin is called * once per checkpoint * one per index build * once in analyze * twice per vacuum * once for HS feedback messages Nothing of that occurs frequently enough that 5 instructions will make a difference. I would be happy to go an alternative path, but right now I don't see any nice one. A already_locked parameter to GetOldestXmin seems to be a cure worse than the disease. I'm not sure that would be so bad, but I guess I question the need to do it this way at all. Most of the time, if you need to advertise your global xmin, you use GetSnapshotData(), not GetOldestXmin(), and I guess I'm not seeing why that wouldn't also work here. Am I dumb? I wondered upthread whether that would be better: On 2012-12-13 21:03:44 +0100, Andres Freund wrote: Another alternative to this would be to get a snapshot with GetSnapshotData(), copy the xmin to the logical slot, then call ProcArrayEndTransaction(). But that doesn't really seem to be nicer to me. Not sure why I considered it ugly anymore, but it actually has a noticeable disadvantage. GetOldestXmin is nicer is than GetSnapshotData as the latter set a fairly new xid as xmin whereas GetOldestXmin returns the actual current xmin horizon. Thats preferrable because it allows us to start up more quickly. snapbuild.c can only start building a snapshot once it has seen a xl_running_xact with oldestRunningXid = own_xmin. Otherwise we cannot be sure that no relevant catalog tuples have been removed. I'm a bit confused. Are you talking about the difference between RecentGlobalXmin and RecentXmin? I think GetSnapshotData() updates both. Anyway, if there's no nicer way, I think it's probably OK to add a parameter to GetOldestXmin(). It seems like kind of a hack, though. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] logical decoding - GetOldestXmin
Hi, Robert Haas robertmh...@gmail.com schrieb: On Fri, Dec 14, 2012 at 7:19 PM, Andres Freund and...@2ndquadrant.com wrote: On 2012-12-14 14:01:30 -0500, Robert Haas wrote: On Fri, Dec 14, 2012 at 6:46 AM, Andres Freund and...@2ndquadrant.com wrote: Just moving that tidbit inside the lock seems to be the pragmatic choice. GetOldestXmin is called * once per checkpoint * one per index build * once in analyze * twice per vacuum * once for HS feedback messages Nothing of that occurs frequently enough that 5 instructions will make a difference. I would be happy to go an alternative path, but right now I don't see any nice one. A already_locked parameter to GetOldestXmin seems to be a cure worse than the disease. I'm not sure that would be so bad, but I guess I question the need to do it this way at all. Most of the time, if you need to advertise your global xmin, you use GetSnapshotData(), not GetOldestXmin(), and I guess I'm not seeing why that wouldn't also work here. Am I dumb? I wondered upthread whether that would be better: On 2012-12-13 21:03:44 +0100, Andres Freund wrote: Another alternative to this would be to get a snapshot with GetSnapshotData(), copy the xmin to the logical slot, then call ProcArrayEndTransaction(). But that doesn't really seem to be nicer to me. Not sure why I considered it ugly anymore, but it actually has a noticeable disadvantage. GetOldestXmin is nicer is than GetSnapshotData as the latter set a fairly new xid as xmin whereas GetOldestXmin returns the actual current xmin horizon. Thats preferrable because it allows us to start up more quickly. snapbuild.c can only start building a snapshot once it has seen a xl_running_xact with oldestRunningXid = own_xmin. Otherwise we cannot be sure that no relevant catalog tuples have been removed. I'm a bit confused. Are you talking about the difference between RecentGlobalXmin and RecentXmin? I think GetSnapshotData() updates both. The problem is that at the time GetSnapshotData returns the xmin horizon might have gone upwards and tuples required for decoding might get removed by other backends. That needs to be prevented while holding the procarray lock exclusively. Does it make more sense now? Andres --- Please excuse the brevity and formatting - I am writing this on my mobile phone. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] logical decoding - GetOldestXmin
On Tue, Dec 18, 2012 at 5:25 PM, anara...@anarazel.de and...@anarazel.de wrote: The problem is that at the time GetSnapshotData returns the xmin horizon might have gone upwards and tuples required for decoding might get removed by other backends. That needs to be prevented while holding the procarray lock exclusively. Well, for the ordinary use of GetSnapshotData(), that doesn't matter, because GetSnapshotData() also updates proc-xmin. If you're trying to store a different value in that field then of course it matters. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] logical decoding - GetOldestXmin
On 2012-12-18 19:56:18 -0500, Robert Haas wrote: On Tue, Dec 18, 2012 at 5:25 PM, anara...@anarazel.de and...@anarazel.de wrote: The problem is that at the time GetSnapshotData returns the xmin horizon might have gone upwards and tuples required for decoding might get removed by other backends. That needs to be prevented while holding the procarray lock exclusively. Well, for the ordinary use of GetSnapshotData(), that doesn't matter, because GetSnapshotData() also updates proc-xmin. If you're trying to store a different value in that field then of course it matters. Absolutely right. I don't want to say there's anything wrong with it right now. The problem for me is that it sets proc-xmin to the newest value it can while I want/need the oldest valid value... I will go with adding a already_locked parameter to GetOldestXmin then. Thanks for the input, Andres -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] logical decoding - GetOldestXmin
On Tue, Dec 18, 2012 at 7:59 PM, Andres Freund and...@2ndquadrant.com wrote: On 2012-12-18 19:56:18 -0500, Robert Haas wrote: On Tue, Dec 18, 2012 at 5:25 PM, anara...@anarazel.de and...@anarazel.de wrote: The problem is that at the time GetSnapshotData returns the xmin horizon might have gone upwards and tuples required for decoding might get removed by other backends. That needs to be prevented while holding the procarray lock exclusively. Well, for the ordinary use of GetSnapshotData(), that doesn't matter, because GetSnapshotData() also updates proc-xmin. If you're trying to store a different value in that field then of course it matters. Absolutely right. I don't want to say there's anything wrong with it right now. The problem for me is that it sets proc-xmin to the newest value it can while I want/need the oldest valid value... I will go with adding a already_locked parameter to GetOldestXmin then. Or instead of bool already_locked, maybe bool advertise_xmin? Seems like that might be more friendly to the abstraction boundaries. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] logical decoding - GetOldestXmin
On 2012-12-15 01:19:26 +0100, Andres Freund wrote: On 2012-12-14 14:01:30 -0500, Robert Haas wrote: On Fri, Dec 14, 2012 at 6:46 AM, Andres Freund and...@2ndquadrant.com wrote: Just moving that tidbit inside the lock seems to be the pragmatic choice. GetOldestXmin is called * once per checkpoint * one per index build * once in analyze * twice per vacuum * once for HS feedback messages Nothing of that occurs frequently enough that 5 instructions will make a difference. I would be happy to go an alternative path, but right now I don't see any nice one. A already_locked parameter to GetOldestXmin seems to be a cure worse than the disease. I'm not sure that would be so bad, but I guess I question the need to do it this way at all. Most of the time, if you need to advertise your global xmin, you use GetSnapshotData(), not GetOldestXmin(), and I guess I'm not seeing why that wouldn't also work here. Am I dumb? I wondered upthread whether that would be better: On 2012-12-13 21:03:44 +0100, Andres Freund wrote: Another alternative to this would be to get a snapshot with GetSnapshotData(), copy the xmin to the logical slot, then call ProcArrayEndTransaction(). But that doesn't really seem to be nicer to me. Not sure why I considered it ugly anymore, but it actually has a noticeable disadvantage. GetOldestXmin is nicer is than GetSnapshotData as the latter set a fairly new xid as xmin whereas GetOldestXmin returns the actual current xmin horizon. Thats preferrable because it allows us to start up more quickly. snapbuild.c can only start building a snapshot once it has seen a xl_running_xact with oldestRunningXid = own_xmin. Otherwise we cannot be sure that no relevant catalog tuples have been removed. Hm. One way that could work with fewer changes is to exploit the fact that a) it seems to be possible to acquire a shared lwlock twice in the same backend and b) both GetOldestXmin GetSnapshotData acquire only a shared lwlock. Are we willing to guarantee that recursive acquiration of shared lwlocks continues to work? Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] logical decoding - GetOldestXmin
On 13 December 2012 20:03, Andres Freund and...@2ndquadrant.com wrote: Does anybody have an opinion on the attached patches? Especially 0001, which contains the procarray changes? It moves a computation of the sort of: result -= vacuum_defer_cleanup_age; if (!TransactionIdIsNormal(result)) result = FirstNormalTransactionId; inside ProcArrayLock. But I can't really imagine that to be relevant... I don't see why this is hard. Just make the lock acquisition/release conditional on another parameter. That way the only thing you'll be moving inside the lock is an if test on a constant boolean. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] logical decoding - GetOldestXmin
On 2012-12-16 16:44:04 +, Simon Riggs wrote: On 13 December 2012 20:03, Andres Freund and...@2ndquadrant.com wrote: Does anybody have an opinion on the attached patches? Especially 0001, which contains the procarray changes? It moves a computation of the sort of: result -= vacuum_defer_cleanup_age; if (!TransactionIdIsNormal(result)) result = FirstNormalTransactionId; inside ProcArrayLock. But I can't really imagine that to be relevant... I don't see why this is hard. Just make the lock acquisition/release conditional on another parameter. That way the only thing you'll be moving inside the lock is an if test on a constant boolean. Thats not really cheaper. Two branches + additional parameter passed/pushed vs one branch, one subtransaction, two assignments is a close call. As I don't think either really matters in the GetOldestXmin case, I would be happy with that as well. If people prefer an additional parameter + adjusting the few callsite vs. a separate function I will go that way. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] logical decoding - GetOldestXmin
On 2012-12-13 23:35:00 +, Simon Riggs wrote: On 13 December 2012 22:37, Andres Freund and...@2ndquadrant.com wrote: On 2012-12-13 17:29:06 -0500, Robert Haas wrote: On Thu, Dec 13, 2012 at 3:03 PM, Andres Freund and...@2ndquadrant.com wrote: It moves a computation of the sort of: result -= vacuum_defer_cleanup_age; if (!TransactionIdIsNormal(result)) result = FirstNormalTransactionId; inside ProcArrayLock. But I can't really imagine that to be relevant... I can. Go look at some of the 9.2 optimizations around GetSnapshotData(). Those made a BIG difference under heavy concurrency and they were definitely micro-optimization. For example, the introduction of NormalTransactionIdPrecedes() was shockingly effective. But GetOldestXmin() should be called less frequently than GetSnapshotData() by several orders of magnitudes. I don't really see it being used in any really hot code paths? Maybe, but that calculation doesn't *need* to be inside the lock, that is just a consequence of the current coding. I am open to suggestion how to do that in a way we a) can hold the lock already (to safely nail the global xmin to the current value) b) without duplicating all the code. Just moving that tidbit inside the lock seems to be the pragmatic choice. GetOldestXmin is called * once per checkpoint * one per index build * once in analyze * twice per vacuum * once for HS feedback messages Nothing of that occurs frequently enough that 5 instructions will make a difference. I would be happy to go an alternative path, but right now I don't see any nice one. A already_locked parameter to GetOldestXmin seems to be a cure worse than the disease. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] logical decoding - GetOldestXmin
On Fri, Dec 14, 2012 at 6:46 AM, Andres Freund and...@2ndquadrant.com wrote: Just moving that tidbit inside the lock seems to be the pragmatic choice. GetOldestXmin is called * once per checkpoint * one per index build * once in analyze * twice per vacuum * once for HS feedback messages Nothing of that occurs frequently enough that 5 instructions will make a difference. I would be happy to go an alternative path, but right now I don't see any nice one. A already_locked parameter to GetOldestXmin seems to be a cure worse than the disease. I'm not sure that would be so bad, but I guess I question the need to do it this way at all. Most of the time, if you need to advertise your global xmin, you use GetSnapshotData(), not GetOldestXmin(), and I guess I'm not seeing why that wouldn't also work here. Am I dumb? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] logical decoding - GetOldestXmin
On 2012-12-14 14:01:30 -0500, Robert Haas wrote: On Fri, Dec 14, 2012 at 6:46 AM, Andres Freund and...@2ndquadrant.com wrote: Just moving that tidbit inside the lock seems to be the pragmatic choice. GetOldestXmin is called * once per checkpoint * one per index build * once in analyze * twice per vacuum * once for HS feedback messages Nothing of that occurs frequently enough that 5 instructions will make a difference. I would be happy to go an alternative path, but right now I don't see any nice one. A already_locked parameter to GetOldestXmin seems to be a cure worse than the disease. I'm not sure that would be so bad, but I guess I question the need to do it this way at all. Most of the time, if you need to advertise your global xmin, you use GetSnapshotData(), not GetOldestXmin(), and I guess I'm not seeing why that wouldn't also work here. Am I dumb? I wondered upthread whether that would be better: On 2012-12-13 21:03:44 +0100, Andres Freund wrote: Another alternative to this would be to get a snapshot with GetSnapshotData(), copy the xmin to the logical slot, then call ProcArrayEndTransaction(). But that doesn't really seem to be nicer to me. Not sure why I considered it ugly anymore, but it actually has a noticeable disadvantage. GetOldestXmin is nicer is than GetSnapshotData as the latter set a fairly new xid as xmin whereas GetOldestXmin returns the actual current xmin horizon. Thats preferrable because it allows us to start up more quickly. snapbuild.c can only start building a snapshot once it has seen a xl_running_xact with oldestRunningXid = own_xmin. Otherwise we cannot be sure that no relevant catalog tuples have been removed. This also made me notice that my changes to GetSnapshotData were quite pessimal... I do set the xmin of the new snapshot to the logical xmin instead of doing it only to globalxmin/RecentGlobalXmin. Andres -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] logical decoding - GetOldestXmin
On 2012-12-13 18:29:00 +0100, Andres Freund wrote:
On 2012-12-13 00:05:41 +, Peter Geoghegan wrote:
Initialisation means finding a free “logical slot” from shared memory,
then looping until the new magic xmin horizon for logical walsenders
(stored in their “slot”) is that of the weakest link (think local
global xmin).
+* FIXME: think about solving the race conditions in a nicer way.
+*/
+ recompute_xmin:
+ walsnd-xmin = GetOldestXmin(true, true);
+ ComputeLogicalXmin();
+ if (walsnd-xmin != GetOldestXmin(true, true))
+ goto recompute_xmin;
Apart from the race conditions that I'm not confident are addressed
here, I think that the above could easily get stuck indefinitely in
the event of contention.
I don't like that part the slightest bit but I don't think its actually
in danger of looping forever. In fact I think its so broken it won't
ever loop ;). (ComputeLogicalXmin() will set the current global minimum
which will then be returned by GetOldestXmin()).
I would like to solve this properly without copying GetOldestXmin once
more (so we can compute and set the logical xmin while holding
ProcArrayLock), but I am not yet sure how a nice way to do that would
look like.
I guess GetOldestXminNoLock? That gets called while we already hold the
procarray lock? Yuck.
Does anybody have an opinion on the attached patches? Especially 0001,
which contains the procarray changes?
It moves a computation of the sort of:
result -= vacuum_defer_cleanup_age;
if (!TransactionIdIsNormal(result))
result = FirstNormalTransactionId;
inside ProcArrayLock. But I can't really imagine that to be relevant...
Another alternative to this would be to get a snapshot with
GetSnapshotData(), copy the xmin to the logical slot, then call
ProcArrayEndTransaction(). But that doesn't really seem to be nicer to
me.
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training Services
From 660b6995c0260eb112d7b6f7158e3b4654c3d9bf Mon Sep 17 00:00:00 2001
From: Andres Freund and...@anarazel.de
Date: Thu, 13 Dec 2012 20:47:57 +0100
Subject: [PATCH 1/2] Add GetOldestXminNoLock as a variant (and
implementation) of GetOldestXmin
This is useful because it allows to compute the current OldestXmin while
already holding the procarray lock which enables setting the own xmin horizon
safely.
---
src/backend/storage/ipc/procarray.c | 30 +-
src/include/storage/procarray.h | 1 +
2 files changed, 22 insertions(+), 9 deletions(-)
diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c
index 985350e..a704c9c 100644
--- a/src/backend/storage/ipc/procarray.c
+++ b/src/backend/storage/ipc/procarray.c
@@ -1104,6 +1104,27 @@ TransactionIdIsActive(TransactionId xid)
TransactionId
GetOldestXmin(bool allDbs, bool ignoreVacuum)
{
+ TransactionId res;
+
+ /* Cannot look for individual databases during recovery */
+ Assert(allDbs || !RecoveryInProgress());
+
+ LWLockAcquire(ProcArrayLock, LW_SHARED);
+ res = GetOldestXminNoLock(allDbs, ignoreVacuum);
+ LWLockRelease(ProcArrayLock);
+ return res;
+}
+
+/*
+ * GetOldestXminNoLock -- worker routine for GetOldestXmin and others
+ *
+ * Requires ProcArrayLock to be already locked!
+ *
+ * Check GetOldestXmin for the semantics of this.
+ */
+TransactionId
+GetOldestXminNoLock(bool allDbs, bool ignoreVacuum)
+{
ProcArrayStruct *arrayP = procArray;
TransactionId result;
int index;
@@ -,8 +1132,6 @@ GetOldestXmin(bool allDbs, bool ignoreVacuum)
/* Cannot look for individual databases during recovery */
Assert(allDbs || !RecoveryInProgress());
- LWLockAcquire(ProcArrayLock, LW_SHARED);
-
/*
* We initialize the MIN() calculation with latestCompletedXid + 1. This
* is a lower bound for the XIDs that might appear in the ProcArray later,
@@ -1174,8 +1193,6 @@ GetOldestXmin(bool allDbs, bool ignoreVacuum)
*/
TransactionId kaxmin = KnownAssignedXidsGetOldestXmin();
- LWLockRelease(ProcArrayLock);
-
if (TransactionIdIsNormal(kaxmin)
TransactionIdPrecedes(kaxmin, result))
result = kaxmin;
@@ -1183,11 +1200,6 @@ GetOldestXmin(bool allDbs, bool ignoreVacuum)
else
{
/*
- * No other information needed, so release the lock immediately.
- */
- LWLockRelease(ProcArrayLock);
-
- /*
* Compute the cutoff XID by subtracting vacuum_defer_cleanup_age,
* being careful not to generate a permanent XID.
*
diff --git a/src/include/storage/procarray.h b/src/include/storage/procarray.h
index 9933dad..ce8d98b 100644
--- a/src/include/storage/procarray.h
+++ b/src/include/storage/procarray.h
@@ -50,6 +50,7 @@ extern RunningTransactions GetRunningTransactionData(void);
extern bool TransactionIdIsInProgress(TransactionId xid);
extern bool TransactionIdIsActive(TransactionId xid);
extern TransactionId GetOldestXmin(bool allDbs, bool ignoreVacuum);
+extern
Re: [HACKERS] logical decoding - GetOldestXmin
On Thu, Dec 13, 2012 at 3:03 PM, Andres Freund and...@2ndquadrant.com wrote: It moves a computation of the sort of: result -= vacuum_defer_cleanup_age; if (!TransactionIdIsNormal(result)) result = FirstNormalTransactionId; inside ProcArrayLock. But I can't really imagine that to be relevant... I can. Go look at some of the 9.2 optimizations around GetSnapshotData(). Those made a BIG difference under heavy concurrency and they were definitely micro-optimization. For example, the introduction of NormalTransactionIdPrecedes() was shockingly effective. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] logical decoding - GetOldestXmin
On 2012-12-13 17:29:06 -0500, Robert Haas wrote: On Thu, Dec 13, 2012 at 3:03 PM, Andres Freund and...@2ndquadrant.com wrote: It moves a computation of the sort of: result -= vacuum_defer_cleanup_age; if (!TransactionIdIsNormal(result)) result = FirstNormalTransactionId; inside ProcArrayLock. But I can't really imagine that to be relevant... I can. Go look at some of the 9.2 optimizations around GetSnapshotData(). Those made a BIG difference under heavy concurrency and they were definitely micro-optimization. For example, the introduction of NormalTransactionIdPrecedes() was shockingly effective. But GetOldestXmin() should be called less frequently than GetSnapshotData() by several orders of magnitudes. I don't really see it being used in any really hot code paths? Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] logical decoding - GetOldestXmin
On 13 December 2012 22:37, Andres Freund and...@2ndquadrant.com wrote: On 2012-12-13 17:29:06 -0500, Robert Haas wrote: On Thu, Dec 13, 2012 at 3:03 PM, Andres Freund and...@2ndquadrant.com wrote: It moves a computation of the sort of: result -= vacuum_defer_cleanup_age; if (!TransactionIdIsNormal(result)) result = FirstNormalTransactionId; inside ProcArrayLock. But I can't really imagine that to be relevant... I can. Go look at some of the 9.2 optimizations around GetSnapshotData(). Those made a BIG difference under heavy concurrency and they were definitely micro-optimization. For example, the introduction of NormalTransactionIdPrecedes() was shockingly effective. But GetOldestXmin() should be called less frequently than GetSnapshotData() by several orders of magnitudes. I don't really see it being used in any really hot code paths? Maybe, but that calculation doesn't *need* to be inside the lock, that is just a consequence of the current coding. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] logical decoding - GetOldestXmin
On Fri, Dec 14, 2012 at 2:29 AM, Robert Haas robertmh...@gmail.com wrote: On Thu, Dec 13, 2012 at 3:03 PM, Andres Freund and...@2ndquadrant.com wrote: It moves a computation of the sort of: result -= vacuum_defer_cleanup_age; if (!TransactionIdIsNormal(result)) result = FirstNormalTransactionId; inside ProcArrayLock. But I can't really imagine that to be relevant... I can. Go look at some of the 9.2 optimizations around GetSnapshotData(). Those made a BIG difference under heavy concurrency and they were definitely micro-optimization. For example, the introduction of NormalTransactionIdPrecedes() was shockingly effective. The two commits coming to my mind are: - ed0b409 (Separate PGPROC into PGPROC and PGXACT) - 0d76b60 (introduction of NormalTransactionIdPrecedes) Those ones really improved concurrency performance. -- Michael Paquier http://michael.otacoo.com
