Re: [HACKERS] CompactCheckpointerRequestQueue versus pad bytes

2012-07-18 Thread Robert Haas
On Tue, Jul 17, 2012 at 1:26 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Mon, Jul 16, 2012 at 9:58 PM, Tom Lane t...@sss.pgh.pa.us wrote: BTW, I wonder whether the code that checks for relfilenode conflict when selecting a pg_class or relfilenode OID

Re: [HACKERS] CompactCheckpointerRequestQueue versus pad bytes

2012-07-17 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes: On Mon, Jul 16, 2012 at 9:58 PM, Tom Lane t...@sss.pgh.pa.us wrote: BTW, I wonder whether the code that checks for relfilenode conflict when selecting a pg_class or relfilenode OID tries both file naming conventions? If not, should we make it do so?

Re: [HACKERS] CompactCheckpointerRequestQueue versus pad bytes

2012-07-17 Thread Tom Lane
I wrote: I had thought that we might get a performance boost here by saving fsync queue traffic, but I see that md.c was already not calling register_dirty_segment for temp rels, so there's no joy there. Actually, wait a second. We were smart enough to not send fsync requests in the first

Re: [HACKERS] CompactCheckpointerRequestQueue versus pad bytes

2012-07-16 Thread Amit Kapila
From: pgsql-hackers-ow...@postgresql.org [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Tom Lane Sent: Monday, July 16, 2012 3:06 AM It might have accidentally failed to fail if tested on a compiler that gives a full 32 bits to enum ForkNumber, but there absolutely would be padding

Re: [HACKERS] CompactCheckpointerRequestQueue versus pad bytes

2012-07-16 Thread Robert Haas
On Sun, Jul 15, 2012 at 5:36 PM, Tom Lane t...@sss.pgh.pa.us wrote: We could fairly cheaply dodge the problem with padding after ForkNumber if we were to zero out the whole request array at shmem initialization, so that any such pad bytes are guaranteed zero. However, padding in

Re: [HACKERS] CompactCheckpointerRequestQueue versus pad bytes

2012-07-16 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes: On Sun, Jul 15, 2012 at 5:36 PM, Tom Lane t...@sss.pgh.pa.us wrote: We could fairly cheaply dodge the problem with padding after ForkNumber if we were to zero out the whole request array at shmem initialization, so that any such pad bytes are

Re: [HACKERS] CompactCheckpointerRequestQueue versus pad bytes

2012-07-16 Thread Heikki Linnakangas
On 16.07.2012 18:24, Robert Haas wrote: On Sun, Jul 15, 2012 at 5:36 PM, Tom Lanet...@sss.pgh.pa.us wrote: We could fairly cheaply dodge the problem with padding after ForkNumber if we were to zero out the whole request array at shmem initialization, so that any such pad bytes are guaranteed

Re: [HACKERS] CompactCheckpointerRequestQueue versus pad bytes

2012-07-16 Thread Tom Lane
I wrote: Robert Haas robertmh...@gmail.com writes: So I'm having a hard time understanding under what imaginable set of circumstances this might break. Padding inside RelFileNodeBackend would break it, because ForwardFsyncRequest copies the rnode as a struct. So that's why I'm asking

Re: [HACKERS] CompactCheckpointerRequestQueue versus pad bytes

2012-07-16 Thread Robert Haas
On Mon, Jul 16, 2012 at 11:57 AM, Tom Lane t...@sss.pgh.pa.us wrote: I wrote: Robert Haas robertmh...@gmail.com writes: So I'm having a hard time understanding under what imaginable set of circumstances this might break. Padding inside RelFileNodeBackend would break it, because

Re: [HACKERS] CompactCheckpointerRequestQueue versus pad bytes

2012-07-16 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes: On Mon, Jul 16, 2012 at 11:57 AM, Tom Lane t...@sss.pgh.pa.us wrote: BTW, I'd be a lot happier about assuming that bare RelFileNode contains no padding, because that's at least got all the fields the same type. So that brings us back to the question of

Re: [HACKERS] CompactCheckpointerRequestQueue versus pad bytes

2012-07-16 Thread Robert Haas
On Mon, Jul 16, 2012 at 11:44 AM, Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: I wouldn't rely on that, though. I wouldn't be surprised if there was some debugging flag or similar that initialized all pages to random values or 0xdeadbeef or something, before handing them out to

Re: [HACKERS] CompactCheckpointerRequestQueue versus pad bytes

2012-07-16 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes: The documentation on MacOS X isn't quite as explicit, but I'd still be astonished if we found any other behavior. TBH, I'd be kind of surprised if this is the only place in our code base that relies on the initial contents of shared memory being

Re: [HACKERS] CompactCheckpointerRequestQueue versus pad bytes

2012-07-16 Thread Robert Haas
On Mon, Jul 16, 2012 at 12:27 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: The documentation on MacOS X isn't quite as explicit, but I'd still be astonished if we found any other behavior. TBH, I'd be kind of surprised if this is the only place in our code

Re: [HACKERS] CompactCheckpointerRequestQueue versus pad bytes

2012-07-16 Thread Tom Lane
I wrote: Robert Haas robertmh...@gmail.com writes: On Mon, Jul 16, 2012 at 11:57 AM, Tom Lane t...@sss.pgh.pa.us wrote: So that brings us back to the question of why this code is supporting fsync requests for local relations in the first place. Couldn't we have it ignore those, and then only

Re: [HACKERS] CompactCheckpointerRequestQueue versus pad bytes

2012-07-16 Thread Robert Haas
On Mon, Jul 16, 2012 at 2:47 PM, Tom Lane t...@sss.pgh.pa.us wrote: I wrote: Robert Haas robertmh...@gmail.com writes: On Mon, Jul 16, 2012 at 11:57 AM, Tom Lane t...@sss.pgh.pa.us wrote: So that brings us back to the question of why this code is supporting fsync requests for local relations

Re: [HACKERS] CompactCheckpointerRequestQueue versus pad bytes

2012-07-16 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes: On Mon, Jul 16, 2012 at 2:47 PM, Tom Lane t...@sss.pgh.pa.us wrote: So I started to do this, and immediately hit a roadblock: although it's clear that we can discard any attempt to fsync a backend-local relation, it's not so clear that we don't need to

Re: [HACKERS] CompactCheckpointerRequestQueue versus pad bytes

2012-07-16 Thread Robert Haas
On Mon, Jul 16, 2012 at 7:17 PM, Tom Lane t...@sss.pgh.pa.us wrote: The attached patch covers everything discussed in this thread, except for the buggy handling of stats, which I think should be fixed in a separate patch since it's only relevant to 9.2+. With respect to this chunk: + * We do

Re: [HACKERS] CompactCheckpointerRequestQueue versus pad bytes

2012-07-16 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes: With respect to this chunk: + * We do not need to go through this dance for temp relations, though, because + * we never make WAL entries for temp rels, and so a temp rel poses no threat + * to the health of a regular rel that has taken over

Re: [HACKERS] CompactCheckpointerRequestQueue versus pad bytes

2012-07-16 Thread Robert Haas
On Mon, Jul 16, 2012 at 9:58 PM, Tom Lane t...@sss.pgh.pa.us wrote: BTW, I wonder whether the code that checks for relfilenode conflict when selecting a pg_class or relfilenode OID tries both file naming conventions? If not, should we make it do so? I don't believe it does, nor do I see what

[HACKERS] CompactCheckpointerRequestQueue versus pad bytes

2012-07-15 Thread Tom Lane
CompactCheckpointerRequestQueue supposes that it can use an entry of the checkpointer request queue directly as a hash table key. This will work reliably only if there are no pad bytes in the CheckpointerRequest struct, which means in turn that neither RelFileNodeBackend nor RelFileNode can