Re: [PATCH 3/8] Add support for a generic wal reading facility dubbed XLogReader

From: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
To: Heikki Linnakangas <hlinnakangas(at)vmware(dot)com>
Cc: Andres Freund <andres(at)2ndquadrant(dot)com>, pgsql-hackers(at)postgresql(dot)org, Robert Haas <robertmhaas(at)gmail(dot)com>, Kevin Grittner <Kevin(dot)Grittner(at)wicourts(dot)gov>, Simon Riggs <simon(at)2ndquadrant(dot)com>
Subject: Re: [PATCH 3/8] Add support for a generic wal reading facility dubbed XLogReader
Date: 2012-10-29 19:58:53
Message-ID: 20121029195853.GI12961@alvh.no-ip.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Heikki Linnakangas escribió:

> Hmm. I was thinking that making this work in a non-backend context
> would be too hard, so I didn't give that much thought, but I guess
> there isn't many dependencies to backend functions after all.
> palloc/pfree are straightforward to replace with malloc/free. That's
> what we could easily do with the error messages too, just malloc a
> suitably sized buffer.
>
> How does a non-backend program get access to xlogreader.c? Copy
> xlogreader.c from the source tree at build time and link into the
> program? Or should we turn it into a shared library?

It links the object file into the src/bin/ subdir or whatever. I don't
think a shared library is warranted at this point.

One further (relatively minor) problem with what you propose here is
that the xlogreader.c code is using emode_for_corrupt_record(), which
not only lives in xlog.c but also depends on readSource. I guess we
still want the message-supressing abilities of that function, so some
more thinking is required in this area.

I think you may have converted some malloc() calls from Andres' patch
into palloc() -- because you have some palloc() calls which are later
checked for NULL results, which obviously doesn't make sense. At the
same time, if we're going to use malloc() instead of palloc(), we need
to check for NULL return value in XLogReaderAllocate() callers. This
seems easy to fix at first glance, but what is the correct response if
it fails during StartupXLOG()? Should we just elog(FATAL) and hope it
never happens in practice?

Andres commented elsewhere about reading xlog records, processing them
as they came in, and do a running CRC while we're still reading it. I
think this is a mistake; we shouldn't do anything with a record until
the CRC has been verified. Otherwise we risk reading arbitrarily
corrupt data.

Overall, I think I like Heikki's minimal patch better than Andres'
original proposal, but I'll keep looking at both.

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Jim Nasby 2012-10-29 20:10:31 Re: Deparsing DDL command strings
Previous Message Claudio Freire 2012-10-29 19:46:46 Re: [PATCH] Prefetch index pages for B-Tree index scans