Re: PATCH: tracking temp files in pg_stat_database

From: "Tomas Vondra" <tv(at)fuzzy(dot)cz>
To: "Magnus Hagander" <magnus(at)hagander(dot)net>
Cc: "Tomas Vondra" <tv(at)fuzzy(dot)cz>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: PATCH: tracking temp files in pg_stat_database
Date: 2012-01-21 19:12:39
Message-ID: b02369ae3bb76125688ee5ce86c8bcd4.squirrel@sq.gransy.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 21 Leden 2012, 18:13, Magnus Hagander wrote:
> On Sat, Jan 21, 2012 at 18:02, Tomas Vondra <tv(at)fuzzy(dot)cz> wrote:
>> On 20 Leden 2012, 13:23, Magnus Hagander wrote:
>>> I'm wondering if this (and also my deadlocks stats patch that's int he
>>> queue) should instead of inventing new pgstats messages, add fields to
>>> the tabstat message. It sounds like that one is just for tables, but
>>> it's already the one collecting info about commits and rollbacks, and
>>> it's already sent on every commit.
>>
>> Hmmm, I'm not against that, but I'd recommend changing the message name
>> to
>> something that reflects the reality. If it's not just about table
>> statistics, it should not be named 'tabstats' IMHO. Or maybe split that
>> into two messages, both sent at the commit time.
>
> Yes, renaming it might be a good idea. If you split it into two
> messages that would defeat much of the point.

Not really, if combined with the 'send after each N updates or at the
transaction end' because then those parts might be sent independently
(thus not transmitting the whole message).

> Though I just looked at the tabstat code again, and we already split
> that message up at regular intervals. Which would make it quite weird
> to have global counters in it as well.
>
> But instead of there, perhaps we need a general "non table, but more
> than one type of data" message sent out at the same time. There is
> other stuff in the queue for it.
>
> I'm not convinced either way - I'm not against the original way in
> your patch either. I just wanted to throw the idea out there, and was
> hoping somebody else would have an opinion too :-)

Let's make that in a separate patch. It seems like a good thing to do, but
I don't think it should be mixed with this patch.

>> I do like the idea of not sending the message for each temp file,
>> though.
>> One thing that worries me are long running transactions (think about a
>> batch process that runs for several hours within a single transaction).
>> By
>> sending the data only at the commit, such transactions would not be
>> accounted properly. So I'd suggest sending the message either at commit
>
> By that argument they are *already* not accounted properly, because
> the "number of rows" and those counters are wrong. By sending the temp
> file data in the middle of the transaction, you won't be able to
> correlate those numbers with the temp file usage.
>
> I'm not saying the other usecase isn't more common, but whichever way
> you do it, it's going to get inconsistent with *something*.

The question is whether the patch should be modified to send the message
only at commit (i.e. just like tabstats) or if it can remain the way it is
now. And maybe we could change the way all those messages are sent (e.g.
to the regular submits I've proposed in the previous post) to make them
more consistent.

I'm not asking for 100% consistency. What I don't like is a batch process
consuming resources but not reflected in the stats until the final commit.
With a huge spike in the stats right after that, although the activity was
actually spread over several hours.

Tomas

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Noah Misch 2012-01-21 20:42:23 Re: [PATCH] Support for foreign keys with arrays
Previous Message Peter Eisentraut 2012-01-21 18:06:15 Re: lots of unused variable warnings in assert-free builds