Thread: 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
Attachment
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
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
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