Re: [HACKERS] [COMMITTERS] pgsql: Introduce latches.
Gurjeet Singh writes: >> Portions Copyright (c) 1994, Regents of the University of California > Is that line needed for new files introduced by the Community? If you can show clearly that no part of a new file has any connection to the historical Postgres code base, then no you wouldn't need to include such a credit. But that seems like an improbable assumption for most new code added to Postgres. By and large, I think we should acknowledge our connections to the work that's gone before, not try to deny that there's any. regards, tom lane -- 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] including backend ID in relpath of temp rels - updated patch
On Thu, Aug 12, 2010 at 1:29 PM, Robert Haas wrote: >> Lastly, it bothers me that you've put in code to delete files belonging >> to temp rels during crash restart, without any code to clean up their >> catalog entries. This will therefore lead to dangling pg_class >> references, with uncertain but probably not very nice consequences. >> I think that probably shouldn't go in until there's code to drop the >> catalog entries too. > > I thought about this pretty carefully, and I don't believe that there > are any unpleasant consequences. The code that assigns relfilenode > numbers is pretty careful to check that the newly assigned value is > unused BOTH in pg_class and in the directory where the file will be > created, so there should be no danger of a number getting used over > again while the catalog entries remain. Also, the drop-object code > doesn't mind that the physical storage doesn't exist; it's perfectly > happy with that situation. It is true that you will get an ERROR if > you try to SELECT from a table whose physical storage has been > removed, but that seems OK. After further reflection, I think that the above reasoning is wrong. GetNewRelFileNode() guarantees that the OID doesn't collide with the OID column of pg_class, not the relfilenode column of pg_class; and, per the comments to that function, it only guarantees this when creating a new relation (to which we're therefore assigning an OID) and not when rewriting an existing relation. So it provides no protection against the scenario where backend #1 drops a table that lacks physical storage just as backend #2 assigns that OID to some other relation and begins creating files, which backend #1 then blows away. However, I think we're safe for a different reason. The only time we unlink the physical storage of a relation without nuking its catalog entries is during the startup sequence, when we wipe out the physical storage for any leftover temp tables. So if there's a race condition, it can only apply to temp tables. But temp tables for a particular backend ID can only be created by that backend, and before a backend will create any temp tables it will drop any previously existing temp schema. Thus, by the time a temp table can get created, there CAN'T be any leftover catalog entries from previous sessions for it to potentially collide with. I think the reasoning above probably should be added to the comment at the beginning of GetNewRelFileNode(), and I'll go do that unless someone thinks it's incorrect. The first sentence of that comment is also now technically incorrect: what it now does is generate a relfilenode such that is unique. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres 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] [COMMITTERS] pgsql: Introduce latches.
Portions Copyright (c) 1994, Regents of the University of California
Is that line needed for new files introduced by the Community?
Regards,
On Sat, Sep 11, 2010 at 11:48 AM, Heikki Linnakangas
wrote:
> Log Message:
> ---
> Introduce latches. A latch is a boolean variable, with the capability to
> wait until it is set. Latches can be used to reliably wait until a signal
> arrives, which is hard otherwise because signals don't interrupt select()
> on some platforms, and even when they do, there's race conditions.
>
> On Unix, latches use the so called self-pipe trick under the covers to
> implement the sleep until the latch is set, without race conditions. On
> Windows, Windows events are used.
>
> Use the new latch abstraction to sleep in walsender, so that as soon as
> a transaction finishes, walsender is woken up to immediately send the WAL
> to the standby. This reduces the latency between master and standby, which
> is good.
>
> Preliminary work by Fujii Masao. The latch implementation is by me, with
> helpful comments from many people.
>
> Modified Files:
> --
>pgsql:
>configure (r1.685 -> r1.686)
>(
> http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/configure?r1=1.685&r2=1.686
> )
>configure.in (r1.633 -> r1.634)
>(
> http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/configure.in?r1=1.633&r2=1.634
> )
>pgsql/src/backend/access/transam:
>twophase.c (r1.63 -> r1.64)
>(
> http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/access/transam/twophase.c?r1=1.63&r2=1.64
> )
>xact.c (r1.298 -> r1.299)
>(
> http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/access/transam/xact.c?r1=1.298&r2=1.299
> )
>pgsql/src/backend/port:
>Makefile (r1.28 -> r1.29)
>(
> http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/port/Makefile?r1=1.28&r2=1.29
> )
>pgsql/src/backend/replication:
>walsender.c (r1.29 -> r1.30)
>(
> http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/replication/walsender.c?r1=1.29&r2=1.30
> )
>pgsql/src/backend/storage/ipc:
>ipci.c (r1.104 -> r1.105)
>(
> http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/storage/ipc/ipci.c?r1=1.104&r2=1.105
> )
>procsignal.c (r1.7 -> r1.8)
>(
> http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/storage/ipc/procsignal.c?r1=1.7&r2=1.8
> )
>pgsql/src/include/replication:
>walsender.h (r1.4 -> r1.5)
>(
> http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/include/replication/walsender.h?r1=1.4&r2=1.5
> )
>pgsql/src/tools/msvc:
>Mkvcbuild.pm (r1.59 -> r1.60)
>(
> http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/tools/msvc/Mkvcbuild.pm?r1=1.59&r2=1.60
> )
>
> Added Files:
> ---
>pgsql/src/backend/port:
>unix_latch.c (r1.1)
>(
> http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/port/unix_latch.c?rev=1.1&content-type=text/x-cvsweb-markup
> )
>win32_latch.c (r1.1)
>(
> http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/port/win32_latch.c?rev=1.1&content-type=text/x-cvsweb-markup
> )
>pgsql/src/include/storage:
>latch.h (r1.1)
>(
> http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/include/storage/latch.h?rev=1.1&content-type=text/x-cvsweb-markup
> )
>
> --
> Sent via pgsql-committers mailing list (pgsql-committ...@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-committers
>
--
gurjeet.singh
@ EnterpriseDB - The Enterprise Postgres Company
http://www.EnterpriseDB.com
singh.gurj...@{ gmail | yahoo }.com
Twitter/Skype: singh_gurjeet
Mail sent from my BlackLaptop device
Re: [HACKERS] "serializable" in comments and names
On 09/09/2010 09:11 AM, Joe Conway wrote: > The attached patch is updated for the various comments, as well as some > wording tweaks by me. If there are no objections I'd like to commit this > in a day or two. Committed. Joe -- Joe Conway credativ LLC: http://www.credativ.us Linux, PostgreSQL, and general Open Source Training, Service, Consulting, & 24x7 Support signature.asc Description: OpenPGP digital signature
Re: Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)
On 11/09/10 18:02, Tom Lane wrote: Heikki Linnakangas writes: Barring any last-minute objections, I'll commit this in the next few days. This patch doesn't affect walreceiver yet; I think the next step is to use the latches to eliminate the polling loop in walreceiver too, so that as soon as a piece of WAL is fsync'd to disk in the standby, it's applied. I will do some work as well once it's in. Since I was the one complaining about extra wakeups in the other processes, I'm willing to go fix 'em. Committed. I'll take a look at making walreceiver respond quickly when WAL arrives in the standby, using latches, but that shouldn't interfere with what you're doing. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)
Heikki Linnakangas writes: > On 06/09/10 19:27, Heikki Linnakangas wrote: >> It seems that NumSharedLatches() is entirely wrongly placed if it's in >> the win32 specific code! That needs to be somewhere shared, so people find >> it, > Yeah. There's a notice of that in OwnLatch(), but it would be nice if we > could make it even more prominent. One idea is to put in latch.h as: > #define NumSharedLatches() (max_wal_senders /* + something else in the > future */ ) > When it's a #define, we don't need to put #include "walsender.h" in > latch.h, it's enough to put it in win32_latch.c. It's a bit weird to > have a #define in one header file that doesn't work unless you #include > another header file in where you use it, but it would work. We have other precedents for that. But having said that ... > Any opinions > on whether that's better than having NumSharedLatches() defined in the > Win32-specific win32_latch.c file? I'm inclined to leave it as it is, in > win32_latch.c, but I'm not sure. I'd leave it alone. I see no very good reason to expose NumSharedLatches globally. > Barring any last-minute objections, I'll commit this in the next few > days. This patch doesn't affect walreceiver yet; I think the next step > is to use the latches to eliminate the polling loop in walreceiver too, > so that as soon as a piece of WAL is fsync'd to disk in the standby, > it's applied. I will do some work as well once it's in. Since I was the one complaining about extra wakeups in the other processes, I'm willing to go fix 'em. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: Interruptible sleeps (was Re: [HACKERS] CommitFest 2009-07: Yay, Kevin! Thanks, reviewers!)
On 06/09/10 19:27, Heikki Linnakangas wrote: On 06/09/10 17:18, Tom Lane wrote: BTW, on reflection the AcquireLatch/ReleaseLatch terminology seems a bit ill chosen: ReleaseLatch sounds way too much like something that would just unlock or clear the latch. Perhaps OwnLatch/DisownLatch, or something along that line. Yeah, I see what you mean. Although, maybe it's just me but Own/Disown looks ugly. Anyone have a better suggestion? Magnus suggested AssociateLatch, given that the description of the function is that it associates the latch with the current process. I went with Own/Disown after all, it feels more precise, and having made the changes it doesn't look that ugly to me anymore. Here's an updated patch, with all the issues reported this far fixed, except for that naming issue, and Fujii's suggestion to use poll() instead of select() where available. I've also polished it quite a bit, improving comments etc. Magnus, can you take a look at the Windows implementation to check that it's sane? At least it seems to work. We discussed the patch over IM, there's one point minor point I'd like to get into the archives: It seems that NumSharedLatches() is entirely wrongly placed if it's in the win32 specific code! That needs to be somewhere shared, so people find it, Yeah. There's a notice of that in OwnLatch(), but it would be nice if we could make it even more prominent. One idea is to put in latch.h as: #define NumSharedLatches() (max_wal_senders /* + something else in the future */ ) When it's a #define, we don't need to put #include "walsender.h" in latch.h, it's enough to put it in win32_latch.c. It's a bit weird to have a #define in one header file that doesn't work unless you #include another header file in where you use it, but it would work. Any opinions on whether that's better than having NumSharedLatches() defined in the Win32-specific win32_latch.c file? I'm inclined to leave it as it is, in win32_latch.c, but I'm not sure. Barring any last-minute objections, I'll commit this in the next few days. This patch doesn't affect walreceiver yet; I think the next step is to use the latches to eliminate the polling loop in walreceiver too, so that as soon as a piece of WAL is fsync'd to disk in the standby, it's applied. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
