Re: [PATCHES] Lazy xid assignment V4
Florian G. Pflug wrote: 1) 2PC was broken in V3. I added code that skips LOCKTYPE_VIRTUALTRANSACTION locks when writing the locks to the 2PC state file, but I didn't add the same exception to the code that reassigns the locks to a dummy PGROC afterwards. So the locks weren't released at PREPARE time. Fixed now. Let me check if I got this right: We only use the lock on virtual transaction id in CREATE INDEX CONCURRENTLY, to wait until everyone that might insert to the table sees the new index. Releasing the virtual transaction id right away at PREPARE TRANSACTION, instead of reassigning it to the dummy PGPROC, is ok because the transaction can't insert anything to the table after PREPARE TRANSACTION. Sounds valid to me, but better add some comments to note that the lock is released early, in case it's going to be used for some other purpose in the future. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] Lazy xid assignment V4
Tom Lane wrote: Florian G. Pflug [EMAIL PROTECTED] writes: Here is an updated patch, following the discussion. The patch can be found at: http://soc.phlo.org/lazyxidassign.v4.patch I've been working through this, and found a couple items that seem like judgment calls: * Is there a good reason for formatting VXIDs as %d/%X rather than %d/%u? There are no other columns in pg_locks that we use hex for, so this seems a bit jarring. The reason was the desire not to bloat the string length unnecessarily. Since the first part is now %d anyways, it only really saves 2 bytes, though... * What is the rationale for keeping the transaction column in pg_locks? The useful uniqueness/join column will be virtualtransaction. I don't see a very strong backwards-compatibility argument for keeping it, because any code that depends on it being there probably thinks it's a unique key, which it cannot be anymore. One could actually make an argument to rename the virtualtransaction column as transaction. This will avoid breaking client code that thinks that the transaction column is a unique key and isn't too wedded to the assumption that the contents look like an integer. If it is so wedded, it's most likely busted anyway by the possibility that the column is null... Wouldn't code that assumes that transaction is not null be broken already, because of session locks? I left it there because they only way to get it back if we remove it is to join pg_locks on itself. Thats quite a lot of work - both in terms of typing and CPU cycles to just get that one column. I felt that if we remove the holder's xid from pg_locks, we ought to add it pg_stat_activity, probably along with the virtual transaction id. I actually wanted to do this, but then didn't because currently pg_stat_activity is rather tightly bound to the stats collector. Adding random other values seemes like a bit of a hack... However, none of these are very strong reasons - certainly weaker than doing what ensures to cause the least confusion. I'm therefore starting to think that we should remove transaction, and keep the name virtualtransaction for the VXID. That will ensure that clients who *do* rely on pg_locks and the transaction column (which will be few, I guess) at least fail early and visibly, instead of producing bogus results... If we go ahead, and rename virtualtransaction to transaction, I think we should at least put some non-numeric character in front of the virtualtransaction. Most language's string-to-integer functions will happily convert numberstring to the integer number. So if they indeed treat virtualtransaction as something int-like, they'd silently use only the backendId instead of the full VXID. greetings, Florian Pflug ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
Re: [PATCHES] Lazy xid assignment V4
Heikki Linnakangas wrote: Florian G. Pflug wrote: 1) 2PC was broken in V3. I added code that skips LOCKTYPE_VIRTUALTRANSACTION locks when writing the locks to the 2PC state file, but I didn't add the same exception to the code that reassigns the locks to a dummy PGROC afterwards. So the locks weren't released at PREPARE time. Fixed now. Let me check if I got this right: We only use the lock on virtual transaction id in CREATE INDEX CONCURRENTLY, to wait until everyone that might insert to the table sees the new index. Releasing the virtual transaction id right away at PREPARE TRANSACTION, instead of reassigning it to the dummy PGPROC, is ok because the transaction can't insert anything to the table after PREPARE TRANSACTION. Yes. Or to put it another way: We use the transaction id to wait for transactions that did on-disk changes, and the virtual transaction id to wait for transactions only found in shared memory. Since xacts mostly vanishe from shmem at PREPARE time, we drop the lock on the VXID, but keep the one on XID. Sounds valid to me, but better add some comments to note that the lock is released early, in case it's going to be used for some other purpose in the future. Yeah, more comments are always a Good Thing I guess. greetings, Florian Pflug ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
Re: [PATCHES] Lazy xid assignment V4
Florian G. Pflug [EMAIL PROTECTED] writes: However, none of these are very strong reasons - certainly weaker than doing what ensures to cause the least confusion. I'm therefore starting to think that we should remove transaction, and keep the name virtualtransaction for the VXID. That will ensure that clients who *do* rely on pg_locks and the transaction column (which will be few, I guess) at least fail early and visibly, instead of producing bogus results... Barring other objections, I'll do it that way. regards, tom lane ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] Lazy xid assignment V4
Florian G. Pflug [EMAIL PROTECTED] writes: Here is an updated patch, following the discussion. The patch can be found at: http://soc.phlo.org/lazyxidassign.v4.patch (I seems I still can't get attachments through to this list) Applied with revisions --- mostly cosmetic, but there were a couple of things that seemed really broken. In particular, I didn't trust at all your use of struct assignment to copy VXIDs into and out of PGPROC. I believe that the C compiler is entitled to implement struct assignment by bytewise memcpy, for instance, and so it wouldn't be atomic. The LocalTransactionId can be fetched or stored atomically, but you have to write it as an integer assignment to be sure that that's what happens. I also fixed sequence.c to not force XID assignment --- it can perfectly well use the LocalTransactionId for what it's doing. Also, I didn't add the proposed regression test, as it seems much too fragile --- concurrent autovacuum activity would make it fail, for instance. There are a couple of loose ends, which I'll post about separately on -hackers. regards, tom lane ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] Lazy xid assignment V4
Tom Lane wrote: Florian G. Pflug [EMAIL PROTECTED] writes: Here is an updated patch, following the discussion. The patch can be found at: http://soc.phlo.org/lazyxidassign.v4.patch (I seems I still can't get attachments through to this list) Applied with revisions --- mostly cosmetic, but there were a couple of things that seemed really broken. In particular, I didn't trust at all your use of struct assignment to copy VXIDs into and out of PGPROC. I believe that the C compiler is entitled to implement struct assignment by bytewise memcpy, for instance, and so it wouldn't be atomic. The LocalTransactionId can be fetched or stored atomically, but you have to write it as an integer assignment to be sure that that's what happens. Ah, OK - I wasn't aware of that. I also fixed sequence.c to not force XID assignment --- it can perfectly well use the LocalTransactionId for what it's doing. Full ack. I must have missed that user for GetTopTransactionId(). Anyway, thanks for fixing these things. Also, I didn't add the proposed regression test, as it seems much too fragile --- concurrent autovacuum activity would make it fail, for instance. I was aware that they are fragile, but not that they are *that* fragile ;-) Anyway they have fullfilled their duty during development, so it's only fair to let them go now... Thanks to all of you who helped with making this happen! greetings, Florian Pflug ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] Lazy xid assignment V4
[EMAIL PROTECTED] (Florian G. Pflug) writes: Chris Browne wrote: Similarly, does it seem likely that Slony-I users would need to worry about this? No.. it should have zero negative effects for Slony-I. In fact, it will be an advantage in some cases I think. I remember something about troubles with Slony-I if the in-use xids on a intermediate subscribe (one that also acts as a origin) drift too bar away from those on the master. If that still is an issue, than lazy xid assignment might help a bit - it might reduce xid consumption on that intermediate subscriber. The problem isn't usually the growth of XID numbers, but more the general notion that the open transactions tend to prevent successful vacuums from taking place on tables like pg_listener. The pointed issues with pg_listener has, we think, been mostly resolved, as Slony-I has been getting less aggressive about generating dead tuples there. During initial subscription time, there is a pretty big issue, for very large replicas as there is a single big, long-running transaction running on the origin (the transaction pulling initial table data); that is one big XID, and it has history of adversely affecting application of replication data during the catch-up period. If flurries of read-only transactions are no longer generating XIDs that are being included in snapshot information, that *may* be something of a help; for our version 2, there is already a change that excludes XIDs for rolled-back transactions, which gets it mostly around the issues with the Big Initial-Subscription-Related Transaction. In general, from a user's point of view, you only see a different if you look at pg_locks - you will now see NULLs in the transaction column, and might need to look at virtualtransaction for some use-cases. [ thinking ] It's been quite a time since I last worked with slony - but isn't there some code that tried to prevent blocking other queries by looking at pg_locks? Or was that before you could conditionally acquire a lock using SQL? Or am I totally mistaken? Anyway, if you *do* scan pg_locks, than you might want to check those parts of the code. There are no references to pg_locks, unless there's some other view that references it, so that's good news :-). -- cbbrowne,@,linuxdatabases.info http://www3.sympatico.ca/cbbrowne/rdbms.html Windows 98 Roast Specialty Blend coffee beans - just like ordinary gourmet coffee except that processing is rushed to leave in the insect larvae. Also sold under the ``Chock Full o' Bugs'' brand name... ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] Lazy xid assignment V4
On Wednesday 05 September 2007 12:56, Tom Lane wrote:
Florian G. Pflug [EMAIL PROTECTED] writes:
However, none of these are very strong reasons - certainly weaker than
doing what ensures to cause the least confusion. I'm therefore
starting to think that we should remove transaction, and keep the name
virtualtransaction for the VXID. That will ensure that clients who
*do* rely on pg_locks and the transaction column (which will be few,
I guess) at least fail early and visibly, instead of producing bogus
results...
Reading the docs, it says Every transaction holds an exclusive lock on its
virtual transaction ID for its entire duration. If a permanent ID is assigned
to the transaction (which normally happens only if the transaction changes
the state of the database), it also holds an exclusive lock on its permanent
transaction ID until it ends.
ISTM that by removing the transaction column, there is no way to see the XID
for relations thats have been updated (which by definition will have locks on
them). Am I mis-reading the docs, or have we lost that functionality?
--
Robert Treat
Build A Brighter LAMP :: Linux Apache {middleware} PostgreSQL
---(end of broadcast)---
TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] Lazy xid assignment V4
Robert Treat [EMAIL PROTECTED] writes: ISTM that by removing the transaction column, there is no way to see the XID for relations thats have been updated (which by definition will have locks on them). Am I mis-reading the docs, or have we lost that functionality? Huh? What do you mean by XID for relations that have been updated? Relations don't have XIDs. regards, tom lane ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] Lazy xid assignment V4
Robert Treat wrote: On Wednesday 05 September 2007 12:56, Tom Lane wrote: Florian G. Pflug [EMAIL PROTECTED] writes: However, none of these are very strong reasons - certainly weaker than doing what ensures to cause the least confusion. I'm therefore starting to think that we should remove transaction, and keep the name virtualtransaction for the VXID. That will ensure that clients who *do* rely on pg_locks and the transaction column (which will be few, I guess) at least fail early and visibly, instead of producing bogus results... Reading the docs, it says Every transaction holds an exclusive lock on its virtual transaction ID for its entire duration. If a permanent ID is assigned to the transaction (which normally happens only if the transaction changes the state of the database), it also holds an exclusive lock on its permanent transaction ID until it ends. ISTM that by removing the transaction column, there is no way to see the XID for relations thats have been updated (which by definition will have locks on them). Am I mis-reading the docs, or have we lost that functionality? I'm sure sure if that is what you mean - but there were two columns carrying transaction ids in pg_locks - the first was called transaction*id*, and held a transaction that either *is* locked or *is* being waited for. The second was called just transaction, and held the xid of the transaction *holding* the lock or waiting *for* the lock. Of course, for exclusive locks on xids, the first and the second xid where always the same, because nobody apart from the transaction itself ever requests an exclusive lock on it's xid. Now, the second column is replaced by virtualtransaction, holding the vxid of the transaction holding the lock. To get the real xid for the *holding* transaction, you'd have to join pg_locks to self, using the vxid as a join key. So, in essence, you get the old pg_locks format back by doing select l1.*, l2.transactionid as transaction from pg_locks l1, pg_locks l2 where l1.vxid = l2.vxid and l2.locktype = 'transaction' and l2.mode='exclusive' and l2.granted=true. Hm.. Maybe we should put that into the docs or into the release notes? greetings, Florian Pflug ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
Re: [PATCHES] Lazy xid assignment V4
Florian G. Pflug wrote: So, in essence, you get the old pg_locks format back by doing select l1.*, l2.transactionid as transaction from pg_locks l1, pg_locks l2 where l1.vxid = l2.vxid and l2.locktype = 'transaction' and l2.mode='exclusive' and l2.granted=true. Hm.. Maybe we should put that into the docs or into the release notes? or make it a system view? cheers andrew ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] Lazy xid assignment V4
Andrew Dunstan wrote: Florian G. Pflug wrote: So, in essence, you get the old pg_locks format back by doing select l1.*, l2.transactionid as transaction from pg_locks l1, pg_locks l2 where l1.vxid = l2.vxid and l2.locktype = 'transaction' and l2.mode='exclusive' and l2.granted=true. Hm.. Maybe we should put that into the docs or into the release notes? or make it a system view? In that case we can just add transaction back to pg_locks, and save the overhead of snapshotting the locking datastructures twice... The reason against it was that we changed the semantics of the column - if we just keep it there, people who use it might silently get wrong results. I think what we have now is fine for 8.3. Should we remove the lock on the xid completely in 8.4, we'll have to revisit pg_locks anway, since than the current views won't show a transactions xid at all. But let's do that when 8.4 opens, and not now. greetings, Florian Pflug ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] Lazy xid assignment V4
Andrew Dunstan [EMAIL PROTECTED] writes: Florian G. Pflug wrote: So, in essence, you get the old pg_locks format back by doing select l1.*, l2.transactionid as transaction from pg_locks l1, pg_locks l2 where l1.vxid = l2.vxid and l2.locktype = 'transaction' and l2.mode='exclusive' and l2.granted=true. You'd want some sort of left join, no doubt, else you're not going to see transactions that have not got an XID. or make it a system view? That would be a bit silly. If there's actually still a use-case for the XID column then we should just put it back. I don't actually see a reasonable use-case for it though. As Florian points out, you can get it if you really need it --- but that view is already annoyingly wide, and I'm not eager to burden it with columns that are usually useless. Also, I still agree with Florian's earlier argument that we should deliberately break any code that's depending on the transaction column. Any such code is unlikely to be prepared for the column containing nulls. regards, tom lane ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
[PATCHES] Lazy xid assignment V4
Hi Here is an updated patch, following the discussion. The patch can be found at: http://soc.phlo.org/lazyxidassign.v4.patch (I seems I still can't get attachments through to this list) Most changes are just small fixes and tweaks. Those are .) Introduced %v for log_line_prefix .) I missed a few places when I renamed ResourceOwnerId to VirtualTransactionId. (In lock.c/.h and lmgr.c/.h + a few comments). Should be fixed now. .) Fixed some typos and outdated-ness in comments. Two changes are a bit larger 1) 2PC was broken in V3. I added code that skips LOCKTYPE_VIRTUALTRANSACTION locks when writing the locks to the 2PC state file, but I didn't add the same exception to the code that reassigns the locks to a dummy PGROC afterwards. So the locks weren't released at PREPARE time. Fixed now. 2) The more I thought about the sessionId, the less I liked it. It adds a new per-backend identifier, beside pid and BackendId, only to than *still* have wraparound issues. That seems a bit stupid. I therefore got rid of the sessionId completely, and replaced it with MyBackendId. To guarantee that VirtualTransactionIds are not reused too quickly, I added an array nextLocalTransactionIds to shared memory, with one entry per backendId. That seems much cleaner - it doesn't add yet another per-backend identifier, doesn't need yet another variable initialized at backend start, and solved the wraparound problem too. It *does* cost a bit of shared memory, but not much. For max_backends=2048 the array is as large as *one* shared buffer. It doesn't need any locking, either, because each backend only ever touches it's own entry in the array. Because the backendId part of VirtualTransactionIds is now always a rather small number (= MaxBackends), and might be interesting to know, I changed the string representation to show the backendId in decimal notation. greetings, Florian Pflug ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] Lazy xid assignment V4
On 9/4/07, Florian G. Pflug [EMAIL PROTECTED] wrote: Hi Here is an updated patch, following the discussion. The patch can be found at: http://soc.phlo.org/lazyxidassign.v4.patch (I seems I still can't get attachments through to this list) I haven't been able to follow the discussions here, but do I need to worry about its interaction with HOT ? Thanks, Pavan -- Pavan Deolasee EnterpriseDB http://www.enterprisedb.com
Re: [PATCHES] Lazy xid assignment V4
Pavan Deolasee [EMAIL PROTECTED] writes: On 9/4/07, Florian G. Pflug [EMAIL PROTECTED] wrote: Here is an updated patch, following the discussion. The patch can be found at: http://soc.phlo.org/lazyxidassign.v4.patch (I seems I still can't get attachments through to this list) I haven't been able to follow the discussions here, but do I need to worry about its interaction with HOT ? There will likely be some minor patch-merging problems, but AFAICT there are no semantic conflicts to worry about. regards, tom lane ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] Lazy xid assignment V4
Pavan Deolasee wrote:
On 9/4/07, Florian G. Pflug [EMAIL PROTECTED] wrote:
Hi
Here is an updated patch, following the discussion.
The patch can be found at: http://soc.phlo.org/lazyxidassign.v4.patch
(I seems I still can't get attachments through to this list)
I haven't been able to follow the discussions here, but do I need to worry
about its interaction with HOT ?
I don't think so. The interactions should be pretty minimal.
Any transaction that actually modifies a tuple will be assigned an xid
(That happens automatically if you call Get{Current|Top}TransactionId() ).
So those work just as they used to, only that the transaction id
is assigned a bit later. But GetSnapshotData() and friends deal correctly
with that.
It's only purely read-only transactions that behave a bit different.
AFAIK, with HOT, those still might try to prune and defragment a page - but they
won't need an xid for that I guess. Even if they did - calling
Get{Current|Top}TransctionId will assign one, so things should just work.
If you scan the procarray, and then wait for some of the transactions you found,
you might need to wait on the virtual transaction id instead of the regular
transaction id (because that might be InvalidTransactionId) - just as
CREATE INDEX CONCURRENTLY does. But the infrastructure for that is already in
place, so thats a rather trivial modification.
If HOT does change the second waiting phase of CREATE INDEX CONCURRENTLY to
use the new xidcreate (or however that is called) field, than that probably
replaces my changes to that code. That's fine - I knew that HOT might make
my changes unnecessary when I coded them, but I didn't want my patch to have a
dependency on HOT - that seemed to complicate things unnecessarily. And I
wasn't sure if HOT changes only normal CREATE INDEX, or both CREATE INDEX
and CREATE INDEX CONCURRENTLY.
greetings, Florian Pflug
---(end of broadcast)---
TIP 6: explain analyze is your friend
Re: [PATCHES] Lazy xid assignment V4
[EMAIL PROTECTED] (Florian G. Pflug) writes: Pavan Deolasee wrote: On 9/4/07, Florian G. Pflug [EMAIL PROTECTED] wrote: Hi Here is an updated patch, following the discussion. The patch can be found at: http://soc.phlo.org/lazyxidassign.v4.patch (I seems I still can't get attachments through to this list) I haven't been able to follow the discussions here, but do I need to worry about its interaction with HOT ? I don't think so. The interactions should be pretty minimal. Similarly, does it seem likely that Slony-I users would need to worry about this? From what I have been seeing, I don't think so, because the transactions that cause the XIDs to get generated that are of interest will all be write transactions, and hence exempt from this: - User transactions that are writing to tables will be exempt :-). - On an origin node, slon connections that write out SYNC events every so often are, obviously, write transactions that will again be exempt. - Transactions that read data from a provider could be affected, as they are indeed read-only, but their XIDs aren't of interest. If I speculate right, then there's not much here to worry about. But best to check... And we'll certainly be testing as this gets applied... -- let name=cbbrowne and tld=acm.org in name ^ @ ^ tld;; http://linuxdatabases.info/info/finances.html Signs of a Klingon Programmer - 3. This machine is GAGH! I need dual Pentium processors if I am to do battle with this code! ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] Lazy xid assignment V4
Chris Browne wrote: Similarly, does it seem likely that Slony-I users would need to worry about this? No.. it should have zero negative effects for Slony-I. In fact, it will be an advantage in some cases I think. I remember something about troubles with Slony-I if the in-use xids on a intermediate subscribe (one that also acts as a origin) drift too bar away from those on the master. If that still is an issue, than lazy xid assignment might help a bit - it might reduce xid consumption on that intermediate subscriber. In general, from a user's point of view, you only see a different if you look at pg_locks - you will now see NULLs in the transaction column, and might need to look at virtualtransaction for some use-cases. [ thinking ] It's been quite a time since I last worked with slony - but isn't there some code that tried to prevent blocking other queries by looking at pg_locks? Or was that before you could conditionally acquire a lock using SQL? Or am I totally mistaken? Anyway, if you *do* scan pg_locks, than you might want to check those parts of the code. greetings, Florian Pflug ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] Lazy xid assignment V4
Florian G. Pflug [EMAIL PROTECTED] writes: Here is an updated patch, following the discussion. The patch can be found at: http://soc.phlo.org/lazyxidassign.v4.patch I've been working through this, and found a couple items that seem like judgment calls: * Is there a good reason for formatting VXIDs as %d/%X rather than %d/%u? There are no other columns in pg_locks that we use hex for, so this seems a bit jarring. * What is the rationale for keeping the transaction column in pg_locks? The useful uniqueness/join column will be virtualtransaction. I don't see a very strong backwards-compatibility argument for keeping it, because any code that depends on it being there probably thinks it's a unique key, which it cannot be anymore. One could actually make an argument to rename the virtualtransaction column as transaction. This will avoid breaking client code that thinks that the transaction column is a unique key and isn't too wedded to the assumption that the contents look like an integer. If it is so wedded, it's most likely busted anyway by the possibility that the column is null... Comments? regards, tom lane ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
