Re: [HACKERS] pg_filedump 9.3: checksums (and a few other fixes)

2013-07-20 Thread Jeff Davis
On Wed, 2013-07-17 at 13:43 -0400, Alvaro Herrera wrote: > Tom Lane escribió: > > > My feeling about this code is that the reason we print the infomask in > > hex is so you can see exactly which bits are set if you care, and that > > the rest of the line ought to be designed to interpret the bits

Re: [HACKERS] pg_filedump 9.3: checksums (and a few other fixes)

2013-07-17 Thread Alvaro Herrera
Tom Lane escribió: > My feeling about this code is that the reason we print the infomask in > hex is so you can see exactly which bits are set if you care, and that > the rest of the line ought to be designed to interpret the bits in as > reader-friendly a way as possible. So I don't buy the noti

Re: [HACKERS] pg_filedump 9.3: checksums (and a few other fixes)

2013-07-17 Thread Tom Lane
Alvaro Herrera writes: > The one I was talking about is the second case, which prints > KEYSHR_LOCK|EXCL_LOCK to mean that there's a FOR SHARE lock. I have no > problem reading it this way, but I fear that someone unfamiliar with > these bits might be confused. On the other hand, trying to be ni

Re: [HACKERS] pg_filedump 9.3: checksums (and a few other fixes)

2013-07-17 Thread Alvaro Herrera
Tom Lane escribió: > Alvaro Herrera writes: > > Well, Tom opined in > > http://www.postgresql.org/message-id/23249.1370878...@sss.pgh.pa.us that > > the current patch is okay. I have a mild opinion that it should instead > > print only SHR_LOCK when both bits are set, and one of the others when >

Re: [HACKERS] pg_filedump 9.3: checksums (and a few other fixes)

2013-07-16 Thread Tom Lane
Josh Berkus writes: > On 07/08/2013 04:59 PM, Tom Lane wrote: >> FWIW, I think that's exactly what I did in the preliminary 9.3 patch >> that I committed to pg_filedump a few weeks ago. Could you take a look >> at what's there now and see if that's what you meant? > So, is this getting committed

Re: [HACKERS] pg_filedump 9.3: checksums (and a few other fixes)

2013-07-16 Thread Josh Berkus
On 07/08/2013 04:59 PM, Tom Lane wrote: > Alvaro Herrera writes: >> Well, Tom opined in >> http://www.postgresql.org/message-id/23249.1370878...@sss.pgh.pa.us that >> the current patch is okay. I have a mild opinion that it should instead >> print only SHR_LOCK when both bits are set, and one of

Re: [HACKERS] pg_filedump 9.3: checksums (and a few other fixes)

2013-07-08 Thread Tom Lane
Alvaro Herrera writes: > Well, Tom opined in > http://www.postgresql.org/message-id/23249.1370878...@sss.pgh.pa.us that > the current patch is okay. I have a mild opinion that it should instead > print only SHR_LOCK when both bits are set, and one of the others when > only one of them is set. Bu

Re: [HACKERS] pg_filedump 9.3: checksums (and a few other fixes)

2013-07-08 Thread Peter Geoghegan
On Mon, Jul 8, 2013 at 11:52 AM, Alvaro Herrera wrote: > Well, Tom opined in > http://www.postgresql.org/message-id/23249.1370878...@sss.pgh.pa.us that > the current patch is okay. I have a mild opinion that it should instead > print only SHR_LOCK when both bits are set, and one of the others whe

Re: [HACKERS] pg_filedump 9.3: checksums (and a few other fixes)

2013-07-08 Thread Alvaro Herrera
Peter Geoghegan escribió: > On Mon, Jul 8, 2013 at 10:28 AM, Jeff Davis wrote: > > I see this patch is still "waiting on author" in the CF. Is there > > something else needed from me, or should we move this to "ready for > > committer"? > > Well, obviously someone still needs to think through the

Re: [HACKERS] pg_filedump 9.3: checksums (and a few other fixes)

2013-07-08 Thread Peter Geoghegan
On Mon, Jul 8, 2013 at 10:28 AM, Jeff Davis wrote: > I see this patch is still "waiting on author" in the CF. Is there > something else needed from me, or should we move this to "ready for > committer"? Well, obviously someone still needs to think through the handling of the infoMask bits. Alvar

Re: [HACKERS] pg_filedump 9.3: checksums (and a few other fixes)

2013-07-08 Thread Jeff Davis
On Fri, 2013-07-05 at 22:43 -0700, Jeff Davis wrote: > On Sat, 2013-07-06 at 10:30 +0900, Satoshi Nagayasu wrote: > > Hi, > > > > It looks fine, but I have one question here. > > > > When I run pg_filedump with -k against a database cluster which > > does not support checksums, pg_filedump produc

Re: [HACKERS] pg_filedump 9.3: checksums (and a few other fixes)

2013-07-05 Thread Jeff Davis
On Sat, 2013-07-06 at 10:30 +0900, Satoshi Nagayasu wrote: > Hi, > > It looks fine, but I have one question here. > > When I run pg_filedump with -k against a database cluster which > does not support checksums, pg_filedump produced checksum error as > following. Is this expected or acceptable?

Re: [HACKERS] pg_filedump 9.3: checksums (and a few other fixes)

2013-07-05 Thread Satoshi Nagayasu
Hi, I have reviewed this patch as a CF reviewer. (2013/06/27 4:07), Jeff Davis wrote: On Mon, 2013-06-24 at 20:34 -0400, Josh Kupershmidt wrote: This patch is in the current CommitFest, does it still need to be reviewed? If so, I notice that the version in pgfoundry's CVS is rather different t

Re: [HACKERS] pg_filedump 9.3: checksums (and a few other fixes)

2013-06-28 Thread Jeff Davis
On Thu, 2013-06-27 at 15:59 +0200, Andres Freund wrote: > Maybe the trick is to add a recovery.conf option to make postgres replay > to the first restartpoint and then shutdown. At that point you can be > sure there aren't any torn pages anymore (bugs aside). > In fact that sounds like a rather use

Re: [HACKERS] pg_filedump 9.3: checksums (and a few other fixes)

2013-06-27 Thread Tom Lane
Andres Freund writes: > On 2013-06-26 21:18:49 -0700, Peter Geoghegan wrote: >> Heroku are interested in online verification of basebackups (i.e. >> using checksums to verify the integrity of heap files as they are >> backed up, with a view to relying less and less on logical backups). > Why not

Re: [HACKERS] pg_filedump 9.3: checksums (and a few other fixes)

2013-06-27 Thread Andres Freund
On 2013-06-27 09:51:07 -0400, Tom Lane wrote: > Andres Freund writes: > > On 2013-06-26 21:18:49 -0700, Peter Geoghegan wrote: > >> Heroku are interested in online verification of basebackups (i.e. > >> using checksums to verify the integrity of heap files as they are > >> backed up, with a view t

Re: [HACKERS] pg_filedump 9.3: checksums (and a few other fixes)

2013-06-27 Thread Peter Geoghegan
On Thu, Jun 27, 2013 at 12:07 AM, Peter Geoghegan wrote: >> I'm not sure what the resolution of Alvaro's concern was, so I left the >> flag reporting the same as the previous patch. > > Alvaro's concern was that the new flags added (those added by the > foreign key locks patch) do something cute w

Re: [HACKERS] pg_filedump 9.3: checksums (and a few other fixes)

2013-06-27 Thread Peter Geoghegan
On Tue, Jun 18, 2013 at 9:42 AM, Jeff Davis wrote: > I'm not sure what the resolution of Alvaro's concern was, so I left the > flag reporting the same as the previous patch. Alvaro's concern was that the new flags added (those added by the foreign key locks patch) do something cute with re-using

Re: [HACKERS] pg_filedump 9.3: checksums (and a few other fixes)

2013-06-26 Thread Andres Freund
On 2013-06-26 23:42:55 -0700, Peter Geoghegan wrote: > On Wed, Jun 26, 2013 at 11:27 PM, Andres Freund > wrote: > > Why not do this from a function/background worker in the backend where > > you can go via the buffer manager to avoid torn pages et al. If you use > > a buffer strategy the cache po

Re: [HACKERS] pg_filedump 9.3: checksums (and a few other fixes)

2013-06-26 Thread Peter Geoghegan
On Wed, Jun 26, 2013 at 11:27 PM, Andres Freund wrote: > Why not do this from a function/background worker in the backend where > you can go via the buffer manager to avoid torn pages et al. If you use > a buffer strategy the cache poisoning et al should be controlleable. I had considered that, b

Re: [HACKERS] pg_filedump 9.3: checksums (and a few other fixes)

2013-06-26 Thread Andres Freund
On 2013-06-26 21:18:49 -0700, Peter Geoghegan wrote: > On Wed, Jun 26, 2013 at 8:27 PM, Tom Lane wrote: > > TBH, I've always been annoyed that pg_filedump is GPL and so there's no > > way for us to just ship it in contrib. (That stems from Red Hat > > corporate policy of a dozen years ago, but th

Re: [HACKERS] pg_filedump 9.3: checksums (and a few other fixes)

2013-06-26 Thread Peter Geoghegan
On Wed, Jun 26, 2013 at 8:27 PM, Tom Lane wrote: > TBH, I've always been annoyed that pg_filedump is GPL and so there's no > way for us to just ship it in contrib. (That stems from Red Hat > corporate policy of a dozen years ago, but the conflict is real anyway.) > If somebody is sufficiently exc

Re: [HACKERS] pg_filedump 9.3: checksums (and a few other fixes)

2013-06-26 Thread Tom Lane
Jeff Davis writes: > On Mon, 2013-06-24 at 20:34 -0400, Josh Kupershmidt wrote: >> This patch is in the current CommitFest, does it still need to be >> reviewed? If so, I notice that the version in pgfoundry's CVS is >> rather different than the version the patch seems to have been built >> agains

Re: [HACKERS] pg_filedump 9.3: checksums (and a few other fixes)

2013-06-26 Thread Jeff Davis
On Mon, 2013-06-24 at 20:34 -0400, Josh Kupershmidt wrote: > This patch is in the current CommitFest, does it still need to be > reviewed? If so, I notice that the version in pgfoundry's CVS is > rather different than the version the patch seems to have been built > against (presumably the pg_filed

Re: [HACKERS] pg_filedump 9.3: checksums (and a few other fixes)

2013-06-25 Thread Jeff Davis
On Mon, 2013-06-24 at 20:34 -0400, Josh Kupershmidt wrote: > This patch is in the current CommitFest, does it still need to be > reviewed? If so, I notice that the version in pgfoundry's CVS is > rather different than the version the patch seems to have been built > against (presumably the pg_filed

Re: [HACKERS] pg_filedump 9.3: checksums (and a few other fixes)

2013-06-24 Thread Tom Lane
Josh Kupershmidt writes: > This patch is in the current CommitFest, does it still need to be > reviewed? If so, I notice that the version in pgfoundry's CVS is > rather different than the version the patch seems to have been built > against (presumably the pg_filedump-9.2.0.tar.gz release), and >

Re: [HACKERS] pg_filedump 9.3: checksums (and a few other fixes)

2013-06-24 Thread Josh Kupershmidt
On Tue, Jun 18, 2013 at 12:42 PM, Jeff Davis wrote: > Attached a new diff for pg_filedump that makes use of the above change. > > I'm not sure what the resolution of Alvaro's concern was, so I left the > flag reporting the same as the previous patch. This patch is in the current CommitFest, does

Re: [HACKERS] pg_filedump 9.3: checksums (and a few other fixes)

2013-06-18 Thread Jeff Davis
On Thu, 2013-06-13 at 20:09 -0400, Tom Lane wrote: > What I propose we do about this is reduce backend/storage/page/checksum.c > to something like > > #include "postgres.h" > #include "storage/checksum.h" > #include "storage/checksum_impl.h" Attached a new diff for pg_filedump that makes use of t

Re: [HACKERS] pg_filedump 9.3: checksums (and a few other fixes)

2013-06-14 Thread Tom Lane
Andres Freund writes: > On 2013-06-14 11:59:04 -0400, Tom Lane wrote: >> Ah, you are right, I forgot the #ifndef CHECKSUM_IMPL_H dance. Will fix >> in a bit. > That won't help against errors if it's included in two different > files/translation units though. Good point, but there's not any real

Re: [HACKERS] pg_filedump 9.3: checksums (and a few other fixes)

2013-06-14 Thread Andres Freund
On 2013-06-14 11:59:04 -0400, Tom Lane wrote: > Jeff Davis writes: > > I have a question about the commit though: shouldn't both functions be > > static if they are in a .h file? Otherwise, it could lead to naming > > conflicts. I suppose it's wrong to include the implementation file > > twice, bu

Re: [HACKERS] pg_filedump 9.3: checksums (and a few other fixes)

2013-06-14 Thread Tom Lane
Jeff Davis writes: > I have a question about the commit though: shouldn't both functions be > static if they are in a .h file? Otherwise, it could lead to naming > conflicts. I suppose it's wrong to include the implementation file > twice, but it still might be confusing if someone tries. Two idea

Re: [HACKERS] pg_filedump 9.3: checksums (and a few other fixes)

2013-06-14 Thread Jeff Davis
On Thu, 2013-06-13 at 20:09 -0400, Tom Lane wrote: > What I propose we do about this is reduce backend/storage/page/checksum.c > to something like > > #include "postgres.h" > #include "storage/checksum.h" > #include "storage/checksum_impl.h" > > moving all the code currently in the file into a ne

Re: [HACKERS] pg_filedump 9.3: checksums (and a few other fixes)

2013-06-13 Thread Tom Lane
Jeff Davis writes: > The patch is a bit ugly: I had to copy some code, and copy the entire > checksum.c file (minus some Asserts, which don't work in an external > program). Suggestions welcome. What I propose we do about this is reduce backend/storage/page/checksum.c to something like #include

Re: [HACKERS] pg_filedump 9.3: checksums (and a few other fixes)

2013-06-10 Thread Tom Lane
Alvaro Herrera writes: > Jeff Davis wrote: >> The CRC implementation is entirely in header files. Do you think we need >> to go that far, or is it fine to just put it in libpgport and link that >> to pg_filedump? > If a lib is okay, use libpgcommon please, not libpgport. But I think a > .h would

Re: [HACKERS] pg_filedump 9.3: checksums (and a few other fixes)

2013-06-10 Thread Alvaro Herrera
Jeff Davis wrote: > On Mon, 2013-06-10 at 11:38 -0400, Tom Lane wrote: > > The thing I'm not too happy about is having to copy the checksum code > > into pg_filedump. We just got rid of the need to do that for the CRC > > code, and here it is coming back again. Can't we rearrange the core > > che

Re: [HACKERS] pg_filedump 9.3: checksums (and a few other fixes)

2013-06-10 Thread Jeff Davis
On Mon, 2013-06-10 at 11:38 -0400, Tom Lane wrote: > The thing I'm not too happy about is having to copy the checksum code > into pg_filedump. We just got rid of the need to do that for the CRC > code, and here it is coming back again. Can't we rearrange the core > checksum code similarly to what

Re: [HACKERS] pg_filedump 9.3: checksums (and a few other fixes)

2013-06-10 Thread Tom Lane
Alvaro Herrera writes: > Jeff Davis wrote: >> I was hesitant to do too much interpretation of the bits. Do you think >> it would be better to just remove the test for XMAX_SHR_LOCK? > I don't know, but then I'm biased because I know what that specific bit > combination means. I guess someone tha

Re: [HACKERS] pg_filedump 9.3: checksums (and a few other fixes)

2013-06-10 Thread Alvaro Herrera
Jeff Davis wrote: > I was hesitant to do too much interpretation of the bits. Do you think > it would be better to just remove the test for XMAX_SHR_LOCK? I don't know, but then I'm biased because I know what that specific bit combination means. I guess someone that doesn't know is going to be s

Re: [HACKERS] pg_filedump 9.3: checksums (and a few other fixes)

2013-06-09 Thread Jeff Davis
On Mon, 2013-06-10 at 01:28 -0400, Alvaro Herrera wrote: > Hm, note that XMAX_SHR_LOCK is two bits, so when that flag is present > you will get the three lock modes displayed with the above code, which is > probably going to be misleading. htup_details.h does this: > > /* > * Use these to test w

Re: [HACKERS] pg_filedump 9.3: checksums (and a few other fixes)

2013-06-09 Thread Alvaro Herrera
Jeff Davis wrote: > --- 1000,1015 > strcat (flagString, "HASEXTERNAL|"); > if (infoMask & HEAP_HASOID) > strcat (flagString, "HASOID|"); > + if (infoMask & HEAP_XMAX_KEYSHR_LOCK) > + strcat (flagString, "XMAX_KEYSHR_LOCK|"); > if (infoMask & H

[HACKERS] pg_filedump 9.3: checksums (and a few other fixes)

2013-06-09 Thread Jeff Davis
Attached is a first draft of an update to pg_filedump for 9.3. I know pg_filedump is a pgfoundry project, but that seems like it's just there to host the download; so please excuse the slightly off-topic post here on -hackers. I made a few changes to support 9.3, which were mostly fixes related tw