pgstat_reset_remove_files ignores its argument
in 9.3 and 9.4, pgstat_reset_remove_files uses the global variable
pgstat_stat_directory rather than the argument it is passed, "directory".
On crash recovery, this means the tmp directory gets cleared twice and the
permanent pg_stat doesn't get cleared at all.
It seems like the obvious one line change would fix it, but I haven't
tested it because I don't know how to cause a crash without pg_stat already
being empty.
pgstat_reset_remove_files(const char *directory)
{
DIR *dir;
struct dirent *entry;
char fname[MAXPGPATH];
dir = AllocateDir(pgstat_stat_directory);
Cheers,
Jeff
On Wed, Aug 14, 2013 at 12:13 AM, Jeff Janes <jeff.janes@gmail.com> wrote:
in 9.3 and 9.4, pgstat_reset_remove_files uses the global variable
pgstat_stat_directory rather than the argument it is passed, "directory".
On crash recovery, this means the tmp directory gets cleared twice and the
permanent pg_stat doesn't get cleared at all.It seems like the obvious one line change would fix it, but I haven't tested
it because I don't know how to cause a crash without pg_stat already being
empty.
I think there are three lines to change, as in the attached patch.
Am I wrong?
...Robert
Attachments:
pgstat-reset-remove-files.patchapplication/octet-stream; name=pgstat-reset-remove-files.patchDownload
diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c
index dac5bca..36f751c 100644
--- a/src/backend/postmaster/pgstat.c
+++ b/src/backend/postmaster/pgstat.c
@@ -562,16 +562,15 @@ pgstat_reset_remove_files(const char *directory)
struct dirent *entry;
char fname[MAXPGPATH];
- dir = AllocateDir(pgstat_stat_directory);
- while ((entry = ReadDir(dir, pgstat_stat_directory)) != NULL)
+ dir = AllocateDir(directory);
+ while ((entry = ReadDir(dir, directory)) != NULL)
{
if (strcmp(entry->d_name, ".") == 0 || strcmp(entry->d_name, "..") == 0)
continue;
/* XXX should we try to ignore files other than the ones we write? */
- snprintf(fname, MAXPGPATH, "%s/%s", pgstat_stat_directory,
- entry->d_name);
+ snprintf(fname, MAXPGPATH, "%s/%s", directory, entry->d_name);
unlink(fname);
}
FreeDir(dir);
On 16.8.2013 21:38, Robert Haas wrote:
On Wed, Aug 14, 2013 at 12:13 AM, Jeff Janes <jeff.janes@gmail.com>
wrote:in 9.3 and 9.4, pgstat_reset_remove_files uses the global variable
pgstat_stat_directory rather than the argument it is passed,
"directory". On crash recovery, this means the tmp directory gets
cleared twice and the permanent pg_stat doesn't get cleared at
all.It seems like the obvious one line change would fix it, but I
haven't tested it because I don't know how to cause a crash without
pg_stat already being empty.I think there are three lines to change, as in the attached patch.
Am I wrong?
...Robert
I think the patch is OK.
Tomas
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Fri, Aug 16, 2013 at 12:38 PM, Robert Haas <robertmhaas@gmail.com> wrote:
On Wed, Aug 14, 2013 at 12:13 AM, Jeff Janes <jeff.janes@gmail.com> wrote:
in 9.3 and 9.4, pgstat_reset_remove_files uses the global variable
pgstat_stat_directory rather than the argument it is passed, "directory".
On crash recovery, this means the tmp directory gets cleared twice and the
permanent pg_stat doesn't get cleared at all.It seems like the obvious one line change would fix it, but I haven't tested
it because I don't know how to cause a crash without pg_stat already being
empty.I think there are three lines to change, as in the attached patch.
Am I wrong?
No, you are right, I too realized I missed a couple more spots. Your
patch looks just like the one I eventually arrived at, before I got
distracted thinking about how to implement the regex
/^(global|db_\d+)\.stat$/ in C and forgot to post a correction.
Is the regex code in src/backend/regex allowed to be used from "flat"
C code, or does it have to be in the context of a transaction, memory
context, etc.?
Cheers,
Jeff
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Jeff Janes escribi�:
No, you are right, I too realized I missed a couple more spots. Your
patch looks just like the one I eventually arrived at, before I got
distracted thinking about how to implement the regex
/^(global|db_\d+)\.stat$/ in C and forgot to post a correction.Is the regex code in src/backend/regex allowed to be used from "flat"
C code, or does it have to be in the context of a transaction, memory
context, etc.?
See 20130819180437.GF9264@eldon.alvh.no-ip.org .. the only thing missing
there is the $, AFAICT.
--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers