Re: [HACKERS] pg_xlogdump --stats

2014-09-19 Thread Abhijit Menon-Sen
Hi. I've attached two patches here: 0001-Make-pg_xlogdump-record-stats-display-summary-statis.patch is my earlier patch to pg_xlogdump, rebased to master. It introduces the new rm_identify callback, but doesn't touch rm_desc. Other than rebasing to master after the INT64_FORMAT changes, I

Re: [HACKERS] pg_xlogdump --stats

2014-09-19 Thread Andres Freund
On 2014-09-19 13:24:11 +0530, Abhijit Menon-Sen wrote: diff --git a/configure.in b/configure.in index 1392277..c3c458c 100644 --- a/configure.in +++ b/configure.in @@ -1637,7 +1637,7 @@ fi # If we found long int is 64 bits, assume snprintf handles it. If # we found we need to use long

Re: [HACKERS] pg_xlogdump --stats

2014-09-19 Thread Abhijit Menon-Sen
At 2014-09-19 13:24:11 +0530, a...@2ndquadrant.com wrote: Good enough? Not quite. I've attached a small additional patch that shifts the responsibility of adding rm_name to the output from xlog_outrec to xlog_outdesc. Now we get WAL_DEBUG output like this: LOG: INSERT @ 0/16C51D0: prev

Re: [HACKERS] pg_xlogdump --stats

2014-09-19 Thread Abhijit Menon-Sen
At 2014-09-19 10:44:48 +0200, and...@2ndquadrant.com wrote: # snprintfs that use %lld, %qd, or %I64d as the format. If none of these -# work, fall back to our own snprintf emulation (which we know uses %lld). +# works, fall back to our own snprintf emulation (which we know uses %lld).

Re: [HACKERS] pg_xlogdump --stats

2014-09-19 Thread Andres Freund
Hi, On 2014-09-19 14:38:29 +0530, Abhijit Menon-Sen wrote: b) I'm not against it, but I wonder if the best way to add the SizeOfXLogRecord to the record size. It's just as much part of the FPI. And this means that the record length will be 0 even if all the record data has been

Re: [HACKERS] pg_xlogdump --stats

2014-09-19 Thread Abhijit Menon-Sen
I've attached the revised patch, split up differently: 1. Introduce rm_identify, change rm_desc, glue the two together in xlog.c 2. Introduce pg_xlogdump --stats[=record]. The requested changes (16, filter, z:, UNKNOWN) are included. The grammar nitpicking and rebase cruft is^Ware^Wain't

Re: [HACKERS] pg_xlogdump --stats

2014-09-19 Thread Abhijit Menon-Sen
At 2014-09-19 15:39:37 +0530, a...@2ndquadrant.com wrote: I hope I didn't miss anything this time. But of course I did. The attached fixup makes the output of pg_xlogdump match that of xlog_outdesc for unidentifiable records (UNKNOWN (%x)). Sorry for the inconvenience. -- Abhijit diff --git

Re: [HACKERS] pg_xlogdump --stats

2014-09-19 Thread Andres Freund
Hi, On 2014-09-19 15:39:37 +0530, Abhijit Menon-Sen wrote: I've attached the revised patch, split up differently: 1. Introduce rm_identify, change rm_desc, glue the two together in xlog.c 2. Introduce pg_xlogdump --stats[=record]. I've pushed these after some fixing up. As we

Re: [HACKERS] pg_xlogdump --stats

2014-09-19 Thread Alvaro Herrera
Andres Freund wrote: Hi, On 2014-09-19 15:39:37 +0530, Abhijit Menon-Sen wrote: I've attached the revised patch, split up differently: 1. Introduce rm_identify, change rm_desc, glue the two together in xlog.c 2. Introduce pg_xlogdump --stats[=record]. I've pushed these after

Re: [HACKERS] pg_xlogdump --stats

2014-09-19 Thread Andres Freund
On 2014-09-19 19:34:08 -0300, Alvaro Herrera wrote: Andres Freund wrote: Hi, On 2014-09-19 15:39:37 +0530, Abhijit Menon-Sen wrote: I've attached the revised patch, split up differently: 1. Introduce rm_identify, change rm_desc, glue the two together in xlog.c 2.

Re: [HACKERS] pg_xlogdump --stats

2014-09-11 Thread Abhijit Menon-Sen
At 2014-08-21 10:06:39 +0300, hlinnakan...@vmware.com wrote: Committed the patch to add INT64_MODIFIER, with minor fixes. Thank you. The new rm_identify method needs to be documented. […] Perhaps add comments to the RmgrData struct, explaining all of the methods. OK, I'll submit a patch to

Re: [HACKERS] pg_xlogdump --stats

2014-09-11 Thread Heikki Linnakangas
On 09/11/2014 11:43 AM, Abhijit Menon-Sen wrote: At 2014-08-21 10:06:39 +0300, hlinnakan...@vmware.com wrote: I think the names that rm_identify returns should match those that the rm_desc functions print. I had originally started off trying to keep the output in sync, but it doesn't work

Re: [HACKERS] pg_xlogdump --stats

2014-09-11 Thread Andres Freund
On 2014-09-11 12:14:42 +0300, Heikki Linnakangas wrote: On 09/11/2014 11:43 AM, Abhijit Menon-Sen wrote: At 2014-08-21 10:06:39 +0300, hlinnakan...@vmware.com wrote: I think the names that rm_identify returns should match those that the rm_desc functions print. I had originally started off

Re: [HACKERS] pg_xlogdump --stats

2014-09-11 Thread Heikki Linnakangas
On 09/11/2014 12:22 PM, Andres Freund wrote: On 2014-09-11 12:14:42 +0300, Heikki Linnakangas wrote: On 09/11/2014 11:43 AM, Abhijit Menon-Sen wrote: At 2014-08-21 10:06:39 +0300, hlinnakan...@vmware.com wrote: I think the names that rm_identify returns should match those that the rm_desc

Re: [HACKERS] pg_xlogdump --stats

2014-08-21 Thread Heikki Linnakangas
On 07/07/2014 10:32 PM, Abhijit Menon-Sen wrote: At 2014-07-07 09:48:44 +0200, and...@2ndquadrant.com wrote: I'd suggest only defining INT64_MODIFIER here and build INT64_FORMAT, UINT64_FORMAT ontop, in c.h. Oh, I see. I'm sorry, I misread your earlier suggestion. Regenerated patches

Re: [HACKERS] pg_xlogdump --stats

2014-07-07 Thread Andres Freund
On 2014-07-04 18:59:07 +0530, Abhijit Menon-Sen wrote: At 2014-07-04 11:34:21 +0200, and...@2ndquadrant.com wrote: I think we're going to have to redefine the PGAC_FUNC_SNPRINTF_LONG_LONG_INT_FORMAT callsite in configure.in to define INT64_MODIFIER='ll/l/I64D' I've attached a patch to

Re: [HACKERS] pg_xlogdump --stats

2014-07-07 Thread Abhijit Menon-Sen
At 2014-07-07 09:48:44 +0200, and...@2ndquadrant.com wrote: I'd suggest only defining INT64_MODIFIER here and build INT64_FORMAT, UINT64_FORMAT ontop, in c.h. Oh, I see. I'm sorry, I misread your earlier suggestion. Regenerated patches attached. Is this what you had in mind? -- Abhijit diff

Re: [HACKERS] pg_xlogdump --stats

2014-07-04 Thread Abhijit Menon-Sen
At 2014-06-30 05:19:10 +, dilip.ku...@huawei.com wrote: Please fix these issues and send the updated patch.. I will continue reviewing the patch.. Did you get anywhere with the updated patch? -- Abhijit -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make

Re: [HACKERS] pg_xlogdump --stats

2014-07-04 Thread Dilip kumar
On 04 July 2014 12:07, Abhijit Menon-Sen Wrote, -Original Message- From: Abhijit Menon-Sen [mailto:a...@2ndquadrant.com] Sent: 04 July 2014 12:07 To: Dilip kumar Cc: pgsql-hackers@postgresql.org; furu...@pm.nttdata.co.jp Subject: Re: [HACKERS] pg_xlogdump --stats At 2014-06-30 05

Re: [HACKERS] pg_xlogdump --stats

2014-07-04 Thread Abhijit Menon-Sen
At 2014-07-04 08:38:17 +, dilip.ku...@huawei.com wrote: Once you fix above erros, I think patch is ok from my side. I've attached two updated patches here, including the fix to the usage message. I'll mark this ready for committer now. (The rm_identify patch is posted separately from the

Re: [HACKERS] pg_xlogdump --stats

2014-07-04 Thread Andres Freund
On 2014-07-04 14:16:42 +0530, Abhijit Menon-Sen wrote: At 2014-07-04 08:38:17 +, dilip.ku...@huawei.com wrote: Once you fix above erros, I think patch is ok from my side. I've attached two updated patches here, including the fix to the usage message. I'll mark this ready for committer

Re: [HACKERS] pg_xlogdump --stats

2014-07-04 Thread Abhijit Menon-Sen
At 2014-07-04 10:54:08 +0200, and...@2ndquadrant.com wrote: I'm pretty sure that most committers would want to apply the rm_identity part in a separate commit first. Could you make it two patches, one introducing rm_identity, and another with xlogdump using it? Sure, attached. -- Abhijit

Re: [HACKERS] pg_xlogdump --stats

2014-07-04 Thread Andres Freund
Hi, On 2014-07-04 14:42:03 +0530, Abhijit Menon-Sen wrote: Sure, attached. +const char * +heap_identify(uint8 info) +{ + const char *id = NULL; + + switch (info XLOG_HEAP_OPMASK) + { + case XLOG_HEAP_INSERT: + id = INSERT; +

Re: [HACKERS] pg_xlogdump --stats

2014-07-04 Thread Abhijit Menon-Sen
At 2014-07-04 11:34:21 +0200, and...@2ndquadrant.com wrote: So we're leaking memory here? For --stats that could well be relevant... Will fix (I think using a static buffer would be OK here). I think we're going to have to redefine the PGAC_FUNC_SNPRINTF_LONG_LONG_INT_FORMAT callsite in

Re: [HACKERS] pg_xlogdump --stats

2014-07-04 Thread Andres Freund
On 2014-07-04 15:34:14 +0530, Abhijit Menon-Sen wrote: At 2014-07-04 11:34:21 +0200, and...@2ndquadrant.com wrote: So we're leaking memory here? For --stats that could well be relevant... Will fix (I think using a static buffer would be OK here). In that place, yes. There there's

Re: [HACKERS] pg_xlogdump --stats

2014-07-04 Thread gotoschool6g
I'm pretty sure that most committers would want to apply the rm_identity part in a separate commit first. Could you make it two patches, one introducing rm_identity, and another with xlogdump using it? Carp the code: const char * clog_identify(uint8 info) { switch (info) {

Re: [HACKERS] pg_xlogdump --stats

2014-07-04 Thread Andres Freund
On 2014-07-04 18:31:34 +0800, gotoschool6g wrote: I'm pretty sure that most committers would want to apply the rm_identity part in a separate commit first. Could you make it two patches, one introducing rm_identity, and another with xlogdump using it? Carp the code: const char

Re: [HACKERS] pg_xlogdump --stats

2014-07-04 Thread Abhijit Menon-Sen
At 2014-07-04 11:34:21 +0200, and...@2ndquadrant.com wrote: I think we're going to have to redefine the PGAC_FUNC_SNPRINTF_LONG_LONG_INT_FORMAT callsite in configure.in to define INT64_MODIFIER='ll/l/I64D' I've attached a patch to do this, and also add INT64_MODIFIER to pg_config.h.in:

Re: [HACKERS] pg_xlogdump --stats

2014-07-04 Thread Alvaro Herrera
Andres Freund wrote: On 2014-07-04 18:31:34 +0800, gotoschool6g wrote: Carp the code: const char * clog_identify(uint8 info) { switch (info) { case CLOG_ZEROPAGE: return ZEROPAGE; case CLOG_TRUNCATE: return TRUNCATE; break; }

Re: [HACKERS] pg_xlogdump --stats

2014-07-04 Thread Abhijit Menon-Sen
At 2014-07-04 13:43:46 -0400, alvhe...@2ndquadrant.com wrote: Now, I see that pg_xlogdump is checking for NULL return, but I'm not sure this is the best possible API. Note that the patched pg_xlogdump will display rm_name/0xNN for any records that rm_identify returns a NULL for. Earlier,

Re: [HACKERS] pg_xlogdump --stats

2014-07-01 Thread Marti Raudsepp
On Wed, Jun 4, 2014 at 1:47 PM, Abhijit Menon-Sen a...@2ndquadrant.com wrote: Here's a patch to make pg_xlogdump print summary statistics instead of individual records. Thanks! I had a use for this feature so I backported the (first) patch to PostgreSQL 9.3. It's a rush job so it's ugly and may

Re: [HACKERS] pg_xlogdump --stats

2014-07-01 Thread Abhijit Menon-Sen
At 2014-07-01 16:39:57 +0300, ma...@juffo.org wrote: Here's a patch to make pg_xlogdump print summary statistics instead of individual records. Thanks! I had a use for this feature so I backported the (first) patch to PostgreSQL 9.3. It's a rush job so it's ugly and may have bugs, but it

Re: [HACKERS] pg_xlogdump --stats

2014-07-01 Thread Marti Raudsepp
On Tue, Jul 1, 2014 at 11:13 PM, Abhijit Menon-Sen a...@2ndquadrant.com wrote: In CF terms, did you form any opinion while porting the patch I posted about whether it's sensible/ready for inclusion in 9.5? I didn't look at the code more than necessary to make the build work. As far as

Re: [HACKERS] pg_xlogdump --stats

2014-07-01 Thread Abhijit Menon-Sen
At 2014-07-02 04:20:31 +0300, ma...@juffo.org wrote: As far as functionality goes, it does exactly what I needed it to do; the output is very clear. Good to hear. You might also add units (kB/MB) to the table like pg_size_pretty, although that would make the magnitudes harder to gauge. I

Re: [HACKERS] pg_xlogdump --stats

2014-07-01 Thread Craig Ringer
On 07/02/2014 09:20 AM, Marti Raudsepp wrote: You might also add units (kB/MB) to the table like pg_size_pretty, although that would make the magnitudes harder to gauge. What 'du' does, or a subset of, may be worthwhile. -k: kB -b: blocks -m: MB -h: human-readable auto-scaling though I think

Re: [HACKERS] pg_xlogdump --stats

2014-06-30 Thread Abhijit Menon-Sen
At 2014-06-30 05:19:10 +, dilip.ku...@huawei.com wrote: I have started reviewing the patch.. Thanks. 1. Patch applied to git head cleanly. 2. Compiled in Linux -- Some warnings same as mentioned by furuyao I've attached an updated version of the patch which should fix the warnings by

Re: [HACKERS] pg_xlogdump --stats

2014-06-30 Thread Abhijit Menon-Sen
At 2014-06-30 12:05:06 +0530, a...@2ndquadrant.com wrote: It may be that the best thing to do would be to avoid using optional_argument altogether, and have separate --stats and --stats-per-record options. Thoughts? That's what I've done in the attached patch, except I've called the new

Re: [HACKERS] pg_xlogdump --stats

2014-06-29 Thread Dilip kumar
On 13 June 2014 13:01, Abhijit Menon-Sen Wrote I've changed this to use %zu at Álvaro's suggestion. I'll post an updated patch after I've finished some (unrelated) refactoring. I have started reviewing the patch.. 1. Patch applied to git head cleanly. 2. Compiled in Linux -- Some warnings

Re: [HACKERS] pg_xlogdump --stats

2014-06-13 Thread Abhijit Menon-Sen
At 2014-06-10 18:04:13 +0900, furu...@pm.nttdata.co.jp wrote: The function works fine. It is a good to the learning of PostgreSQL. Thanks for having a look. pg_xlogdump.c: In function ‘XLogDumpStatsRow’: pg_xlogdump.c:851: warning: format ‘%20llu’ expects type ‘long long unsigned int’, but

Re: [HACKERS] pg_xlogdump --stats

2014-06-10 Thread furuyao
-Original Message- From: pgsql-hackers-ow...@postgresql.org [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Abhijit Menon-Sen Sent: Wednesday, June 04, 2014 7:47 PM To: pgsql-hackers@postgresql.org Subject: [HACKERS] pg_xlogdump --stats Hi. Here's a patch to make