Thread: pgsql: Unite ReadBufferWithFork, ReadBufferWithStrategy, and
pgsql: Unite ReadBufferWithFork, ReadBufferWithStrategy, and
From
heikki@postgresql.org (Heikki Linnakangas)
Date:
Log Message: ----------- Unite ReadBufferWithFork, ReadBufferWithStrategy, and ZeroOrReadBuffer functions into one ReadBufferExtended function, that takes the strategy and mode as argument. There's three modes, RBM_NORMAL which is the default used by plain ReadBuffer(), RBM_ZERO, which replaces ZeroOrReadBuffer, and a new mode RBM_ZERO_ON_ERROR, which allows callers to read corrupt pages without throwing an error. The FSM needs the new mode to recover from corrupt pages, which could happend if we crash after extending an FSM file, and the new page is "torn". Add fork number to some error messages in bufmgr.c, that still lacked it. Modified Files: -------------- pgsql/contrib/pageinspect: rawpage.c (r1.8 -> r1.9) (http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/contrib/pageinspect/rawpage.c?r1=1.8&r2=1.9) pgsql/src/backend/access/gin: ginvacuum.c (r1.23 -> r1.24) (http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/access/gin/ginvacuum.c?r1=1.23&r2=1.24) pgsql/src/backend/access/gist: gistvacuum.c (r1.38 -> r1.39) (http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/access/gist/gistvacuum.c?r1=1.38&r2=1.39) pgsql/src/backend/access/hash: hashpage.c (r1.77 -> r1.78) (http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/access/hash/hashpage.c?r1=1.77&r2=1.78) pgsql/src/backend/access/heap: heapam.c (r1.266 -> r1.267) (http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/access/heap/heapam.c?r1=1.266&r2=1.267) pgsql/src/backend/access/nbtree: nbtree.c (r1.163 -> r1.164) (http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/access/nbtree/nbtree.c?r1=1.163&r2=1.164) pgsql/src/backend/access/transam: xlog.c (r1.320 -> r1.321) (http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/access/transam/xlog.c?r1=1.320&r2=1.321) xlogutils.c (r1.59 -> r1.60) (http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/access/transam/xlogutils.c?r1=1.59&r2=1.60) pgsql/src/backend/commands: analyze.c (r1.125 -> r1.126) (http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/commands/analyze.c?r1=1.125&r2=1.126) vacuum.c (r1.378 -> r1.379) (http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/commands/vacuum.c?r1=1.378&r2=1.379) vacuumlazy.c (r1.108 -> r1.109) (http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/commands/vacuumlazy.c?r1=1.108&r2=1.109) pgsql/src/backend/storage/buffer: bufmgr.c (r1.239 -> r1.240) (http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/storage/buffer/bufmgr.c?r1=1.239&r2=1.240) pgsql/src/backend/storage/freespace: freespace.c (r1.64 -> r1.65) (http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/storage/freespace/freespace.c?r1=1.64&r2=1.65) pgsql/src/include/access: xlogutils.h (r1.26 -> r1.27) (http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/include/access/xlogutils.h?r1=1.26&r2=1.27) pgsql/src/include/storage: bufmgr.h (r1.115 -> r1.116) (http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/include/storage/bufmgr.h?r1=1.115&r2=1.116)
On Fri, 2008-10-31 at 15:05 +0000, Heikki Linnakangas wrote: > Log Message: > ----------- > Unite ReadBufferWithFork, ReadBufferWithStrategy, and ZeroOrReadBuffer > functions into one ReadBufferExtended function, that takes the strategy > and mode as argument. There's three modes, RBM_NORMAL which is the default > used by plain ReadBuffer(), RBM_ZERO, which replaces ZeroOrReadBuffer, and > a new mode RBM_ZERO_ON_ERROR, which allows callers to read corrupt pages > without throwing an error. The FSM needs the new mode to recover from > corrupt pages, which could happend if we crash after extending an FSM file, > and the new page is "torn". I thought you were adding the "read buffer only if in cache" option also? -- Simon Riggs www.2ndQuadrant.com PostgreSQL Training, Services and Support
Simon Riggs wrote: > On Fri, 2008-10-31 at 15:05 +0000, Heikki Linnakangas wrote: >> Log Message: >> ----------- >> Unite ReadBufferWithFork, ReadBufferWithStrategy, and ZeroOrReadBuffer >> functions into one ReadBufferExtended function, that takes the strategy >> and mode as argument. There's three modes, RBM_NORMAL which is the default >> used by plain ReadBuffer(), RBM_ZERO, which replaces ZeroOrReadBuffer, and >> a new mode RBM_ZERO_ON_ERROR, which allows callers to read corrupt pages >> without throwing an error. The FSM needs the new mode to recover from >> corrupt pages, which could happend if we crash after extending an FSM file, >> and the new page is "torn". > > I thought you were adding the "read buffer only if in cache" option > also? No, but if it's needed, it should now fit well into the infrastructure, as a new ReadBuffer mode. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com
>> On Fri, 2008-10-31 at 15:05 +0000, Heikki Linnakangas wrote: >>> Log Message: >>> ----------- >>> Unite ReadBufferWithFork, ReadBufferWithStrategy, and ZeroOrReadBuffer >>> functions into one ReadBufferExtended function, that takes the strategy >>> and mode as argument. There's three modes, RBM_NORMAL which is the default >>> used by plain ReadBuffer(), RBM_ZERO, which replaces ZeroOrReadBuffer, and >>> a new mode RBM_ZERO_ON_ERROR, which allows callers to read corrupt pages >>> without throwing an error. The FSM needs the new mode to recover from >>> corrupt pages, which could happend if we crash after extending an FSM file, >>> and the new page is "torn". Hmm. I see that some messages are now like this: (errmsg("unexpected data beyond EOF in block %u of relation %u/%u/%u/%u", blockNum, smgr->smgr_rnode.spcNode, smgr->smgr_rnode.dbNode, smgr->smgr_rnode.relNode, forkNum), but it seems that the file names contain symbolic fork names, not numbers. Is it possible to build the error messages so that they report the actual file name, or at least change the last /%u into a _%s with the fork name? -- Alvaro Herrera http://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc.
Alvaro Herrera wrote: > Hmm. I see that some messages are now like this: > > (errmsg("unexpected data beyond EOF in block %u of relation %u/%u/%u/%u", > blockNum, smgr->smgr_rnode.spcNode, smgr->smgr_rnode.dbNode, smgr->smgr_rnode.relNode, forkNum), > > but it seems that the file names contain symbolic fork names, not > numbers. Is it possible to build the error messages so that they report > the actual file name, or at least change the last /%u into a _%s with > the fork name? Agreed. There was some messages like that before, this patch just changed some error messages I had missed before. I'll change them all to match the file names ("%u/%u/%u_%s"). -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com
Heikki Linnakangas wrote: > Alvaro Herrera wrote: >> Hmm. I see that some messages are now like this: >> >> (errmsg("unexpected data beyond EOF in block %u of relation %u/%u/%u/%u", >> blockNum, smgr->smgr_rnode.spcNode, smgr->smgr_rnode.dbNode, >> smgr->smgr_rnode.relNode, forkNum), >> >> but it seems that the file names contain symbolic fork names, not >> numbers. Is it possible to build the error messages so that they report >> the actual file name, or at least change the last /%u into a _%s with >> the fork name? > > Agreed. There was some messages like that before, this patch just > changed some error messages I had missed before. I'll change them all to > match the file names ("%u/%u/%u_%s"). I guess I should just change those error messages to use relpath(). relpath() seems perfectly safe to call from those places; there's e.g. no catalog access. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com