Re: pg_stat directory and pg_stat_statements

Hi,

Thanks to 187492b6c2e8cafc5b39063ca3b67846e8155d24, pgstat
files are now saved to $PGDATA/pg_stat directory at shutdown.
But pg_stat_statements file is saved under $PGDATA/global yet.
Is this intentional or just oversight? Saving that file to global is
harmless, though.

Regards,

--
Fujii Masao

On Wed, May 28, 2014 at 7:01 AM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
> But pg_stat_statements file is saved under $PGDATA/global yet.
> Is this intentional or just oversight?

I think it's an oversight.

--
Peter Geoghegan

On Thu, May 29, 2014 at 12:37 AM, Peter Geoghegan <pg(at)heroku(dot)com> wrote:
> On Wed, May 28, 2014 at 7:01 AM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
>> But pg_stat_statements file is saved under $PGDATA/global yet.
>> Is this intentional or just oversight?
>
>
> I think it's an oversight.

OK, patch attached.

I'm afraid that it's not okay to change the file layout in $PGDATA at this beta1
stage because that change basically seems to need initdb. Otherwise something
like "no such file or directory" error can happen. But in this case what we need
to change is only the location of the pg_stat_statements permanent stats file.
So, without initdb, the server will not be able to find the
pg_stat_statements stats
file, but this is not so harmful. Only the problem is that the
pg_stat_statements
stats which were collected in past would disappear. OTOH, the server can keep
running successfully from then and no critical data will not
disappear. Therefore
I think we can commit this patch even at beta1. Thought?

Regards,

--
Fujii Masao

On 28.5.2014 19:52, Fujii Masao wrote:
> On Thu, May 29, 2014 at 12:37 AM, Peter Geoghegan <pg(at)heroku(dot)com> wrote:
>> On Wed, May 28, 2014 at 7:01 AM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
>>> But pg_stat_statements file is saved under $PGDATA/global yet.
>>> Is this intentional or just oversight?
>>
>>
>> I think it's an oversight.
>
> OK, patch attached.
>
> I'm afraid that it's not okay to change the file layout in $PGDATA at this beta1
> stage because that change basically seems to need initdb. Otherwise something
> like "no such file or directory" error can happen. But in this case what we need
> to change is only the location of the pg_stat_statements permanent stats file.
> So, without initdb, the server will not be able to find the
> pg_stat_statements stats
> file, but this is not so harmful. Only the problem is that the
> pg_stat_statements
> stats which were collected in past would disappear. OTOH, the server can keep
> running successfully from then and no critical data will not
> disappear. Therefore
> I think we can commit this patch even at beta1. Thought?

For HEAD, probably yes. But what about backpatching 9.3?

Tomas

On Thu, May 29, 2014 at 4:55 AM, Tomas Vondra <tv(at)fuzzy(dot)cz> wrote:
> On 28.5.2014 19:52, Fujii Masao wrote:
>> On Thu, May 29, 2014 at 12:37 AM, Peter Geoghegan <pg(at)heroku(dot)com> wrote:
>>> On Wed, May 28, 2014 at 7:01 AM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
>>>> But pg_stat_statements file is saved under $PGDATA/global yet.
>>>> Is this intentional or just oversight?
>>>
>>>
>>> I think it's an oversight.
>>
>> OK, patch attached.
>>
>> I'm afraid that it's not okay to change the file layout in $PGDATA at this beta1
>> stage because that change basically seems to need initdb. Otherwise something
>> like "no such file or directory" error can happen. But in this case what we need
>> to change is only the location of the pg_stat_statements permanent stats file.
>> So, without initdb, the server will not be able to find the
>> pg_stat_statements stats
>> file, but this is not so harmful. Only the problem is that the
>> pg_stat_statements
>> stats which were collected in past would disappear. OTOH, the server can keep
>> running successfully from then and no critical data will not
>> disappear. Therefore
>> I think we can commit this patch even at beta1. Thought?
>
> For HEAD, probably yes. But what about backpatching 9.3?

I think No. So we should not backpatch this to 9.3.

Regards,

--
Fujii Masao

On Thu, May 29, 2014 at 8:52 AM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:

> On Thu, May 29, 2014 at 4:55 AM, Tomas Vondra <tv(at)fuzzy(dot)cz> wrote:
> > On 28.5.2014 19:52, Fujii Masao wrote:
> >> On Thu, May 29, 2014 at 12:37 AM, Peter Geoghegan <pg(at)heroku(dot)com>
> wrote:
> >>> On Wed, May 28, 2014 at 7:01 AM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com>
> wrote:
> >>>> But pg_stat_statements file is saved under $PGDATA/global yet.
> >>>> Is this intentional or just oversight?
> >>>
> >>>
> >>> I think it's an oversight.
> >>
> >> OK, patch attached.
> >>
> >> I'm afraid that it's not okay to change the file layout in $PGDATA at
> this beta1
> >> stage because that change basically seems to need initdb. Otherwise
> something
> >> like "no such file or directory" error can happen. But in this case
> what we need
> >> to change is only the location of the pg_stat_statements permanent
> stats file.
> >> So, without initdb, the server will not be able to find the
> >> pg_stat_statements stats
> >> file, but this is not so harmful. Only the problem is that the
> >> pg_stat_statements
> >> stats which were collected in past would disappear. OTOH, the server
> can keep
> >> running successfully from then and no critical data will not
> >> disappear. Therefore
> >> I think we can commit this patch even at beta1. Thought?
> >
> > For HEAD, probably yes. But what about backpatching 9.3?
>
> I think No. So we should not backpatch this to 9.3.
>
Just curious.
Will it work in upgrade scenario?

--

Thanks & Regards,
Ashesh Vashi

>
> Regards,
>
> --
> Fujii Masao
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>

On Thu, May 29, 2014 at 1:01 PM, Ashesh Vashi
<ashesh(dot)vashi(at)enterprisedb(dot)com> wrote:
> On Thu, May 29, 2014 at 8:52 AM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
>>
>> On Thu, May 29, 2014 at 4:55 AM, Tomas Vondra <tv(at)fuzzy(dot)cz> wrote:
>> > On 28.5.2014 19:52, Fujii Masao wrote:
>> >> On Thu, May 29, 2014 at 12:37 AM, Peter Geoghegan <pg(at)heroku(dot)com>
>> >> wrote:
>> >>> On Wed, May 28, 2014 at 7:01 AM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com>
>> >>> wrote:
>> >>>> But pg_stat_statements file is saved under $PGDATA/global yet.
>> >>>> Is this intentional or just oversight?
>> >>>
>> >>>
>> >>> I think it's an oversight.
>> >>
>> >> OK, patch attached.
>> >>
>> >> I'm afraid that it's not okay to change the file layout in $PGDATA at
>> >> this beta1
>> >> stage because that change basically seems to need initdb. Otherwise
>> >> something
>> >> like "no such file or directory" error can happen. But in this case
>> >> what we need
>> >> to change is only the location of the pg_stat_statements permanent
>> >> stats file.
>> >> So, without initdb, the server will not be able to find the
>> >> pg_stat_statements stats
>> >> file, but this is not so harmful. Only the problem is that the
>> >> pg_stat_statements
>> >> stats which were collected in past would disappear. OTOH, the server
>> >> can keep
>> >> running successfully from then and no critical data will not
>> >> disappear. Therefore
>> >> I think we can commit this patch even at beta1. Thought?
>> >
>> > For HEAD, probably yes. But what about backpatching 9.3?
>>
>> I think No. So we should not backpatch this to 9.3.
>
> Just curious.
> Will it work in upgrade scenario?

You're concerned about the scenario using pg_upgrade? I'm not sure the detail
of pg_upgrade. But if it doesn't work properly, we should have gotten
the trouble
when 9.3 (which pg_stat directory was introduced) was released. But I've not
heard such trouble....

Regards,

--
Fujii Masao

On Wed, May 28, 2014 at 10:49 PM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
> You're concerned about the scenario using pg_upgrade? I'm not sure the detail
> of pg_upgrade. But if it doesn't work properly, we should have gotten
> the trouble

I'm not worried about pg_upgrade, because by design pg_stat_statements
will discard stats files that originated in earlier versions. However,
I don't see a need to change pg_stat_statements to serialize its
statistics to disk in the pg_stat directory before we branch off 9.4.
As you mentioned, it's harmless.

--
Peter Geoghegan

On Thu, May 29, 2014 at 11:32 AM, Peter Geoghegan <pg(at)heroku(dot)com> wrote:

> On Wed, May 28, 2014 at 10:49 PM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com>
> wrote:
> > You're concerned about the scenario using pg_upgrade?
>
Yeah - I was.

> I'm not sure the detail
> > of pg_upgrade. But if it doesn't work properly, we should have gotten
> > the trouble
>
> I'm not worried about pg_upgrade, because by design pg_stat_statements
> will discard stats files that originated in earlier versions. However,
> I don't see a need to change pg_stat_statements to serialize its
> statistics to disk in the pg_stat directory before we branch off 9.4.
> As you mentioned, it's harmless.
>
K.
I was just curious about the scenario.
If it was discarding the stats files that originated in earlier version, It
should be ok.

--

Thanks & Regards,
Ashesh Vashi

>
> --
> Peter Geoghegan
>

On Thu, May 29, 2014 at 3:02 PM, Peter Geoghegan <pg(at)heroku(dot)com> wrote:
> On Wed, May 28, 2014 at 10:49 PM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
>> You're concerned about the scenario using pg_upgrade? I'm not sure the detail
>> of pg_upgrade. But if it doesn't work properly, we should have gotten
>> the trouble
>
> I'm not worried about pg_upgrade, because by design pg_stat_statements
> will discard stats files that originated in earlier versions. However,
> I don't see a need to change pg_stat_statements to serialize its
> statistics to disk in the pg_stat directory before we branch off 9.4.
> As you mentioned, it's harmless.

Yeah, that's an idea. OTOH, there is no *strong* reason to postpone
the fix to 9.5. So I just feel inclined to apply the fix now...

Regards,

--
Fujii Masao

On 2014-06-02 22:59:55 +0900, Fujii Masao wrote:
> On Thu, May 29, 2014 at 3:02 PM, Peter Geoghegan <pg(at)heroku(dot)com> wrote:
> > On Wed, May 28, 2014 at 10:49 PM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
> >> You're concerned about the scenario using pg_upgrade? I'm not sure the detail
> >> of pg_upgrade. But if it doesn't work properly, we should have gotten
> >> the trouble
> >
> > I'm not worried about pg_upgrade, because by design pg_stat_statements
> > will discard stats files that originated in earlier versions. However,
> > I don't see a need to change pg_stat_statements to serialize its
> > statistics to disk in the pg_stat directory before we branch off 9.4.
> > As you mentioned, it's harmless.
>
> Yeah, that's an idea. OTOH, there is no *strong* reason to postpone
> the fix to 9.5. So I just feel inclined to apply the fix now...

+1 for fixing it now.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On Mon, Jun 2, 2014 at 11:07 PM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
> On 2014-06-02 22:59:55 +0900, Fujii Masao wrote:
>> On Thu, May 29, 2014 at 3:02 PM, Peter Geoghegan <pg(at)heroku(dot)com> wrote:
>> > On Wed, May 28, 2014 at 10:49 PM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
>> >> You're concerned about the scenario using pg_upgrade? I'm not sure the detail
>> >> of pg_upgrade. But if it doesn't work properly, we should have gotten
>> >> the trouble
>> >
>> > I'm not worried about pg_upgrade, because by design pg_stat_statements
>> > will discard stats files that originated in earlier versions. However,
>> > I don't see a need to change pg_stat_statements to serialize its
>> > statistics to disk in the pg_stat directory before we branch off 9.4.
>> > As you mentioned, it's harmless.
>>
>> Yeah, that's an idea. OTOH, there is no *strong* reason to postpone
>> the fix to 9.5. So I just feel inclined to apply the fix now...
>
> +1 for fixing it now.
+1. A beta is here for that as well.
--
Michael

On Mon, Jun 2, 2014 at 4:07 PM, Andres Freund <andres(at)2ndquadrant(dot)com>
wrote:

> On 2014-06-02 22:59:55 +0900, Fujii Masao wrote:
> > On Thu, May 29, 2014 at 3:02 PM, Peter Geoghegan <pg(at)heroku(dot)com> wrote:
> > > On Wed, May 28, 2014 at 10:49 PM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com>
> wrote:
> > >> You're concerned about the scenario using pg_upgrade? I'm not sure
> the detail
> > >> of pg_upgrade. But if it doesn't work properly, we should have gotten
> > >> the trouble
> > >
> > > I'm not worried about pg_upgrade, because by design pg_stat_statements
> > > will discard stats files that originated in earlier versions. However,
> > > I don't see a need to change pg_stat_statements to serialize its
> > > statistics to disk in the pg_stat directory before we branch off 9.4.
> > > As you mentioned, it's harmless.
> >
> > Yeah, that's an idea. OTOH, there is no *strong* reason to postpone
> > the fix to 9.5. So I just feel inclined to apply the fix now...
>
> +1 for fixing it now.
>
>
+1 for fixing it for 9.4 before the next beta, but *not* backpatching it to
9.3 - it *is* a behaviour change after all..

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

On Tue, Jun 3, 2014 at 6:38 PM, Magnus Hagander <magnus(at)hagander(dot)net> wrote:
> On Mon, Jun 2, 2014 at 4:07 PM, Andres Freund <andres(at)2ndquadrant(dot)com>
> wrote:
>>
>> On 2014-06-02 22:59:55 +0900, Fujii Masao wrote:
>> > On Thu, May 29, 2014 at 3:02 PM, Peter Geoghegan <pg(at)heroku(dot)com> wrote:
>> > > On Wed, May 28, 2014 at 10:49 PM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com>
>> > > wrote:
>> > >> You're concerned about the scenario using pg_upgrade? I'm not sure
>> > >> the detail
>> > >> of pg_upgrade. But if it doesn't work properly, we should have gotten
>> > >> the trouble
>> > >
>> > > I'm not worried about pg_upgrade, because by design pg_stat_statements
>> > > will discard stats files that originated in earlier versions. However,
>> > > I don't see a need to change pg_stat_statements to serialize its
>> > > statistics to disk in the pg_stat directory before we branch off 9.4.
>> > > As you mentioned, it's harmless.
>> >
>> > Yeah, that's an idea. OTOH, there is no *strong* reason to postpone
>> > the fix to 9.5. So I just feel inclined to apply the fix now...
>>
>> +1 for fixing it now.
>>
>
> +1 for fixing it for 9.4 before the next beta, but *not* backpatching it to
> 9.3 - it *is* a behaviour change after all..

Yep, I just applied the patch only to HEAD.

Regards,

--
Fujii Masao

Fujii-san,

I agree not to backpatch, but I noticed that the 9.3 document about
stats collector doesn't mention $PGDATA/pg_stat.

http://www.postgresql.org/docs/current/static/monitoring-stats.html

It just says:
> When the server shuts down, a permanent copy of the statistics data is stored in the global subdirectory, so that statistics can be retained across server restarts.

I'm not sure whether we should modify the 9.3 document at the moment,
but actually the description made me confused a bit.

2014-05-29 12:22 GMT+09:00 Fujii Masao <masao(dot)fujii(at)gmail(dot)com>:
> On Thu, May 29, 2014 at 4:55 AM, Tomas Vondra <tv(at)fuzzy(dot)cz> wrote:
>> On 28.5.2014 19:52, Fujii Masao wrote:
>>> On Thu, May 29, 2014 at 12:37 AM, Peter Geoghegan <pg(at)heroku(dot)com> wrote:
>>>> On Wed, May 28, 2014 at 7:01 AM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
>>>>> But pg_stat_statements file is saved under $PGDATA/global yet.
>>>>> Is this intentional or just oversight?
>>>>
>>>>
>>>> I think it's an oversight.
>>>
>>> OK, patch attached.
>>>
>>> I'm afraid that it's not okay to change the file layout in $PGDATA at this beta1
>>> stage because that change basically seems to need initdb. Otherwise something
>>> like "no such file or directory" error can happen. But in this case what we need
>>> to change is only the location of the pg_stat_statements permanent stats file.
>>> So, without initdb, the server will not be able to find the
>>> pg_stat_statements stats
>>> file, but this is not so harmful. Only the problem is that the
>>> pg_stat_statements
>>> stats which were collected in past would disappear. OTOH, the server can keep
>>> running successfully from then and no critical data will not
>>> disappear. Therefore
>>> I think we can commit this patch even at beta1. Thought?
>>
>> For HEAD, probably yes. But what about backpatching 9.3?
>
> I think No. So we should not backpatch this to 9.3.
>
> Regards,
>
> --
> Fujii Masao
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers

--
Shigeru HANADA

On Tue, Jun 17, 2014 at 2:11 PM, Shigeru Hanada
<shigeru(dot)hanada(at)gmail(dot)com> wrote:
> Fujii-san,
>
> I agree not to backpatch, but I noticed that the 9.3 document about
> stats collector doesn't mention $PGDATA/pg_stat.
>
> http://www.postgresql.org/docs/current/static/monitoring-stats.html
>
> It just says:
>> When the server shuts down, a permanent copy of the statistics data is stored in the global subdirectory, so that statistics can be retained across server restarts.
>
> I'm not sure whether we should modify the 9.3 document at the moment,
> but actually the description made me confused a bit.

Could you check 8dc90b9c4c45fa30a8f59313e21d294529ef7182 in REL9_3_STABLE
branch? You can see that I added the description of pg_stat directory
into the document
in 9.3.

Regards,

--
Fujii Masao

Fujii-san,

I found the right description in REL9_3_STABLE branch, thanks for
pointing out the commit.

Sorry for noise.

2014-06-18 12:39 GMT+09:00 Fujii Masao <masao(dot)fujii(at)gmail(dot)com>:
> On Tue, Jun 17, 2014 at 2:11 PM, Shigeru Hanada
> <shigeru(dot)hanada(at)gmail(dot)com> wrote:
>> Fujii-san,
>>
>> I agree not to backpatch, but I noticed that the 9.3 document about
>> stats collector doesn't mention $PGDATA/pg_stat.
>>
>> http://www.postgresql.org/docs/current/static/monitoring-stats.html
>>
>> It just says:
>>> When the server shuts down, a permanent copy of the statistics data is stored in the global subdirectory, so that statistics can be retained across server restarts.
>>
>> I'm not sure whether we should modify the 9.3 document at the moment,
>> but actually the description made me confused a bit.
>
> Could you check 8dc90b9c4c45fa30a8f59313e21d294529ef7182 in REL9_3_STABLE
> branch? You can see that I added the description of pg_stat directory
> into the document
> in 9.3.
>
> Regards,
>
> --
> Fujii Masao

--
Shigeru HANADA