On 02/03/2016 06:46 AM, Noah Misch wrote:
> On Mon, Feb 01, 2016 at 07:03:45PM +0100, Tomas Vondra wrote:
>> On 12/22/2015 03:49 PM, Noah Misch wrote:
...
>>> If the collector entered pgstat_write_statsfile() with more
>>> inquiries waiting in its socket receive buffer, it would ignore
>>> them as being too old once it finished the write and resumed
>>> message processing. Commit 187492b converted last_statrequest to
>>> a "last_statrequests" list that we wipe after each write.
>>
>> So essentially we remove the list of requests, and thus on the next
>> round we don't know the timestamp of the last request and write the
>> file again unnecessarily. Do I get that right?
>
> Essentially right. Specifically, for each database, we must remember
> the globalStats.stats_timestamp of the most recent write. It could be
> okay to forget the last request timestamp. (I now doubt I picked the
> best lines to quote, above.)
>
>> What if we instead kept the list but marked the requests as
>> 'invalid' so that we know the timestamp? In that case we'd be able
>> to do pretty much exactly what the original code did (but at per-db
>> granularity).
>
> The most natural translation of the old code would be to add a
> write_time field to struct DBWriteRequest. One can infer "invalid"
> from write_time and request_time. There are other reasonable designs,
> though.
OK, makes sense. I'll look into that.
>
>> We'd have to cleanup the list once in a while not to grow
>> excessively large, but something like removing entries older than
>> PGSTAT_STAT_INTERVAL should be enough.
>
> Specifically, if you assume the socket delivers messages in the order
> sent, you may as well discard entries having write_time at least
> PGSTAT_STAT_INTERVAL older than the most recent cutoff_time seen in a
> PgStat_MsgInquiry. That delivery order assumption does not hold in
> general, but I expect it's close enough for this purpose.
Agreed. If I get that right, it might result in some false negatives (in
the sense that we'll remove a record too early, forcing us to write the
database file again). But I expect that to be a rare case.
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services