Thread: pg_stats_recovery view
Hi, Attached is a patch thats implements a pg_stat_recovery view that keeps counters about processed wal records. I just notice that it still lacks documentation but i will add it during the week. Because it tracks redo time this introduces to GetCurrentTimestamp() calls to the redo main loop, so i add a track_recovery GUC so only the people that wants the view has to spent time in those calls. Probably the most controversial part of the patch will be the addition of a new column in RmgrData that is a pointer to a new function for *_short_desc() these functions are similar to the *_desc functions that already exists and that is called via rm_desc but instead of giving full details about the record being processed it just inform of the type of the record, for example the 2 first columns will look something like: rmgr: XLOG wal_record_type: xlog switch -- Jaime Casanova www.2ndQuadrant.com Professional PostgreSQL: Soporte 24x7 y capacitación
Attachment
--On 15. Januar 2012 02:50:00 -0500 Jaime Casanova <jaime@2ndquadrant.com> wrote: > Attached is a patch thats implements a pg_stat_recovery view that > keeps counters about processed wal records. I just notice that it > still lacks documentation but i will add it during the week. Hi Jaime, do you have an updated patch? The current v1 patch doesn't apply cleanly anymore, and before i go and rebase the patch i thought i'm asking... -- Thanks Bernd
On Thu, Jan 26, 2012 at 4:03 AM, Bernd Helmle <mailings@oopsware.de> wrote: > > > --On 15. Januar 2012 02:50:00 -0500 Jaime Casanova <jaime@2ndquadrant.com> > wrote: > >> Attached is a patch thats implements a pg_stat_recovery view that >> keeps counters about processed wal records. I just notice that it >> still lacks documentation but i will add it during the week. > > > Hi Jaime, > > do you have an updated patch? The current v1 patch doesn't apply cleanly > anymore, and before i go and rebase the patch i thought i'm asking... > here's the patch rebased to this morning's HEAD -- Jaime Casanova www.2ndQuadrant.com Professional PostgreSQL: Soporte 24x7 y capacitación
Attachment
On Fri, Jan 27, 2012 at 6:01 AM, Jaime Casanova <jaime@2ndquadrant.com> wrote: > On Thu, Jan 26, 2012 at 4:03 AM, Bernd Helmle <mailings@oopsware.de> wrote: >> >> >> --On 15. Januar 2012 02:50:00 -0500 Jaime Casanova <jaime@2ndquadrant.com> >> wrote: >> >>> Attached is a patch thats implements a pg_stat_recovery view that >>> keeps counters about processed wal records. I just notice that it >>> still lacks documentation but i will add it during the week. >> >> >> Hi Jaime, >> >> do you have an updated patch? The current v1 patch doesn't apply cleanly >> anymore, and before i go and rebase the patch i thought i'm asking... >> > > here's the patch rebased to this morning's HEAD Before reviewing the patch, I'd like to know: what's the purpose of this view? It's only debug purpose? ISTM that most users don't care about this view at all. Regards, -- Fujii Masao NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center
On Wed, Feb 1, 2012 at 9:18 PM, Fujii Masao <masao.fujii@gmail.com> wrote: >>> >>> --On 15. Januar 2012 02:50:00 -0500 Jaime Casanova <jaime@2ndquadrant.com> >>> wrote: >>> >>>> Attached is a patch thats implements a pg_stat_recovery view that >>>> keeps counters about processed wal records. I just notice that it >>>> still lacks documentation but i will add it during the week. >>> >>> > > Before reviewing the patch, I'd like to know: what's the purpose of this view? > It's only debug purpose? ISTM that most users don't care about this view at all. > yeah! you're right. most users won't care about it... did i tell that i added a track_recovery GUC so only users that wanted pay for it? i probably did not tell that :D -- Jaime Casanova www.2ndQuadrant.com Professional PostgreSQL: Soporte 24x7 y capacitación
On Thu, Feb 2, 2012 at 08:26, Jaime Casanova <jaime@2ndquadrant.com> wrote: > On Wed, Feb 1, 2012 at 9:18 PM, Fujii Masao <masao.fujii@gmail.com> wrote: >>>> >>>> --On 15. Januar 2012 02:50:00 -0500 Jaime Casanova <jaime@2ndquadrant.com> >>>> wrote: >>>> >>>>> Attached is a patch thats implements a pg_stat_recovery view that >>>>> keeps counters about processed wal records. I just notice that it >>>>> still lacks documentation but i will add it during the week. >>>> >>>> >> >> Before reviewing the patch, I'd like to know: what's the purpose of this view? >> It's only debug purpose? ISTM that most users don't care about this view at all. >> > > yeah! you're right. most users won't care about it... did i tell that > i added a track_recovery GUC so only users that wanted pay for it? i > probably did not tell that :D I haven't looked through the code in detail, but one direct comment: do we really need/want to send this through the stats collector? It will only ever have one sender - perhaps we should just either store it in shared memory or store it locally and only send it on demand? (apologies if it already does the on-demand thing, I only spent about 30 seconds looking for it and noticed it did go through the stats collector...) -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/
On Thu, Feb 2, 2012 at 4:26 PM, Jaime Casanova <jaime@2ndquadrant.com> wrote: > On Wed, Feb 1, 2012 at 9:18 PM, Fujii Masao <masao.fujii@gmail.com> wrote: >>>> >>>> --On 15. Januar 2012 02:50:00 -0500 Jaime Casanova <jaime@2ndquadrant.com> >>>> wrote: >>>> >>>>> Attached is a patch thats implements a pg_stat_recovery view that >>>>> keeps counters about processed wal records. I just notice that it >>>>> still lacks documentation but i will add it during the week. >>>> >>>> >> >> Before reviewing the patch, I'd like to know: what's the purpose of this view? >> It's only debug purpose? ISTM that most users don't care about this view at all. >> > > yeah! you're right. most users won't care about it... did i tell that > i added a track_recovery GUC so only users that wanted pay for it? i > probably did not tell that :D If only core developer is interested in this view, ISTM that short description for each WAL record is not required because he or she can know the meaning of each WAL record by reading the source code. No? Adding short descriptions for every WAL records seems to be overkill. Regards, -- Fujii Masao NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center
On Thu, Feb 2, 2012 at 4:32 PM, Magnus Hagander <magnus@hagander.net> wrote: > On Thu, Feb 2, 2012 at 08:26, Jaime Casanova <jaime@2ndquadrant.com> wrote: >> On Wed, Feb 1, 2012 at 9:18 PM, Fujii Masao <masao.fujii@gmail.com> wrote: >>>>> >>>>> --On 15. Januar 2012 02:50:00 -0500 Jaime Casanova <jaime@2ndquadrant.com> >>>>> wrote: >>>>> >>>>>> Attached is a patch thats implements a pg_stat_recovery view that >>>>>> keeps counters about processed wal records. I just notice that it >>>>>> still lacks documentation but i will add it during the week. >>>>> >>>>> >>> >>> Before reviewing the patch, I'd like to know: what's the purpose of this view? >>> It's only debug purpose? ISTM that most users don't care about this view at all. >>> >> >> yeah! you're right. most users won't care about it... did i tell that >> i added a track_recovery GUC so only users that wanted pay for it? i >> probably did not tell that :D > > I haven't looked through the code in detail, but one direct comment: > do we really need/want to send this through the stats collector? It > will only ever have one sender - perhaps we should just either store > it in shared memory or store it locally and only send it on demand? Agreed. I think we should either. Regards, -- Fujii Masao NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center
--On 2. Februar 2012 17:12:11 +0900 Fujii Masao <masao.fujii@gmail.com> wrote: > If only core developer is interested in this view, ISTM that short > description for > each WAL record is not required because he or she can know the meaning of each > WAL record by reading the source code. No? Adding short descriptions for every > WAL records seems to be overkill. Yes, for a developer option alone adding all these *_short_desc functions looks too much code for too less benefit. However, if someone with less code affinity is interested to debug his server during recovery, it might be easier for him to interpret the statistic counters. Unfortunately i didn't manage to do it this week, but what i'm also interested in is how large the performance regression is if the track_recovery variable is activated. Not sure wether it really makes a big difference, but maybe debugging recovery from a large archive could slow down to a degree, where you want the GUC but can't afford it? And, for display purposes, when this is intended for developers only, shouldn't it be treated like all the other debug options as a DEVELOPER_OPTION, too? -- Thanks Bernd
On Thu, Feb 2, 2012 at 2:32 AM, Magnus Hagander <magnus@hagander.net> wrote: > > I haven't looked through the code in detail, but one direct comment: > do we really need/want to send this through the stats collector? It > will only ever have one sender - perhaps we should just either store > it in shared memory or store it locally and only send it on demand? > fyi, i intend to send a reworked patch later today, it will store the info locally and send it on demand. about the _short_desc functions, i added that because i wanted to understand what was happening during recovery and the wal_record_type (xl_info) being a number is not that clear -- Jaime Casanova www.2ndQuadrant.com Professional PostgreSQL: Soporte 24x7 y capacitación
Excerpts from Jaime Casanova's message of mar feb 14 04:10:58 -0300 2012: > On Thu, Feb 2, 2012 at 2:32 AM, Magnus Hagander <magnus@hagander.net> wrote: > > > > I haven't looked through the code in detail, but one direct comment: > > do we really need/want to send this through the stats collector? It > > will only ever have one sender - perhaps we should just either store > > it in shared memory or store it locally and only send it on demand? > > > > fyi, i intend to send a reworked patch later today, it will store the > info locally and send it on demand. > about the _short_desc functions, i added that because i wanted to > understand what was happening during recovery and the wal_record_type > (xl_info) being a number is not that clear Maybe it'd be clearer if you display it in hex and filter out just the bits that are interesting for this use? IIRC xl_info carries some other bits than the ones to identify the record type, which could be confusing. -- Álvaro Herrera <alvherre@commandprompt.com> The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support
On Tue, Feb 14, 2012 at 4:10 PM, Jaime Casanova <jaime@2ndquadrant.com> wrote: > On Thu, Feb 2, 2012 at 2:32 AM, Magnus Hagander <magnus@hagander.net> wrote: >> >> I haven't looked through the code in detail, but one direct comment: >> do we really need/want to send this through the stats collector? It >> will only ever have one sender - perhaps we should just either store >> it in shared memory or store it locally and only send it on demand? >> > > fyi, i intend to send a reworked patch later today, it will store the > info locally and send it on demand. Jaime, are you planning to submit the updated version of the patch? Regards, -- Fujii Masao NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center
On Fri, Mar 9, 2012 at 7:20 AM, Fujii Masao <masao.fujii@gmail.com> wrote: > On Tue, Feb 14, 2012 at 4:10 PM, Jaime Casanova <jaime@2ndquadrant.com> wrote: >> On Thu, Feb 2, 2012 at 2:32 AM, Magnus Hagander <magnus@hagander.net> wrote: >>> >>> I haven't looked through the code in detail, but one direct comment: >>> do we really need/want to send this through the stats collector? It >>> will only ever have one sender - perhaps we should just either store >>> it in shared memory or store it locally and only send it on demand? >>> >> >> fyi, i intend to send a reworked patch later today, it will store the >> info locally and send it on demand. > > Jaime, > are you planning to submit the updated version of the patch? Hearing no response, I have marked this patch Returned with Feedback. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company