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)

Re: pgsql: Unite ReadBufferWithFork, ReadBufferWithStrategy, and

From
Simon Riggs
Date:
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


Re: pgsql: Unite ReadBufferWithFork, ReadBufferWithStrategy, and

From
Heikki Linnakangas
Date:
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

Re: pgsql: Unite ReadBufferWithFork, ReadBufferWithStrategy, and

From
Alvaro Herrera
Date:
>> 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.

Re: pgsql: Unite ReadBufferWithFork, ReadBufferWithStrategy, and

From
Heikki Linnakangas
Date:
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

Re: pgsql: Unite ReadBufferWithFork, ReadBufferWithStrategy, and

From
Heikki Linnakangas
Date:
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