Re: Block-level CRC checks

A customer of ours has been having trouble with corrupted data for some
time. Of course, we've almost always blamed hardware (and we've seen
RAID controllers have their firmware upgraded, among other actions), but
the useful thing to know is when corruption has happened, and where.

So we've been tasked with adding CRCs to data files.

The idea is that these CRCs are going to be checked just after reading
files from disk, and calculated just before writing it. They are
just a protection against the storage layer going mad; they are not
intended to protect against faulty RAM, CPU or kernel.

This code would be run-time or compile-time configurable. I'm not
absolutely sure which yet; the problem with run-time is what to do if
the user restarts the server with the setting flipped. It would have
almost no impact on users who don't enable it.

The implementation I'm envisioning requires the use of a new relation
fork to store the per-block CRCs. Initially I'm aiming at a CRC32 sum
for each block. FlushBuffer would calculate the checksum and store it
in the CRC fork; ReadBuffer_common would read the page, calculate the
checksum, and compare it to the one stored in the CRC fork.

A buffer's io_in_progress lock protects the buffer's CRC. We read and
pin the CRC page before acquiring the lock, to avoid having two buffer
IO operations in flight.

I'd like to submit this for 8.4, but I want to ensure that -hackers at
large approve of this feature before starting serious coding.

Opinions?

--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

On Tue, Sep 30, 2008 at 2:02 PM, Alvaro Herrera
<alvherre(at)commandprompt(dot)com> wrote:
> A customer of ours has been having trouble with corrupted data for some
> time. Of course, we've almost always blamed hardware (and we've seen
> RAID controllers have their firmware upgraded, among other actions), but
> the useful thing to know is when corruption has happened, and where.

Agreed.

> So we've been tasked with adding CRCs to data files.

Awesome.

> The idea is that these CRCs are going to be checked just after reading
> files from disk, and calculated just before writing it. They are
> just a protection against the storage layer going mad; they are not
> intended to protect against faulty RAM, CPU or kernel.

This is the common case.

> This code would be run-time or compile-time configurable. I'm not
> absolutely sure which yet; the problem with run-time is what to do if
> the user restarts the server with the setting flipped. It would have
> almost no impact on users who don't enable it.

I've supported this forever!

> The implementation I'm envisioning requires the use of a new relation
> fork to store the per-block CRCs. Initially I'm aiming at a CRC32 sum
> for each block. FlushBuffer would calculate the checksum and store it
> in the CRC fork; ReadBuffer_common would read the page, calculate the
> checksum, and compare it to the one stored in the CRC fork.
>
> A buffer's io_in_progress lock protects the buffer's CRC. We read and
> pin the CRC page before acquiring the lock, to avoid having two buffer
> IO operations in flight.

If the CRC gets written before the block, how is recovery going to
handle it? I'm not too familiar with the new forks stuff, but
recovery will pull the old block, compare it against the checksum, and
consider the block invalid, correct?

> I'd like to submit this for 8.4, but I want to ensure that -hackers at
> large approve of this feature before starting serious coding.

IMHO, this is a functionality that should be enabled by default (as it
is on most other RDBMS). It would've prevented severe corruption in
the 20 or so databases I've had to fix, and other than making it
optional, I don't see the reasoning for a separate relation fork
rather than storing it directly on the block (as everyone else does).
Similarly, I think Greg Stark was playing with a patch for it
(http://archives.postgresql.org/pgsql-hackers/2007-02/msg01850.php).

--
Jonah H. Harris, Senior DBA
myYearbook.com

Alvaro Herrera <alvherre(at)commandprompt(dot)com> writes:
> The implementation I'm envisioning requires the use of a new relation
> fork to store the per-block CRCs.

That seems bizarre, and expensive, and if you lose one block of the CRC
fork you lose confidence in a LOT of data. Why not keep the CRCs in the
page headers?

> A buffer's io_in_progress lock protects the buffer's CRC.

Unfortunately, it doesn't. See hint bits.

regards, tom lane

Alvaro Herrera wrote:
> A customer of ours has been having trouble with corrupted data for some
> time. Of course, we've almost always blamed hardware (and we've seen
> RAID controllers have their firmware upgraded, among other actions), but
> the useful thing to know is when corruption has happened, and where.
>
> So we've been tasked with adding CRCs to data files.
>
> The idea is that these CRCs are going to be checked just after reading
> files from disk, and calculated just before writing it. They are
> just a protection against the storage layer going mad; they are not
> intended to protect against faulty RAM, CPU or kernel.

This has been suggested before, and the usual objection is precisely
that it only protects from errors in the storage layer, giving a false
sense of security.

Doesn't some filesystems include a per-block CRC, which would achieve
the same thing? ZFS?

> This code would be run-time or compile-time configurable. I'm not
> absolutely sure which yet; the problem with run-time is what to do if
> the user restarts the server with the setting flipped. It would have
> almost no impact on users who don't enable it.

Yeah, seems like it would need to be compile-time or initdb-time
configurable.

> The implementation I'm envisioning requires the use of a new relation
> fork to store the per-block CRCs. Initially I'm aiming at a CRC32 sum
> for each block. FlushBuffer would calculate the checksum and store it
> in the CRC fork; ReadBuffer_common would read the page, calculate the
> checksum, and compare it to the one stored in the CRC fork.

Surely it would be much simpler to just add a field to the page header.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

On Tue, 30 Sep 2008 14:33:04 -0400
"Jonah H. Harris" <jonah(dot)harris(at)gmail(dot)com> wrote:

> > I'd like to submit this for 8.4, but I want to ensure that -hackers
> > at large approve of this feature before starting serious coding.
>
> IMHO, this is a functionality that should be enabled by default (as it
> is on most other RDBMS). It would've prevented severe corruption in

What other RDMS have it enabled by default?

Sincerely,

Joshua D. Drake

--
The PostgreSQL Company since 1997: http://www.commandprompt.com/
PostgreSQL Community Conference: http://www.postgresqlconference.org/
United States PostgreSQL Association: http://www.postgresql.us/

On Tue, Sep 30, 2008 at 2:49 PM, Joshua Drake <jd(at)commandprompt(dot)com> wrote:
> On Tue, 30 Sep 2008 14:33:04 -0400
> "Jonah H. Harris" <jonah(dot)harris(at)gmail(dot)com> wrote:
>
>> > I'd like to submit this for 8.4, but I want to ensure that -hackers
>> > at large approve of this feature before starting serious coding.
>>
>> IMHO, this is a functionality that should be enabled by default (as it
>> is on most other RDBMS). It would've prevented severe corruption in
>
> What other RDMS have it enabled by default?

Oracle and (I belive) SQL Server >= 2005

--
Jonah H. Harris, Senior DBA
myYearbook.com

Hello Alvaro,

some random thoughts while reading your proposal follow...

Alvaro Herrera wrote:
> So we've been tasked with adding CRCs to data files.

Disks get larger and relative reliability shrinks, it seems. So I agree
that this is a worthwhile thing to have. But shouldn't that be the job
of the filesystem? Think of ZFS or the upcoming BTRFS.

> The idea is that these CRCs are going to be checked just after reading
> files from disk, and calculated just before writing it. They are
> just a protection against the storage layer going mad; they are not
> intended to protect against faulty RAM, CPU or kernel.

That sounds reasonable if we do it from Postgres.

> This code would be run-time or compile-time configurable. I'm not
> absolutely sure which yet; the problem with run-time is what to do if
> the user restarts the server with the setting flipped. It would have
> almost no impact on users who don't enable it.

I'd say calculating a CRC is close enough to be considered "no impact".
A single core of a modern CPU easily reaches way above 200 MiB/s
throughput for CRC32 today. See [1].

Maybe consider Adler-32 which is 3-4x faster [2], also part of zlib and
AFAIK about equally safe for 8k blocks and above.

> The implementation I'm envisioning requires the use of a new relation
> fork to store the per-block CRCs. Initially I'm aiming at a CRC32 sum
> for each block. FlushBuffer would calculate the checksum and store it
> in the CRC fork; ReadBuffer_common would read the page, calculate the
> checksum, and compare it to the one stored in the CRC fork.

Huh? Aren't CRCs normally stored as part of the block they are supposed
to protect? Or how do you expect to ensure the data from the CRC
relation fork is correct? How about crash safety (a data block written,
but not its CRC block or vice versa)?

Wouldn't that double the amount of seeking required for writes?

> I'd like to submit this for 8.4, but I want to ensure that -hackers at
> large approve of this feature before starting serious coding.

Very cool!

Regards

Markus Wanner

[1]: Crypto++ benchmarks:
http://www.cryptopp.com/benchmarks.html

[2]: Wikipedia about hash functions:
http://en.wikipedia.org/wiki/List_of_hash_functions#Computational_costs_of_CRCs_vs_Hashes

Alvaro Herrera wrote:
> Initially I'm aiming at a CRC32 sum
> for each block. FlushBuffer would calculate the checksum and store it
> in the CRC fork; ReadBuffer_common would read the page, calculate the
> checksum, and compare it to the one stored in the CRC fork.

There's one fundamental problem with that, related to the way our hint
bits are written.

Currently, hint bit updates are not WAL-logged, and thus no full page
write is done when only hint bits are changed. Imagine what happens if
hint bits are updated on a page, but there's no other changes, and we
crash so that only one half of the new page version makes it to disk (=
torn page). The CRC would not match, even though the page is actually valid.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

> A customer of ours has been having trouble with corrupted data for some
> time. Of course, we've almost always blamed hardware (and we've seen
> RAID controllers have their firmware upgraded, among other actions), but
> the useful thing to know is when corruption has happened, and where.

That is an important statement, to know when it happens not necessarily to
be able to recover the block or where in the block it is corrupt. Is that
correct?

>
> So we've been tasked with adding CRCs to data files.

CRC or checksum? If the objective is merely general "detection" there
should be some latitude in choosing the methodology for performance.

>
> The idea is that these CRCs are going to be checked just after reading
> files from disk, and calculated just before writing it. They are
> just a protection against the storage layer going mad; they are not
> intended to protect against faulty RAM, CPU or kernel.

It will actually find faults in all if it. If the CPU can't add and/or a
RAM location lost a bit, this will blow up just as easily as a bad block.
It may cause "false identification" of an error, but it will keep a bad
system from hiding.

>
> This code would be run-time or compile-time configurable. I'm not
> absolutely sure which yet; the problem with run-time is what to do if
> the user restarts the server with the setting flipped. It would have
> almost no impact on users who don't enable it.

CPU capacity on modern hardware within a small area of RAM is practically
infinite when compared to any sort of I/O.
>
> The implementation I'm envisioning requires the use of a new relation
> fork to store the per-block CRCs. Initially I'm aiming at a CRC32 sum
> for each block. FlushBuffer would calculate the checksum and store it
> in the CRC fork; ReadBuffer_common would read the page, calculate the
> checksum, and compare it to the one stored in the CRC fork.

Hell, all that is needed is a long or a short checksum value in the block.
I mean, if you just want a sanity test, it doesn't take much. Using a
second relation creates confusion. If there is a CRC discrepancy between
two different blocks, who's wrong? You need a third "control" to know. If
the block knows its CRC or checksum and that is in error, the block is
bad.

>
> A buffer's io_in_progress lock protects the buffer's CRC. We read and
> pin the CRC page before acquiring the lock, to avoid having two buffer
> IO operations in flight.
>
> I'd like to submit this for 8.4, but I want to ensure that -hackers at
> large approve of this feature before starting serious coding.
>
> Opinions?

If its fast enough, its a good idea. It could be very helpful in
protecting users data.

>
> --
> Alvaro Herrera
> http://www.CommandPrompt.com/
> PostgreSQL Replication, Consulting, Custom Development, 24x7 support
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>

On Tue, 30 Sep 2008, Heikki Linnakangas wrote:

> Doesn't some filesystems include a per-block CRC, which would achieve the
> same thing? ZFS?

Yes, there is a popular advoacy piece for ZFS with a high-level view of
why and how they implement that at
http://blogs.sun.com/bonwick/entry/zfs_end_to_end_data The guarantees are
stronger than what you can get if you just put a CRC in the block itself.
I'd never really thought too hard about putting this in the database
knowing that ZFS is available for environments where this is a concern,
but it certainly would be a nice addition.

The best analysis I've ever seen that makes a case for OS or higher level
disk checksums of some sort, by looking at the myriad ways that disks and
disk arrays fail in the real world, is in
http://www.usenix.org/event/fast08/tech/full_papers/bairavasundaram/bairavasundaram.pdf
(there is a shorter version that hits the high points of that at
http://www.usenix.org/publications/login/2008-06/openpdfs/bairavasundaram.pdf
)

One really interesting bit in there I'd never seen before is that they
find real data that supports the stand that enterprise drives are
significantly more reliable than consumer ones. While general failure
rates aren't that different, "SATA disks have an order of magnitude higher
probability of developing checksum mismatches than Fibre Channel disks. We
find that 0.66% of SATA disks develop at least one mismatch during the
first 17 months in the field, whereas only 0.06% of Fibre Channel disks
develop a mismatch during that time."

--
* Greg Smith gsmith(at)gregsmith(dot)com http://www.gregsmith.com Baltimore, MD

Alvaro Herrera wrote:
> A customer of ours has been having trouble with corrupted data for some
> time. Of course, we've almost always blamed hardware (and we've seen
> RAID controllers have their firmware upgraded, among other actions), but
> the useful thing to know is when corruption has happened, and where.
>
> So we've been tasked with adding CRCs to data files.

Maybe a stupid question, but what I/O subsystems corrupt data and fail
to report it?

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

On Tue, Sep 30, 2008 at 1:41 PM, Bruce Momjian <bruce(at)momjian(dot)us> wrote:

> Alvaro Herrera wrote:
> > A customer of ours has been having trouble with corrupted data for some
> > time. Of course, we've almost always blamed hardware (and we've seen
> > RAID controllers have their firmware upgraded, among other actions), but
> > the useful thing to know is when corruption has happened, and where.
> >
> > So we've been tasked with adding CRCs to data files.
>
> Maybe a stupid question, but what I/O subsystems corrupt data and fail
> to report it?

Practically all of them. Here is a good paper on various checksums, their
failure rates, and practical applications.

"Parity Lost and Parity Regained"
http://www.usenix.org/event/fast08/tech/full_papers/krioukov/krioukov_html/index.html

-jwb

On Sep 30, 2008, at 2:17 PM, pgsql(at)mohawksoft(dot)com wrote:
>> A customer of ours has been having trouble with corrupted data for
>> some
>> time. Of course, we've almost always blamed hardware (and we've seen
>> RAID controllers have their firmware upgraded, among other
>> actions), but
>> the useful thing to know is when corruption has happened, and where.
>
> That is an important statement, to know when it happens not
> necessarily to
> be able to recover the block or where in the block it is corrupt.
> Is that
> correct?

Oh, correcting the corruption would be AWESOME beyond belief! But at
this point I'd settle for just knowing it had happened.

>> So we've been tasked with adding CRCs to data files.
>
> CRC or checksum? If the objective is merely general "detection" there
> should be some latitude in choosing the methodology for performance.

See above. Perhaps the best win would be a case where you could
choose which method you wanted. We generally have extra CPU on the
servers, so we could afford to burn some cycles with more complex
algorithms.

>> The idea is that these CRCs are going to be checked just after
>> reading
>> files from disk, and calculated just before writing it. They are
>> just a protection against the storage layer going mad; they are not
>> intended to protect against faulty RAM, CPU or kernel.
>
> It will actually find faults in all if it. If the CPU can't add and/
> or a
> RAM location lost a bit, this will blow up just as easily as a bad
> block.
> It may cause "false identification" of an error, but it will keep a
> bad
> system from hiding.

Well, very likely not, since the intention is to only compute the CRC
when we write the block out, at least for now. In the future I would
like to be able to detect when a CPU or memory goes bonkers and poops
on something, because that's actually happened to us as well.

>> The implementation I'm envisioning requires the use of a new relation
>> fork to store the per-block CRCs. Initially I'm aiming at a CRC32
>> sum
>> for each block. FlushBuffer would calculate the checksum and
>> store it
>> in the CRC fork; ReadBuffer_common would read the page, calculate the
>> checksum, and compare it to the one stored in the CRC fork.
>
> Hell, all that is needed is a long or a short checksum value in the
> block.
> I mean, if you just want a sanity test, it doesn't take much. Using a
> second relation creates confusion. If there is a CRC discrepancy
> between
> two different blocks, who's wrong? You need a third "control" to
> know. If
> the block knows its CRC or checksum and that is in error, the block is
> bad.

I believe the idea was to make this as non-invasive as possible. And
it would be really nice if this could be enabled without a dump/
reload (maybe the upgrade stuff would make this possible?)
--
Decibel!, aka Jim C. Nasby, Database Architect decibel(at)decibel(dot)org
Give your computer some brain candy! www.distributed.net Team #1828

On Tue, 30 Sep 2008 13:48:52 -0700
"Jeffrey Baker" <jwbaker(at)gmail(dot)com> wrote:

>
> Practically all of them. Here is a good paper on various checksums,
> their failure rates, and practical applications.
>
> "Parity Lost and Parity Regained"
> http://www.usenix.org/event/fast08/tech/full_papers/krioukov/krioukov_html/index.html
>

In a related article published in Login called "Data Corruption in the
storage stack: a closer look" they say:

During a 41-month period we observed more than 400,000 instances of
checksum mistmatches, 8% of which were discovered during RAID
reconstruction, creating the possibility of real data loss.

They also have a wonderful term they mention, "Silent Data corruptions".

Joshua D. Drake

[1] Login June 2008

> -jwb

--
The PostgreSQL Company since 1997: http://www.commandprompt.com/
PostgreSQL Community Conference: http://www.postgresqlconference.org/
United States PostgreSQL Association: http://www.postgresql.us/

>
> I believe the idea was to make this as non-invasive as possible. And
> it would be really nice if this could be enabled without a dump/
> reload (maybe the upgrade stuff would make this possible?)
> --

It's all about the probability of a duplicate check being generated. If
you use a 32 bit checksum, then you have a theoretical probability of 1 in
4 billion that a corrupt block will be missed (probably much lower
depending on your algorithm). If you use a short, then a 1 in 65 thousand
probability. If you use an 8 bit number, then 1 in 256.

Why am I going on? Well, if there are any spare bits in a block header,
they could be used for the check value.

On Sep 30, 2008, at 1:48 PM, Heikki Linnakangas wrote:
> This has been suggested before, and the usual objection is
> precisely that it only protects from errors in the storage layer,
> giving a false sense of security.

If you can come up with a mechanism for detecting non-storage errors
as well, I'm all ears. :)

In the meantime, you're way, way more likely to experience corruption
at the storage layer than anywhere else. We've had several corruption
events, only one of which was memory related... and we *know* it was
memory related because we actually got logs saying so. But with a SAN
environment there's a lot of moving parts, all waiting to screw up
your data:

filesystem
SAN device driver
SAN network
SAN BIOS
drive BIOS
drive

That's above things that could hose your data outside of storage:
kernel
CPU
memory
motherboard

> Doesn't some filesystems include a per-block CRC, which would
> achieve the same thing? ZFS?

Sure, some do. We're on linux and can't run ZFS. And I'll argue that
no linux FS is anywhere near as tested as ext3 is, which means that
going to some other FS that offers you CRC means you're now exposing
yourself to the possibility of issues with the FS itself. Not to
mention that changing filesystems on a large production system is
very painful.
--
Decibel!, aka Jim C. Nasby, Database Architect decibel(at)decibel(dot)org
Give your computer some brain candy! www.distributed.net Team #1828

On 30 Sep 2008, at 10:17 PM, Decibel! <decibel(at)decibel(dot)org> wrote:

> On Sep 30, 2008, at 1:48 PM, Heikki Linnakangas wrote:
>> This has been suggested before, and the usual objection is
>> precisely that it only protects from errors in the storage layer,
>> giving a false sense of security.
>
> If you can come up with a mechanism for detecting non-storage errors
> as well, I'm all ears. :)
>
> In the meantime, you're way, way more likely to experience
> corruption at the storage layer than anywhere else.

Fwiw this hasn't been my experience. Bad memory is extremely common
and even the storage failures I've seen (excluding the drive crashes)
turned out to actually be caused by bad memory.

That said I've always been interested in doing this. The main use case
in my mind has actually been for data that's been restored from old
backups which have been lying round and floating between machines for
a while with many opportunities for bit errors to show up.

The main stumbling block I ran into was how to deal with turning the
option off and on. I wanted it to be possible to turn off the option
to have the database ignore any errors and to avoid the overhead.

But that means including an escape hatch value which is always
considered to be correct. But that dramatically reduces the
effectiveness of the scheme.

Another issue is it will make space available on each page smaller
making it harder to do in place upgrades.

If you can deal with those issues and carefully deal with the
contingencies so it's clear to people what to do when errra occur or
they want to turn the feature on or off then I'm all for it. That
despite my experience of memory errors being a lot more common than
undetected storage errors.

Joshua Drake wrote:
> During a 41-month period we observed more than 400,000 instances of
> checksum mistmatches, 8% of which were discovered during RAID
> reconstruction, creating the possibility of real data loss.
>
> They also have a wonderful term they mention, "Silent Data corruptions".
>
>
>

Exactely!

From my experience, the only assumption to be made about storage is that it can
and will fail ... frequently! It is unreliable (not to mention slooow) and
should not be trusted; regardless of the price tag or brand.

This could help detect:

- fs corruption
- vfs bug
- raid controller firmware bug
- bad disk sector
- power crash
- weird martian-like raid rebuilds

Although, this idea won't prevent anything. Everything would still sinisterly
fail on you. The difference is, no more silence.

--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/

Jonah H. Harris wrote:
>>>> I'd like to submit this for 8.4, but I want to ensure that -hackers
>>>> at large approve of this feature before starting serious coding.
>>>
>>> IMHO, this is a functionality that should be enabled by default (as it
>>> is on most other RDBMS). It would've prevented severe corruption in
>>
>> What other RDMS have it enabled by default?
>
> Oracle and (I belive) SQL Server >= 2005

References:
http://download-uk.oracle.com/docs/cd/B19306_01/server.102/b14237/initparams040.htm#CHDDCEIC
http://download.oracle.com/docs/cd/B28359_01/server.111/b28320/initparams046.htm#sthref130

Oracle claims that it introduces only 1% to 2% overhead.

Actually I suggested the same feature for 8.3:
http://archives.postgresql.org/pgsql-hackers/2007-08/msg01003.php

It was eventually rejected because the majority felt that it
would be a small benefit (only detects disk corruption and not
software bugs) that would not justify the overhead and the
additional code.

Incidently, Oracle also has a parameter DB_BLOCK_CHECKING
that checks blocks for logical consistency. This is OFF by default,
but Oracle recommends that you activate it if you can live with
the performance impact.

Yours,
Laurenz Albe

Alvaro Herrera napsal(a):

> This code would be run-time or compile-time configurable. I'm not
> absolutely sure which yet; the problem with run-time is what to do if
> the user restarts the server with the setting flipped. It would have
> almost no impact on users who don't enable it.

I prefer runtime configuration. Solaris has two filesystems UFS, ZFS.
ZFS offers strong error detection and a another CRC is overhead. You
need mechanism how to enable this feature on UFS and disable on ZFS. I
suggest to have it as tablespace property and for default tablespace you
can setup it in initdb phase.

Zdenek

CRC-checks will help to detect corrupt data.

my question:

WHAT should happen when corrupted data is detected?

a) PostgreSQL can end with some paniccode
b) a log can be written, with some rather high level

a) has the benefit that it surely will be noticed. Which is a real
benefet, as I suppose that many users of PostgreSQL on the low end do
not have dedicated DBA-Admins who read the *.log every day

b) has the benefit that work goes on

BUT:
What can somebody do AFTER PostgreSQL has detected "data is corrupted,
CRC error in block xxxx" ?

My first thought of action would be: get new drive, pg_dump_all to
save place, install new drive, pg_restore
BUT: how will pg_dump_all be enabled? As PostgreSQL must be forced to
accept the corrupted data to do a pg_dump...

Next step: for huge databases it is tempting to not pg_dump, but
filecopy; with shut down database or pg_backup() etc. What way can
that be supported while data is corrupted?

Please do not misunderstand my question ... I really look forward to
get this kind of information; especially since corrupted data on hard
drives is an early sign of BIG trouble to come. I am just asking to
start thinking about "what do after corruption has been detected"

Harald

--
GHUM Harald Massa
persuadere et programmare
Harald Armin Massa
Spielberger Straße 49
70435 Stuttgart
0173/9409607
no fx, no carrier pigeon
-
EuroPython 2009 will take place in Birmingham - Stay tuned!

On Tue, 2008-09-30 at 17:13 -0400, pgsql(at)mohawksoft(dot)com wrote:
> >
> > I believe the idea was to make this as non-invasive as possible. And
> > it would be really nice if this could be enabled without a dump/
> > reload (maybe the upgrade stuff would make this possible?)
> > --
>
> It's all about the probability of a duplicate check being generated. If
> you use a 32 bit checksum, then you have a theoretical probability of 1 in
> 4 billion that a corrupt block will be missed (probably much lower
> depending on your algorithm). If you use a short, then a 1 in 65 thousand
> probability. If you use an 8 bit number, then 1 in 256.
>
> Why am I going on? Well, if there are any spare bits in a block header,
> they could be used for the check value.

Even and 64-bit integer is just 0.1% of 8k page size, and it is even
less than 0.1% likely that page will be 100% full and thus that 64bits
wastes any real space at all.

So I don't think that this is a space issue.

---------------
Hannu

Hannu Krosing <hannu(at)2ndQuadrant(dot)com> writes:
> So I don't think that this is a space issue.

No, it's all about time penalties and loss of concurrency.

regards, tom lane

"Harald Armin Massa" <haraldarminmassa(at)gmail(dot)com> writes:
> WHAT should happen when corrupted data is detected?

Same thing that happens now, ie, query fails with an error. This would
just be an extension of the existing validity checks done at page read
time.

regards, tom lane

On Wed, Oct 1, 2008 at 9:25 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> "Harald Armin Massa" <haraldarminmassa(at)gmail(dot)com> writes:
>> WHAT should happen when corrupted data is detected?
>
> Same thing that happens now, ie, query fails with an error. This would
> just be an extension of the existing validity checks done at page read
> time.

Agreed.

--
Jonah H. Harris, Senior DBA
myYearbook.com

> On Tue, 2008-09-30 at 17:13 -0400, pgsql(at)mohawksoft(dot)com wrote:
>> >
>> > I believe the idea was to make this as non-invasive as possible. And
>> > it would be really nice if this could be enabled without a dump/
>> > reload (maybe the upgrade stuff would make this possible?)
>> > --
>>
>> It's all about the probability of a duplicate check being generated. If
>> you use a 32 bit checksum, then you have a theoretical probability of 1
>> in
>> 4 billion that a corrupt block will be missed (probably much lower
>> depending on your algorithm). If you use a short, then a 1 in 65
>> thousand
>> probability. If you use an 8 bit number, then 1 in 256.
>>
>> Why am I going on? Well, if there are any spare bits in a block header,
>> they could be used for the check value.
>
> Even and 64-bit integer is just 0.1% of 8k page size, and it is even
> less than 0.1% likely that page will be 100% full and thus that 64bits
> wastes any real space at all.
>
> So I don't think that this is a space issue.
>

Oh, I don't think it is a space issue either, the question was could there
be a way to do it without a dump and restore. The only way that occurs to
me is if there are some unused bits in the block header. I haven't looked
at that code in years. The numerics of it are just a description of the
probability of a duplicate sum or crc, meaning a false OK.

Also, regardless of whether or not the block is full, the block is read
and written as a block and that the underlying data unimportant.

> Hannu Krosing <hannu(at)2ndQuadrant(dot)com> writes:
>> So I don't think that this is a space issue.
>
> No, it's all about time penalties and loss of concurrency.

I don't think that the amount of time it would take to calculate and test
the sum is even important. It may be in older CPUs, but these days CPUs
are so fast in RAM and a block is very small. On x86 systems, depending on
page alignment, we are talking about two or three pages that will be "in
memory" (They were used to read the block from disk or previously
accessed).

pgsql(at)mohawksoft(dot)com writes:
>> No, it's all about time penalties and loss of concurrency.

> I don't think that the amount of time it would take to calculate and test
> the sum is even important. It may be in older CPUs, but these days CPUs
> are so fast in RAM and a block is very small. On x86 systems, depending on
> page alignment, we are talking about two or three pages that will be "in
> memory" (They were used to read the block from disk or previously
> accessed).

Your optimism is showing ;-). XLogInsert routinely shows up as a major
CPU hog in any update-intensive test, and AFAICT that's mostly from the
CRC calculation for WAL records.

We could possibly use something cheaper than a real CRC, though. A
word-wide XOR (ie, effectively a parity calculation) would be sufficient
to detect most problems.

regards, tom lane

On Wed, Oct 1, 2008 at 10:27 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> I don't think that the amount of time it would take to calculate and test
>> the sum is even important. It may be in older CPUs, but these days CPUs
>> are so fast in RAM and a block is very small. On x86 systems, depending on
>> page alignment, we are talking about two or three pages that will be "in
>> memory" (They were used to read the block from disk or previously
>> accessed).
>
> Your optimism is showing ;-). XLogInsert routinely shows up as a major
> CPU hog in any update-intensive test, and AFAICT that's mostly from the
> CRC calculation for WAL records.

I probably wouldn't compare checksumming *every* WAL record to a
single block-level checksum.

--
Jonah H. Harris, Senior DBA
myYearbook.com

Tom Lane wrote:
> Alvaro Herrera <alvherre(at)commandprompt(dot)com> writes:

> > A buffer's io_in_progress lock protects the buffer's CRC.
>
> Unfortunately, it doesn't. See hint bits.

Hmm, so it seems we need to keep held of the bufferhead's spinlock while
calculating the checksum, just after resetting BM_JUST_DIRTIED. Yuck.

--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

Alvaro Herrera <alvherre(at)commandprompt(dot)com> writes:
> Tom Lane wrote:
>> Unfortunately, it doesn't. See hint bits.

> Hmm, so it seems we need to keep held of the bufferhead's spinlock while
> calculating the checksum, just after resetting BM_JUST_DIRTIED. Yuck.

No, holding a spinlock that long is entirely unacceptable, and it's the
wrong thing anyway, because we don't hold the header lock while
manipulating hint bits.

What this would *actually* mean is that we'd need to hold exclusive not
shared buffer lock on a buffer we are about to write, and that would
have to be maintained while computing the checksum and until the write
is completed. The JUST_DIRTIED business could go away, in fact.

(Thinks for a bit...) I wonder if that could induce any deadlock
problems? The concurrency hit might be the least of our worries.

regards, tom lane

"Jonah H. Harris" <jonah(dot)harris(at)gmail(dot)com> writes:
> On Wed, Oct 1, 2008 at 10:27 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> Your optimism is showing ;-). XLogInsert routinely shows up as a major
>> CPU hog in any update-intensive test, and AFAICT that's mostly from the
>> CRC calculation for WAL records.

> I probably wouldn't compare checksumming *every* WAL record to a
> single block-level checksum.

No, not at all. Block-level checksums would be an order of magnitude
more expensive: they're on bigger chunks of data and they'd be done more
often.

regards, tom lane

> pgsql(at)mohawksoft(dot)com writes:
>>> No, it's all about time penalties and loss of concurrency.
>
>> I don't think that the amount of time it would take to calculate and
>> test
>> the sum is even important. It may be in older CPUs, but these days CPUs
>> are so fast in RAM and a block is very small. On x86 systems, depending
>> on
>> page alignment, we are talking about two or three pages that will be "in
>> memory" (They were used to read the block from disk or previously
>> accessed).
>
> Your optimism is showing ;-). XLogInsert routinely shows up as a major
> CPU hog in any update-intensive test, and AFAICT that's mostly from the
> CRC calculation for WAL records.
>
> We could possibly use something cheaper than a real CRC, though. A
> word-wide XOR (ie, effectively a parity calculation) would be sufficient
> to detect most problems.

That was something that I mentioned in my first response. if the *only*
purpose of the check is to generate a "pass" or "fail" status, and not
something to be used to find where in the block it is corrupted or attempt
to regenerate the data, then we could certainly optimize the check
algorithm. A simple checksum may be good enough.

On Wed, Oct 1, 2008 at 11:36 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> I probably wouldn't compare checksumming *every* WAL record to a
>> single block-level checksum.
>
> No, not at all. Block-level checksums would be an order of magnitude
> more expensive: they're on bigger chunks of data and they'd be done more
> often.

That's debatable and would be dependent on cache and the workload.

In our case however, because shared buffers doesn't scale, we would
end up doing a lot more block-level checksums than the other vendors
just pushing the block to/from the OS cache.

--
Jonah H. Harris, Senior DBA
myYearbook.com

Tom Lane escribió:
> "Jonah H. Harris" <jonah(dot)harris(at)gmail(dot)com> writes:

> > I probably wouldn't compare checksumming *every* WAL record to a
> > single block-level checksum.
>
> No, not at all. Block-level checksums would be an order of magnitude
> more expensive: they're on bigger chunks of data and they'd be done more
> often.

More often? My intention is that they are checked when the buffer is
read in, and calculated/stored when the buffer is written out.
In-memory changers of the block do not check nor recalculate the sum.

Is this not OK?

--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> writes:

> "Jonah H. Harris" <jonah(dot)harris(at)gmail(dot)com> writes:
>> On Wed, Oct 1, 2008 at 10:27 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>> Your optimism is showing ;-). XLogInsert routinely shows up as a major
>>> CPU hog in any update-intensive test, and AFAICT that's mostly from the
>>> CRC calculation for WAL records.
>
>> I probably wouldn't compare checksumming *every* WAL record to a
>> single block-level checksum.
>
> No, not at all. Block-level checksums would be an order of magnitude
> more expensive: they're on bigger chunks of data and they'd be done more
> often.

Yeah, it's not a single block, it's the total amount that matters and that's
going to amount to the entire i/o bandwidth of the database. That said I think
the reason WAL checksums are so expensive is the startup and finishing cost.

I wonder if we could do something clever here though. Only one process is busy
calculating the checksum -- it just has to know if anyone fiddles the hint
bits while it's busy.

If setting a hint bit cleared a flag on the buffer header then the
checksumming process could set that flag, begin checksumming, and check that
the flag is still set when he's finished.

Actually I suppose that wouldn't actually be good enough. He would have to do
the i/o and check that the checksum was still valid after the i/o. If not then
he would have to recalculate the checksum and repeat the i/o. That might make
the idea a loser since I think the only way it wins is if you rarely actually
get someone setting the hint bits during i/o anyways.

--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com
Get trained by Bruce Momjian - ask me about EnterpriseDB's PostgreSQL training!

On Wed, Oct 1, 2008 at 11:57 AM, Alvaro Herrera
<alvherre(at)commandprompt(dot)com> wrote:
> Tom Lane escribió:
>> No, not at all. Block-level checksums would be an order of magnitude
>> more expensive: they're on bigger chunks of data and they'd be done more
>> often.
>
> More often? My intention is that they are checked when the buffer is
> read in, and calculated/stored when the buffer is written out.
> In-memory changers of the block do not check nor recalculate the sum.
>
> Is this not OK?

That is the way it should work, only on read-in/write-out.

--
Jonah H. Harris, Senior DBA
myYearbook.com

* Heikki Linnakangas:

> Currently, hint bit updates are not WAL-logged, and thus no full page
> write is done when only hint bits are changed. Imagine what happens if
> hint bits are updated on a page, but there's no other changes, and we
> crash so that only one half of the new page version makes it to disk
> (= torn page). The CRC would not match, even though the page is
> actually valid.

The non-logged hint bit writes are somewhat dangerous anyway. Maybe
it's time to get rid of this peculiarity, despite the performance
impact?

--
Florian Weimer <fweimer(at)bfk(dot)de>
BFK edv-consulting GmbH http://www.bfk.de/
Kriegsstraße 100 tel: +49-721-96201-1
D-76133 Karlsruhe fax: +49-721-96201-99

Alvaro Herrera <alvherre(at)commandprompt(dot)com> writes:
> Tom Lane escribi:
>> No, not at all. Block-level checksums would be an order of magnitude
>> more expensive: they're on bigger chunks of data and they'd be done more
>> often.

> More often? My intention is that they are checked when the buffer is
> read in, and calculated/stored when the buffer is written out.

Right. My point is that the volume of data involved is more than the
WAL traffic. Example: you update one tuple on a page, your WAL record
is that tuple, but you had to checksum the whole page when you read it
in and you'll have to do it again when you write it out. Not to mention
that in a read-mostly query mix there's not much WAL traffic at all,
but plenty of page reading (and maybe some writing too, if hint bit
updates happen).

"Order of magnitude" might be an overstatement, but I don't believe
for a moment that the cost will be negligible. That's why I'm thinking
about something cheaper than a full-blown CRC calculation.

regards, tom lane

On Wed, 2008-10-01 at 16:57 +0100, Gregory Stark wrote:
> I wonder if we could do something clever here though. Only one process
> is busy
> calculating the checksum -- it just has to know if anyone fiddles the hint
> bits while it's busy.

What if the hint bits are added at the very end to the checksum, with an
exclusive lock to them ? Then the exclusive lock should be short
enough... only it might be deadlock-prone as any lock upgrade...

Cheers,
Csaba.

* Tom Lane:

> No, not at all. Block-level checksums would be an order of magnitude
> more expensive: they're on bigger chunks of data and they'd be done more
> often.

For larger blocks, checksumming can be parallelized at the instruction
level, especially if the block size is statically known. And for
large blocks, Adler32 isn't that bad compared to CRC32 from a error
detection POV, so maybe you could use that.

I've seen faults which were uncovered by page-level checksumming, so
I'd be willing to pay the performance cost. 8-/

--
Florian Weimer <fweimer(at)bfk(dot)de>
BFK edv-consulting GmbH http://www.bfk.de/
Kriegsstraße 100 tel: +49-721-96201-1
D-76133 Karlsruhe fax: +49-721-96201-99

* Gregory Stark <stark(at)enterprisedb(dot)com> [081001 11:59]:

> If setting a hint bit cleared a flag on the buffer header then the
> checksumming process could set that flag, begin checksumming, and check that
> the flag is still set when he's finished.
>
> Actually I suppose that wouldn't actually be good enough. He would have to do
> the i/o and check that the checksum was still valid after the i/o. If not then
> he would have to recalculate the checksum and repeat the i/o. That might make
> the idea a loser since I think the only way it wins is if you rarely actually
> get someone setting the hint bits during i/o anyways.

A doubled-write is essentially "free" with PostgreSQL because it's not
doing direct IO, rather relying on the OS page cache to be efficient.
So if you write block A and then call write on block A immediately (or,
realistically, after a redo of the checksum), the first write is almost
*never* going to take IO bandwidth to your spindles...

But the problem is if something crashes (or interrupts PG) between those
two writes, you've got a block of data into the pagecache (and possibly
to the disks) that PG will no longer read in, because the CRC/checksum
fails despite the actual content being valid...

So if you're going to be makeing PG refuse to read-in blocks with bad
CRC/csum, you need to guarnetee that nothing fiddles with the block
between the start of the CRC and the completion of the write().

One possibility would be to "double-buffer" the write... i.e. as you
calculate your CRC, you're doing it on a local copy of the block, which
you hand to the OS to write... If you're touching the whole block of
memory to CRC it, it isn't *ridiculously* more expensive to copy the
memory somewhere else as you do it...

a.

--
Aidan Van Dyk Create like a god,
aidan(at)highrise(dot)ca command like a king,
http://www.highrise.ca/ work like a slave.

Aidan Van Dyk wrote:
> One possibility would be to "double-buffer" the write... i.e. as you
> calculate your CRC, you're doing it on a local copy of the block, which
> you hand to the OS to write... If you're touching the whole block of
> memory to CRC it, it isn't *ridiculously* more expensive to copy the
> memory somewhere else as you do it...
>

Coming in to this late - so apologies if this makes no sense - but
doesn't writev() provide the required capability?

Also, what is the difference between the OS not writing the block at
all, and writing the block but missing the checksum? This window seems
to be small enough (most of the time being faster than the system can
schedule the buffer to be dumped?) that the "additional risk" seems
theoretical rather than real. Unless there is evidence that writev()
performs poorly, I'd suggest that avoiding double-buffering by using
writev() would be preferred.

Cheers,
mark

--
Mark Mielke <mark(at)mielke(dot)cc>

Aidan Van Dyk <aidan(at)highrise(dot)ca> writes:

> * Gregory Stark <stark(at)enterprisedb(dot)com> [081001 11:59]:
>
>> If setting a hint bit cleared a flag on the buffer header then the
>> checksumming process could set that flag, begin checksumming, and check that
>> the flag is still set when he's finished.
>>
>> Actually I suppose that wouldn't actually be good enough. He would have to do
>> the i/o and check that the checksum was still valid after the i/o. If not then
>> he would have to recalculate the checksum and repeat the i/o. That might make
>> the idea a loser since I think the only way it wins is if you rarely actually
>> get someone setting the hint bits during i/o anyways.
>
> A doubled-write is essentially "free" with PostgreSQL because it's not
> doing direct IO, rather relying on the OS page cache to be efficient.

All things are relative. What we're talking about here is all cpu and
memory-bandwidth costs anyways so, yes, it'll be cheap compared to the disk
i/o but it'll still represent doubling the memory bandwidth and cpu cost of
these routines.

That said you would only have to do it in cases where the hint bits actually
get twiddled. That might not actually happen often.

> But the problem is if something crashes (or interrupts PG) between those
> two writes, you've got a block of data into the pagecache (and possibly
> to the disks) that PG will no longer read in, because the CRC/checksum
> fails despite the actual content being valid...

I don't think this is a problem because we're still doing WAL logging. The i/o
isn't allowed to happen until the page has been WAL logged and fsynced
anyways.

Incidentally I think the JUST_DIRTIED bit might actually be sufficient here.
Hint bits already cause the buffer to be marked dirty. So the only case I see
a real problem for is when we're writing a block as part of a checkpoint and
find it's JUST_DIRTIED after writing it. In that case we would have to start
over and write it again rather than leave it marked dirty.

If we're writing the block as part of normal i/o then we could just decide to
leave the possibly-bogus checksum in the table since it'll be overwritten by a
full page write anyways. It'll be overwritten in normal use when the newly
dirty buffer is eventually written out again.

If you're not doing full page writes then you would have to restore from
backup in cases where previously the page might actually have been valid
though. That's kind of unfortunate. In theory it hasn't actually changed
anything the risks of running without full page writes but it has certainly
increased the likelihood of actually having to deal with "corruption" in the
form of a gratuitously invalid checksum. (Of course without checksums you
don't ever actually know if you have corruption -- and real corruption).

> One possibility would be to "double-buffer" the write... i.e. as you
> calculate your CRC, you're doing it on a local copy of the block, which
> you hand to the OS to write... If you're touching the whole block of
> memory to CRC it, it isn't *ridiculously* more expensive to copy the
> memory somewhere else as you do it...

Hm. Well that might actually work. You can do the CRC at the same time as
copying to the buffer, effectively doing it for the same cost as the CRC
alone.

--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com
Ask me about EnterpriseDB's On-Demand Production Tuning

On Wed, Oct 01, 2008 at 11:57:31AM -0400, Alvaro Herrera wrote:
> Tom Lane escribió:
> > "Jonah H. Harris" <jonah(dot)harris(at)gmail(dot)com> writes:
>
> > > I probably wouldn't compare checksumming *every* WAL record to a
> > > single block-level checksum.
> >
> > No, not at all. Block-level checksums would be an order of magnitude
> > more expensive: they're on bigger chunks of data and they'd be done more
> > often.
>
> More often? My intention is that they are checked when the buffer is
> read in, and calculated/stored when the buffer is written out.
> In-memory changers of the block do not check nor recalculate the sum.

I know you said detecting memory errors wasn't being attempted, but
bad memory accounts for a reasonable number of reports of database
corruption on -general so I was wondering if moving the checks around
could catch some of these.

How about updating the block's checksum immediately whenever a tuple is
modified within it? Checksums would be checked whenever data is read in
and just before it's written out. Checksum failure on write would cause
PG to abort noisily with complaints about bad memory?

If some simple checksum could be found (probably a parity check) that
would allow partial updates PG wouldn't need to scan too much data when
regenerating it. It would also cause the checksum to stay bad if data
goes bad in memory between reading from disk and its eventual write out,
a block remaining in cache for a large amount of time makes this case
appear possible.

Sam

Aidan Van Dyk <aidan(at)highrise(dot)ca> writes:
> One possibility would be to "double-buffer" the write... i.e. as you
> calculate your CRC, you're doing it on a local copy of the block, which
> you hand to the OS to write... If you're touching the whole block of
> memory to CRC it, it isn't *ridiculously* more expensive to copy the
> memory somewhere else as you do it...

That actually seems like a really good idea. We don't have to increase
the buffer locking requirements, or make much of any change at all in
the existing logic. +1, especially if this is intended to be an
optional feature (which I agree with).

regards, tom lane

> Aidan Van Dyk <aidan(at)highrise(dot)ca> writes:
>> One possibility would be to "double-buffer" the write... i.e. as you
>> calculate your CRC, you're doing it on a local copy of the block, which
>> you hand to the OS to write... If you're touching the whole block of
>> memory to CRC it, it isn't *ridiculously* more expensive to copy the
>> memory somewhere else as you do it...
>
> That actually seems like a really good idea. We don't have to increase
> the buffer locking requirements, or make much of any change at all in
> the existing logic. +1, especially if this is intended to be an
> optional feature (which I agree with).
>
I don't think it make sense at all!!!

If you are going to double buffer, one presumes that for some non-zero
period of time, the block must be locked during which it is copied. You
wouldn't want it changing "mid-copy" would you? How is this any less of a
hit than just calculating the checksum?

pgsql(at)mohawksoft(dot)com writes:
>> That actually seems like a really good idea.

> I don't think it make sense at all!!!

> If you are going to double buffer, one presumes that for some non-zero
> period of time, the block must be locked during which it is copied. You
> wouldn't want it changing "mid-copy" would you? How is this any less of a
> hit than just calculating the checksum?

It only has to be share-locked. That locks out every change *but* hint
bit updates, and we don't really care whether we catch a hint bit update
or not, so long as it doesn't break our CRC. This is really exactly the
same way it works now: there is no way to know whether the page image
sent to the kernel includes hint bit updates made after the write() call
starts. But we don't care. (The JUST_DIRTIED business ensures that
we'll catch any such hint bit update next time.)

Thought experiment: suppose that write() had some magic option that made
it calculate a CRC on the data after it was pulled from userspace, while
it's sitting in a kernel buffer. Then we'd have exactly the same
guarantees as we do now about the consistency of data written to disk,
except that the CRC magically got in there too. The double-buffer idea
implements exactly that behavior, without any magic write option.

regards, tom lane

pgsql(at)mohawksoft(dot)com writes:

> If you are going to double buffer, one presumes that for some non-zero
> period of time, the block must be locked during which it is copied. You
> wouldn't want it changing "mid-copy" would you? How is this any less of a
> hit than just calculating the checksum?

a) You wouldn't have to keep the lock while doing the I/O. Once the CRC+copy
is done you can release the lock secure in the knowledge that nobody is going
to modify your buffered copy before the kernel can grab its copy.

b) You don't have to worry about hint bits being modified underneath you. As
long as the CRC+copy is carefully written to copy whole atomic-sized
words/bytes and only read the original once then it won't matter if it catches
the hint bit before or after it's set. The CRC will reflect the value buffered
and eventually written.

--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com
Ask me about EnterpriseDB's RemoteDBA services!

Gregory Stark <stark(at)enterprisedb(dot)com> writes:
> a) You wouldn't have to keep the lock while doing the I/O.

Hoo, yeah, so the period of holding the share-lock could well be
*shorter* than it is now. Most especially so if the write() blocks
instead of just transferring the data to kernel space and returning.

I wonder whether that could mean that it's a win to double-buffer
even if we aren't computing a checksum? Nah, probably not.

regards, tom lane

* Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> [081001 19:42]:
> Gregory Stark <stark(at)enterprisedb(dot)com> writes:
> > a) You wouldn't have to keep the lock while doing the I/O.
>
> Hoo, yeah, so the period of holding the share-lock could well be
> *shorter* than it is now. Most especially so if the write() blocks
> instead of just transferring the data to kernel space and returning.
>
> I wonder whether that could mean that it's a win to double-buffer
> even if we aren't computing a checksum? Nah, probably not.

That all depends on what you think is longer: copy 8K & release, or
syscall(which obviously does a copy)+return & release... And whether you
want to shorten the lock hold time (but it's only shared), or the time
until write is done (isn't the goal to have writes being done in the
background during checkpoint so write latency isn't a problem)... This
might be an interesting experiment for someone to do with a very high
concurrency, high-write load...

a.

--
Aidan Van Dyk Create like a god,
aidan(at)highrise(dot)ca command like a king,
http://www.highrise.ca/ work like a slave.

Rather than potentially letting this slide past 8.4, I threw together
an extremely quick-hack patch at the smgr-layer for block-level
checksums.

There are some nasties in that the CRC is the first member of
PageHeaderData (in order to guarantee inclusion of the LSN), and that
it bumps the size of the page header from 24-32 bytes on MAXALIGN=8.
I really think most people should use checksums and so I didn't make
it optional storage within the page (which would of course increase
the complexity).

Second, I didn't bump page version numbers.

Third, rather than a zero-filled page (which has been commonplace for
as long as I can remember), I used a fixed magic number (0xdeadbeef)
for pd_checksum as the default CRC; that way, if someone
enables/disables it at runtime, they won't get invalid checksums for
blocks which hadn't been checksummed previously. This may as well be
zero (which means PageInit/PageHeaderIsValid wouldn't have to be
touched), but I used it as a test.

I ran the regressions and several concurrent benchmark tests which
passed successfully, but I'm sure I'm missing quite a bit due to the
the fact that it's late, it's just a quick hack, and I haven't gone
through the buffer manager locking code in awhile.

I'll be happy to work on this or let Alvaro take it; just as long as
it gets done for 8.4.

--
Jonah H. Harris, Senior DBA
myYearbook.com

On Thu, Oct 2, 2008 at 1:29 AM, Jonah H. Harris <jonah(dot)harris(at)gmail(dot)com> wrote:
> I ran the regressions and several concurrent benchmark tests which
> passed successfully, but I'm sure I'm missing quite a bit due to the
> the fact that it's late, it's just a quick hack, and I haven't gone
> through the buffer manager locking code in awhile.

Don't know how I missed this obvious one... should not be coding this
late @ night :(

Patch updated.

--
Jonah H. Harris, Senior DBA
myYearbook.com

Jonah H. Harris wrote:
> Rather than potentially letting this slide past 8.4, I threw together
> an extremely quick-hack patch at the smgr-layer for block-level
> checksums.

One hard problem is how to deal with torn pages with non-WAL-logged
changes. Like heap hint bit updates, killing tuples in index pages
during a scan, and the new FSM pages.

Currently, a torn page when writing a hint-bit-updated page doesn't
matter, but it would break the checksum.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

On Thu, 2008-10-02 at 09:35 +0300, Heikki Linnakangas wrote:
> Jonah H. Harris wrote:
> > Rather than potentially letting this slide past 8.4, I threw together
> > an extremely quick-hack patch at the smgr-layer for block-level
> > checksums.
>
> One hard problem is how to deal with torn pages with non-WAL-logged
> changes. Like heap hint bit updates, killing tuples in index pages
> during a scan, and the new FSM pages.

Hit bit updates and killing tuples in index pages during a scan can
probably be brute-forced quite cheaply after we find a CRC mismatch.
Not sure about new FSM pages.

> Currently, a torn page when writing a hint-bit-updated page doesn't
> matter, but it would break the checksum.

Another idea is to just ignore non-WAL-logged bits when calculating
CRC-s, by masking them out before adding corresponding bytes to CRC.

This requires page-type aware CRC functions and is more expensive to
calculate. How much more expensive is something that only testing can
tell. Probably not very much, as everything needed should be in L1
caches already.

---------------
Hannu

I have a stupid question wrt hint bits and CRC checksums- it seems to me
that it should be possible, if you change the hint bits, to be able to
very easily calculate what the change in the CRC checksum should be.

The basic idea of the CRC checksum is that, given a message x, the
checksum is x mod p where p is some constant polynomial (all operations
are in GF(2^n)). Now, the interesting thing about modulo is that it's
distributable- that is to say, (x ^ y) mod p = (x mod p) ^ (y mod p),
and that
(x * y) mod p = ((x mod p) * (y mod p)) mod p (I'm using ^ instead of
the more traditional + here to emphasize that it's xor, not addition,
I'm doing). So let's assume we're updating a word a known n bytes from
the end of the message- we calculate y = old_value ^ new_value, so our
change is the equivalent of changing the original block m to (m ^ (y *
x^{8n})). The new checksum is then (m ^ (y * x^{8n})) mod p =
(m mod p) ^ (((y mod p) * (x^{8n} mod p)) mod p). Now, m mod p is the
original checksum, and (x^{8n} mod p) is a constant for a given n, and
the multiplication modulo p can be implemented as a set of table
lookups, one per byte.

The take away here is that, if we know ahead of time where the
modifications are going to be, we can make updating the CRC checksum
(relatively) cheap. So, for example, a change of the hint bits would
only need 4 tables lookups and a couple of xors to update the block's
CRC checksum. We could extended this idea- break the 8K page up into,
say, 32 256-byte "subblocks". Changing any given subblock would require
only re-checksumming that subblock and then updating the CRC checksum.
The reason for the subblocks would be to limit the number of tables
necessary- each subblock requires it's own set of 4 256-word tables, so
having 32 subblocks means that the tables involved would be 32*4*256*4 =
128K in size. Going to, say, 64 byte subblocks means needing 128 tables
or 512K of tables.

If people are interested, I could bang out the code tonight, and post it
to the list tomorrow.

Brian

On Thu, Oct 2, 2008 at 9:07 AM, Brian Hurt <bhurt(at)janestcapital(dot)com> wrote:
> I have a stupid question wrt hint bits and CRC checksums- it seems to me
> that it should be possible, if you change the hint bits, to be able to very
> easily calculate what the change in the CRC checksum should be.

Doesn't the problem still remain? The problem being that the buffer
can be changed as it's written, yes?

--
Jonah H. Harris, Senior DBA
myYearbook.com

Jonah H. Harris wrote:
> On Thu, Oct 2, 2008 at 9:07 AM, Brian Hurt <bhurt(at)janestcapital(dot)com> wrote:
>
>> I have a stupid question wrt hint bits and CRC checksums- it seems to me
>> that it should be possible, if you change the hint bits, to be able to very
>> easily calculate what the change in the CRC checksum should be.
>>
>
> Doesn't the problem still remain? The problem being that the buffer
> can be changed as it's written, yes?
>
>
Another possibility is to just not checksum the hint bits...

Brian

"Jonah H. Harris" <jonah(dot)harris(at)gmail(dot)com> writes:

> On Thu, Oct 2, 2008 at 9:07 AM, Brian Hurt <bhurt(at)janestcapital(dot)com> wrote:
>> I have a stupid question wrt hint bits and CRC checksums- it seems to me
>> that it should be possible, if you change the hint bits, to be able to very
>> easily calculate what the change in the CRC checksum should be.
>
> Doesn't the problem still remain? The problem being that the buffer
> can be changed as it's written, yes?

It's even worse than that. Two processes can both be fiddling hint bits on
different tuples (or even the same tuple) at the same time.

--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com
Ask me about EnterpriseDB's RemoteDBA services!

On Thu, Oct 2, 2008 at 9:36 AM, Brian Hurt <bhurt(at)janestcapital(dot)com> wrote:
> Another possibility is to just not checksum the hint bits...

Seems like that would just complicate matters and prevent a viable checksum.

--
Jonah H. Harris, Senior DBA
myYearbook.com

On Thu, Oct 2, 2008 at 9:42 AM, Gregory Stark <stark(at)enterprisedb(dot)com> wrote:
> It's even worse than that. Two processes can both be fiddling hint bits on
> different tuples (or even the same tuple) at the same time.

Agreed. Back to the double-buffer idea, we could have a temporary
BLCKSZ buffer we could use immediately before write() which we could
copy the block to, perform the checksum on, and write out... is that
what you were thinking Tom?

--
Jonah H. Harris, Senior DBA
myYearbook.com

Brian Hurt wrote:
> Jonah H. Harris wrote:
>> On Thu, Oct 2, 2008 at 9:07 AM, Brian Hurt <bhurt(at)janestcapital(dot)com>
>> wrote:
>>> I have a stupid question wrt hint bits and CRC checksums- it seems to me
>>> that it should be possible, if you change the hint bits, to be able
>>> to very
>>> easily calculate what the change in the CRC checksum should be.
>>
>> Doesn't the problem still remain? The problem being that the buffer
>> can be changed as it's written, yes?
>>
> Another possibility is to just not checksum the hint bits...

That would work. But I'm afraid it'd make the implementation a lot more
invasive, and also slower. The buffer manager would have to know what
kind of a page it's dealing with, heap or index or FSM or what, to know
where the hint bits are. Then it would have to follow the line pointers
to locate the hint bits, and mask them out for the CRC calculation.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com> writes:
> Brian Hurt wrote:
>> Another possibility is to just not checksum the hint bits...

> That would work. But I'm afraid it'd make the implementation a lot more
> invasive, and also slower. The buffer manager would have to know what
> kind of a page it's dealing with, heap or index or FSM or what, to know
> where the hint bits are. Then it would have to follow the line pointers
> to locate the hint bits, and mask them out for the CRC calculation.

Right. The odds are that this'd actually be slower than the
double-buffer method, because of all the added complexity. And it would
really suck from a modularity standpoint to have bufmgr know about all
that.

The problem we still have to solve is torn pages when writing back a
hint-bit update ...

regards, tom lane

Tom Lane wrote:
> Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com> writes:
>> Brian Hurt wrote:
>>> Another possibility is to just not checksum the hint bits...
>
>> That would work. But I'm afraid it'd make the implementation a lot more
>> invasive, and also slower. The buffer manager would have to know what
>> kind of a page it's dealing with, heap or index or FSM or what, to know
>> where the hint bits are. Then it would have to follow the line pointers
>> to locate the hint bits, and mask them out for the CRC calculation.
>
> Right. The odds are that this'd actually be slower than the
> double-buffer method, because of all the added complexity.

I was thinking that masking out the hint bits would be implemented by
copying the page to the temporary buffer, ANDing out the hint bits
there, and then calculating the CRC and issuing the write. So we'd still
need to double-buffer.

> The problem we still have to solve is torn pages when writing back a
> hint-bit update ...

Not checksumming the hint bits *is* a solution to the torn page problem.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

Jonah H. Harris wrote:
> On Thu, Oct 2, 2008 at 1:29 AM, Jonah H. Harris <jonah(dot)harris(at)gmail(dot)com> wrote:
>> I ran the regressions and several concurrent benchmark tests which
>> passed successfully, but I'm sure I'm missing quite a bit due to the
>> the fact that it's late, it's just a quick hack, and I haven't gone
>> through the buffer manager locking code in awhile.
>
> Don't know how I missed this obvious one... should not be coding this
> late @ night :(
>
> Patch updated.
>

I read through this patch and am curious why 0xdeadbeef was used as an
uninitialized value for the page crc. Is this value somehow less likely
to have collisons than zero (or any other arbitrary value)?

Would it not be better to add a boolean bit or byte to inidcate the crc
state?

--
Andrew Chernow
eSilo, LLC
every bit counts
http://www.esilo.com/

On Thu, Oct 2, 2008 at 10:09 AM, Andrew Chernow <ac(at)esilo(dot)com> wrote:
> I read through this patch and am curious why 0xdeadbeef was used as an
> uninitialized value for the page crc. Is this value somehow less likely to
> have collisons than zero (or any other arbitrary value)?

It was just an arbitrary value I chose to identify non-checksummed
pages; I believe would have the same collision rate as anything else.

> Would it not be better to add a boolean bit or byte to inidcate the crc
> state?

Ideally, though we don't have any spare bits to play with in MAXALIGN=4.

--
Jonah H. Harris, Senior DBA
myYearbook.com

Jonah H. Harris wrote:
> On Thu, Oct 2, 2008 at 10:09 AM, Andrew Chernow <ac(at)esilo(dot)com> wrote:
>> Would it not be better to add a boolean bit or byte to inidcate the crc
>> state?
>
> Ideally, though we don't have any spare bits to play with in MAXALIGN=4.

In the page header? There's plenty of free bits in pd_flags.

But isn't it a bit dangerous to have a single flag on the page
indicating whether the CRC is valid or not? Any corruption that flips
that bit would make the CRC check to be skipped.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

On Thu, Oct 2, 2008 at 10:27 AM, Heikki Linnakangas
<heikki(dot)linnakangas(at)enterprisedb(dot)com> wrote:
>> Ideally, though we don't have any spare bits to play with in MAXALIGN=4.
>
> In the page header? There's plenty of free bits in pd_flags.

Ahh, didn't see that. Good catch!

> But isn't it a bit dangerous to have a single flag on the page indicating
> whether the CRC is valid or not? Any corruption that flips that bit would
> make the CRC check to be skipped.

Agreed.

--
Jonah H. Harris, Senior DBA
myYearbook.com

Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com> writes:
> Tom Lane wrote:
>> The problem we still have to solve is torn pages when writing back a
>> hint-bit update ...

> Not checksumming the hint bits *is* a solution to the torn page problem.

Yeah, but it has enough drawbacks that I'd like to keep looking for
alternatives.

One argument that I've not seen raised is that not checksumming the hint
bits leaves you open to a single-bit error that incorrectly sets a hint
bit.

regards, tom lane

Andrew Chernow <ac(at)esilo(dot)com> writes:
> I read through this patch and am curious why 0xdeadbeef was used as an
> uninitialized value for the page crc. Is this value somehow less likely
> to have collisons than zero (or any other arbitrary value)?

Actually, because that's a favorite bit pattern for programs to fill
unused memory with, I'd venture that it has measurably HIGHER odds
of being bogus than any other bit pattern. Consider the possibility
that a database page got overwritten with someone's core dump.

> Would it not be better to add a boolean bit or byte to inidcate the crc
> state?

No, as noted that would give you a one-in-two chance of incorrectly
skipping the CRC check, not one-in-2^32 or so. If we're going to allow
a silent skip of the CRC check then a special value of CRC is a good way
to do it ... just not this particular one.

regards, tom lane

On Thu, Oct 2, 2008 at 10:41 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> Not checksumming the hint bits *is* a solution to the torn page problem.
>
> Yeah, but it has enough drawbacks that I'd like to keep looking for
> alternatives.

Agreed.

> One argument that I've not seen raised is that not checksumming the hint
> bits leaves you open to a single-bit error that incorrectly sets a hint
> bit.

Agreed.

--
Jonah H. Harris, Senior DBA
myYearbook.com

So, it comes down to two possible designs, each with its own set of challenges.

Just to see where to go from here... I want to make sure the options
I've seen in this thread are laid out clearly:

1. Hold an exclusive lock on the buffer during the call to smgrwrite
OR
2. Doublebuffer the write
OR
3. Do some crufty magic to ignore hint-bit updates

Because option 3 not only complicates the entire thing, but also makes
corruption more difficult to detect, I don't consider it viable. Can
anyone provide a reason that makes this option viable?

Option 1 will prevent hint-bit updates during write, which means we
can checksum the buffer and not worry about it. Also, is only the
buffer content lock required? This could potentially slow down
concurrent transactions reading the block and/or writing hint bits.

Option #2 consists of copying the block to a temporary buffer,
checksumming it, and pushing the checksummed block down to write() (at
smgr/md/fd depending on where we want to perform the checksum).

From my perspective, I prefer #2 and performing it at the sgmr layer,
but I am open to suggestions. Tom, what are your thoughts? #1 isn't
very difficult, but I can see it potentially causing a number of
side-problems and it would require a fair amount of testing.

--
Jonah H. Harris, Senior DBA
myYearbook.com

"Jonah H. Harris" <jonah(dot)harris(at)gmail(dot)com> writes:
> Just to see where to go from here... I want to make sure the options
> I've seen in this thread are laid out clearly:

> 1. Hold an exclusive lock on the buffer during the call to smgrwrite
> OR
> 2. Doublebuffer the write
> OR
> 3. Do some crufty magic to ignore hint-bit updates

Right, I think everyone agrees now that #2 seems like the most
preferable option for writing the checksum. However, that still
leaves us lacking a solution for torn pages during a write that
follows a hint bit update. We may end up with some "crufty
magic" anyway for dealing with that.

regards, tom lane

On Tuesday 30 September 2008 17:17:10 Decibel! wrote:
> On Sep 30, 2008, at 1:48 PM, Heikki Linnakangas wrote:
> > Doesn't some filesystems include a per-block CRC, which would
> > achieve the same thing? ZFS?
>
> Sure, some do. We're on linux and can't run ZFS. And I'll argue that
> no linux FS is anywhere near as tested as ext3 is, which means that
> going to some other FS that offers you CRC means you're now exposing
> yourself to the possibility of issues with the FS itself. Not to
> mention that changing filesystems on a large production system is
> very painful.

Actually we had someone on irc yesterday explaining how they were able to run
zfs on debian linux, so that option might be closer than you think.

On a side note, I believe there are a couple of companies that do postgresql
consulting that have pretty good experience running it atop solaris... just
in case you guys ever do want to do a migration or something ;-)

--
Robert Treat
Build A Brighter LAMP :: Linux Apache {middleware} PostgreSQL

* Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> [081002 11:40]:
> "Jonah H. Harris" <jonah(dot)harris(at)gmail(dot)com> writes:
> > Just to see where to go from here... I want to make sure the options
> > I've seen in this thread are laid out clearly:
>
> > 1. Hold an exclusive lock on the buffer during the call to smgrwrite
> > OR
> > 2. Doublebuffer the write
> > OR
> > 3. Do some crufty magic to ignore hint-bit updates
>
> Right, I think everyone agrees now that #2 seems like the most
> preferable option for writing the checksum. However, that still
> leaves us lacking a solution for torn pages during a write that
> follows a hint bit update. We may end up with some "crufty
> magic" anyway for dealing with that.

How does your current "write" strategy handle this situation. I mean,
how do you currently guarnetee that between when you call write() and
the kernel copies the buffer internally, no hint-bit are updated?
#define write(fd, buf, count) buffer_crc_write(fd, buf, count)

whatever protection you have on the regular write is sufficient. The
time of the protection will need to start before the "buffer" period
instead of just the write, (and maybe not the write syscall anymore) but
with CPU caches and speed, the buffer period should be <= the time of
the write() syscall... Your fsync is your "on disk guarentee", not the
write, and that won't change.

But I thought you didn't really care about hint-bit updates, even in the
current strategy... but I'm fully ignorant about the code, sorry...

a.

--
Aidan Van Dyk Create like a god,
aidan(at)highrise(dot)ca command like a king,
http://www.highrise.ca/ work like a slave.

On Thu, 02 Oct 2008 11:57:30 -0400
Robert Treat <xzilla(at)users(dot)sourceforge(dot)net> wrote:

> Actually we had someone on irc yesterday explaining how they were
> able to run zfs on debian linux, so that option might be closer than
> you think.

Its user mode. Not sure I would suggest that from a production server
perspective.

>
> On a side note, I believe there are a couple of companies that do
> postgresql consulting that have pretty good experience running it
> atop solaris... just in case you guys ever do want to do a migration
> or something ;-)

:P

Joshua D. Drake

--
The PostgreSQL Company since 1997: http://www.commandprompt.com/
PostgreSQL Community Conference: http://www.postgresqlconference.org/
United States PostgreSQL Association: http://www.postgresql.us/

On Thu, Oct 2, 2008 at 12:05 PM, Aidan Van Dyk <aidan(at)highrise(dot)ca> wrote:
> How does your current "write" strategy handle this situation. I mean,
> how do you currently guarnetee that between when you call write() and
> the kernel copies the buffer internally, no hint-bit are updated?

Working on the exact double-buffering technique now.

> #define write(fd, buf, count) buffer_crc_write(fd, buf, count)

I certainly wouldn't interpose the write() call itself; that's just
asking for trouble.

> whatever protection you have on the regular write is sufficient. The
> time of the protection will need to start before the "buffer" period
> instead of just the write, (and maybe not the write syscall anymore) but
> with CPU caches and speed, the buffer period should be <= the time of
> the write() syscall... Your fsync is your "on disk guarentee", not the
> write, and that won't change.

Agreed.

> But I thought you didn't really care about hint-bit updates, even in the
> current strategy... but I'm fully ignorant about the code, sorry...

The current implementation does not take it into account.

--
Jonah H. Harris, Senior DBA
myYearbook.com

* Jonah H. Harris <jonah(dot)harris(at)gmail(dot)com> [081002 12:43]:

> > #define write(fd, buf, count) buffer_crc_write(fd, buf, count)
>
> I certainly wouldn't interpose the write() call itself; that's just
> asking for trouble.

Of course not, that was only to show that whatever you currenlty pritect
"write()" with, is valid for protecting the buffer+write.

> > But I thought you didn't really care about hint-bit updates, even in the
> > current strategy... but I'm fully ignorant about the code, sorry...
>
> The current implementation does not take it into account.

So if PG currently doesn't care about the hit-bits being updated, during
the write, then why should introducing a double-buffer introduce the a
torn-page problem Tom mentions? I admit, I'm fishing for information
from those in the know, because I haven't been looking at the code long
enough (or all of it enough) to to know all the ins-and-outs...

a.
--
Aidan Van Dyk Create like a god,
aidan(at)highrise(dot)ca command like a king,
http://www.highrise.ca/ work like a slave.

On Thu, Oct 2, 2008 at 12:51 PM, Aidan Van Dyk <aidan(at)highrise(dot)ca> wrote:
>> > But I thought you didn't really care about hint-bit updates, even in the
>> > current strategy... but I'm fully ignorant about the code, sorry...
>>
>> The current implementation does not take it into account.
>
> So if PG currently doesn't care about the hit-bits being updated, during
> the write, then why should introducing a double-buffer introduce the a
> torn-page problem Tom mentions? I admit, I'm fishing for information
> from those in the know, because I haven't been looking at the code long
> enough (or all of it enough) to to know all the ins-and-outs...

PG doesn't care because during hint-bits aren't logged and during
normal WAL replay, the old page will be pulled from the WAL. I
believe what Tom is referring to is that the buffer PG sends to
write() can still be modified by way of SetHintBits between the time
smgrwrite is called and the time the actual write takes place, which
is why we can't rely on a checksum of the buffer pointer passed to
smgrwrite and friends.

If we're double-buffering the write, I don't see where we could be
introducing a torn-page, as we'd actually be writing a copied version
of the buffer. Will look into this.

--
Jonah H. Harris, Senior DBA
myYearbook.com

Jonah H. Harris wrote:
> PG doesn't care because during hint-bits aren't logged and during
> normal WAL replay, the old page will be pulled from the WAL. I
> believe what Tom is referring to is that the buffer PG sends to
> write() can still be modified by way of SetHintBits between the time
> smgrwrite is called and the time the actual write takes place, which
> is why we can't rely on a checksum of the buffer pointer passed to
> smgrwrite and friends.
>
> If we're double-buffering the write, I don't see where we could be
> introducing a torn-page, as we'd actually be writing a copied version
> of the buffer. Will look into this.

The torn page is during kernel write to disk, I assume, so it is still
possible.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

* Bruce Momjian <bruce(at)momjian(dot)us> [081002 13:07]:
> Jonah H. Harris wrote:
> > PG doesn't care because during hint-bits aren't logged and during
> > normal WAL replay, the old page will be pulled from the WAL. I
> > believe what Tom is referring to is that the buffer PG sends to
> > write() can still be modified by way of SetHintBits between the time
> > smgrwrite is called and the time the actual write takes place, which
> > is why we can't rely on a checksum of the buffer pointer passed to
> > smgrwrite and friends.
> >
> > If we're double-buffering the write, I don't see where we could be
> > introducing a torn-page, as we'd actually be writing a copied version
> > of the buffer. Will look into this.
>
> The torn page is during kernel write to disk, I assume, so it is still
> possible.

Ah, I see... So your full-page-write in the WAL, protecting the torn
page has to be aware of the need for a valid CRC32 as well...

--
Aidan Van Dyk Create like a god,
aidan(at)highrise(dot)ca command like a king,
http://www.highrise.ca/ work like a slave.

On Thu, Oct 2, 2008 at 1:07 PM, Bruce Momjian <bruce(at)momjian(dot)us> wrote:
>> If we're double-buffering the write, I don't see where we could be
>> introducing a torn-page, as we'd actually be writing a copied version
>> of the buffer. Will look into this.
>
> The torn page is during kernel write to disk, I assume, so it is still
> possible.

Well, we can't really control too much of that. The most common
solution to that I've seen is to double-write the page (which some
OSes already do regardless). Or, are you meaning something else?

--
Jonah H. Harris, Senior DBA
myYearbook.com

On 2 Oct 2008, at 05:51 PM, Aidan Van Dyk <aidan(at)highrise(dot)ca> wrote:

> So if PG currently doesn't care about the hit-bits being updated,
> during
> the write, then why should introducing a double-buffer introduce the a
> torn-page problem Tom mentions? I admit, I'm fishing for information
> from those in the know, because I haven't been looking at the code
> long
> enough (or all of it enough) to to know all the ins-and-outs...

It's not the buffeting it's the checksum. The problem arises if a page
is read in but no wal logged modifications are done against it. If a
hint bit is modified it won't be wal logged but the page is marked
dirty.

When we write the page there's a chance only part of the page actually
makes it to disk if the system crashes before the whole page is flushed.

Wal logged changes are safe because of full_page_writes. Hint bits are
safe because either the old or the new value will be on disk and we
don't care which. It doesn't matter if some hint bits are set and some
aren't.

However the checksum won't match because the checksum will have been
calculated on the whole block and part of it was never written.

Writing this explanation did bring to mind one solution which we had
already discussed for other reasons: not marking blocks dirty after
hint bit setting.

Alternatively if we detect a block is dirty but the lsn is older than
the last checkpoint is that the only time we need to worry? Then we
could either discard the writes or generate a noop wal log record just
for the full page write in that case.

Jonah H. Harris wrote:
> On Thu, Oct 2, 2008 at 1:07 PM, Bruce Momjian <bruce(at)momjian(dot)us> wrote:
> >> If we're double-buffering the write, I don't see where we could be
> >> introducing a torn-page, as we'd actually be writing a copied version
> >> of the buffer. Will look into this.
> >
> > The torn page is during kernel write to disk, I assume, so it is still
> > possible.
>
> Well, we can't really control too much of that. The most common
> solution to that I've seen is to double-write the page (which some
> OSes already do regardless). Or, are you meaning something else?

I just don't see how writing a copy of the page (rather than the
original) to the kernel affects issues about torn pages.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

> It's not the buffeting it's the checksum. The problem arises if a page is
> read in but no wal logged modifications are done against it. If a hint bit
> is modified it won't be wal logged but the page is marked dirty.

Ahhhhh. Thanks Greg. Let me look into this a bit before I respond :)

--
Jonah H. Harris, Senior DBA
myYearbook.com

Greg Stark escribió:

> Writing this explanation did bring to mind one solution which we had
> already discussed for other reasons: not marking blocks dirty after hint
> bit setting.

How about when a hint bit is set and the page is not already dirty, set
the checksum to the "always valid" value? The problem I have with this
idea is that there would be lots of pages excluded from the CRC checks,
a non-trivial percentage of the time.

Maybe we could mix this with Simon's approach to counting hint bit
setting, and calculate a valid CRC on the page every n-th non-logged
change.

--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

* Greg Stark <greg(dot)stark(at)enterprisedb(dot)com> [081002 13:37]:

> It's not the buffeting it's the checksum. The problem arises if a page
> is read in but no wal logged modifications are done against it. If a
> hint bit is modified it won't be wal logged but the page is marked
> dirty.
>
> When we write the page there's a chance only part of the page actually
> makes it to disk if the system crashes before the whole page is flushed.

Yup, Brucess message pointed me in the right direction..

> Wal logged changes are safe because of full_page_writes. Hint bits are
> safe because either the old or the new value will be on disk and we
> don't care which. It doesn't matter if some hint bits are set and some
> aren't.
>
> However the checksum won't match because the checksum will have been
> calculated on the whole block and part of it was never written.

Correct. But now doesn't full-page-writes give us the same protection
here against a half-write as it did for the previous case?

On recovery after a torn-page write, won't the recovery of the
full_page_write WAL + WAL changes get us back to the page as it was
before the buffer+checksum+write? The checksum on that block *now in
memory* is irrelevant, because it wasn't "read from disk", it was
completely constructed from WAL records, which are protected by
checksums themselves individually..., and is now ready to be "written to
disk" which will force a valid checksum to be on it.

a.

--
Aidan Van Dyk Create like a god,
aidan(at)highrise(dot)ca command like a king,
http://www.highrise.ca/ work like a slave.

Aidan Van Dyk <aidan(at)highrise(dot)ca> writes:

>> Wal logged changes are safe because of full_page_writes. Hint bits are
>> safe because either the old or the new value will be on disk and we
>> don't care which. It doesn't matter if some hint bits are set and some
>> aren't.
>>
>> However the checksum won't match because the checksum will have been
>> calculated on the whole block and part of it was never written.
>
> Correct. But now doesn't full-page-writes give us the same protection
> here against a half-write as it did for the previous case?
>
> On recovery after a torn-page write, won't the recovery of the
> full_page_write WAL + WAL changes get us back to the page as it was
> before the buffer+checksum+write?

Hint bit setting doesn't trigger a WAL record.

--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com
Ask me about EnterpriseDB's PostGIS support!

On Thu, Oct 2, 2008 at 1:58 PM, Gregory Stark <stark(at)enterprisedb(dot)com> wrote:
>> On recovery after a torn-page write, won't the recovery of the
>> full_page_write WAL + WAL changes get us back to the page as it was
>> before the buffer+checksum+write?
>
> Hint bit setting doesn't trigger a WAL record.

Hence, no page image is written to WAL for later use in recovery.

--
Jonah H. Harris, Senior DBA
myYearbook.com

On Thu, Oct 2, 2008 at 1:44 PM, Alvaro Herrera
<alvherre(at)commandprompt(dot)com> wrote:
> How about when a hint bit is set and the page is not already dirty, set
> the checksum to the "always valid" value? The problem I have with this
> idea is that there would be lots of pages excluded from the CRC checks,
> a non-trivial percentage of the time.

I don't like that because it trades-off corruption detection (the
whole point of this feature) for a slight performance improvement.

> Maybe we could mix this with Simon's approach to counting hint bit
> setting, and calculate a valid CRC on the page every n-th non-logged
> change.

I still think we should only calculate checksums on the actual write.
And, this still seems to have an issue with WAL, unless Simon's
original idea somehow included recording hint bit settings/dirtying
the page in WAL.

--
Jonah H. Harris, Senior DBA
myYearbook.com

* Jonah H. Harris <jonah(dot)harris(at)gmail(dot)com> [081002 14:01]:
> On Thu, Oct 2, 2008 at 1:58 PM, Gregory Stark <stark(at)enterprisedb(dot)com> wrote:
> >> On recovery after a torn-page write, won't the recovery of the
> >> full_page_write WAL + WAL changes get us back to the page as it was
> >> before the buffer+checksum+write?
> >
> > Hint bit setting doesn't trigger a WAL record.
>
> Hence, no page image is written to WAL for later use in recovery.

OK. Got it... The block is dirty (only because of hint bits). write
starts, crash, torn page, recovery doesn't "fix" the torn page...
because it's never been changed (according WAL), so on next read...

Without the CRC it doesn't matter, because the only change was
hint-bits, so the page is half-old+half-new, but new == old+only
hint-bits...

Because ther'es no WAP. the torn page will be read next time that buffer
is needed...

a.

--
Aidan Van Dyk Create like a god,
aidan(at)highrise(dot)ca command like a king,
http://www.highrise.ca/ work like a slave.

On Wednesday 01 October 2008 10:27:52 Tom Lane wrote:
> pgsql(at)mohawksoft(dot)com writes:
> >> No, it's all about time penalties and loss of concurrency.
> >
> > I don't think that the amount of time it would take to calculate and test
> > the sum is even important. It may be in older CPUs, but these days CPUs
> > are so fast in RAM and a block is very small. On x86 systems, depending
> > on page alignment, we are talking about two or three pages that will be
> > "in memory" (They were used to read the block from disk or previously
> > accessed).
>
> Your optimism is showing ;-). XLogInsert routinely shows up as a major
> CPU hog in any update-intensive test, and AFAICT that's mostly from the
> CRC calculation for WAL records.
>

Yeah... for those who run on filesystems that do checksumming for you, I'd bet
they'd much rather see time spent in turning that off rather than
checksumming everything else. (just guessing)

--
Robert Treat
Build A Brighter LAMP :: Linux Apache {middleware} PostgreSQL

Robert Treat wrote:
> On Wednesday 01 October 2008 10:27:52 Tom Lane wrote:

> > Your optimism is showing ;-). XLogInsert routinely shows up as a major
> > CPU hog in any update-intensive test, and AFAICT that's mostly from the
> > CRC calculation for WAL records.
>
> Yeah... for those who run on filesystems that do checksumming for you, I'd bet
> they'd much rather see time spent in turning that off rather than
> checksumming everything else. (just guessing)

I don't think it can be turned off, because ISTR a failed checksum is
used to detect end of the WAL stream to be recovered.

--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

Jonah H. Harris escribió:
> On Thu, Oct 2, 2008 at 1:44 PM, Alvaro Herrera
> <alvherre(at)commandprompt(dot)com> wrote:
> > How about when a hint bit is set and the page is not already dirty, set
> > the checksum to the "always valid" value? The problem I have with this
> > idea is that there would be lots of pages excluded from the CRC checks,
> > a non-trivial percentage of the time.
>
> I don't like that because it trades-off corruption detection (the
> whole point of this feature) for a slight performance improvement.

I agree that giving up corruption detection is not such a hot idea, but
what I'm intending to get back is not performance but correctness (in
this case protection from the torn page problem)

> > Maybe we could mix this with Simon's approach to counting hint bit
> > setting, and calculate a valid CRC on the page every n-th non-logged
> > change.
>
> I still think we should only calculate checksums on the actual write.

Well, if we could trade off a bit of performance for correctness, I
would give up on that :-) However, you're right that this tradeoff is
not what we're having here.

> And, this still seems to have an issue with WAL, unless Simon's
> original idea somehow included recording hint bit settings/dirtying
> the page in WAL.

I have to admit I don't remember exactly how it worked :-) I think the
idea was avoiding setting the page dirty until a certain number of hint
bit setting operations had been done (which I think means it's not
useful for the present purpose).

--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

* Alvaro Herrera <alvherre(at)commandprompt(dot)com> [081002 16:18]:

> > And, this still seems to have an issue with WAL, unless Simon's
> > original idea somehow included recording hint bit settings/dirtying
> > the page in WAL.
>
> I have to admit I don't remember exactly how it worked :-) I think the
> idea was avoiding setting the page dirty until a certain number of hint
> bit setting operations had been done (which I think means it's not
> useful for the present purpose).

How crazy would I be to wonder about the performance impact of doing the
full_page_write xlog block backups at buffer dirtying time
(MarkBufferDirty and SetBufferCommitInfoNeedsSave), instead of at
XLogInsert?

A few thoughts, quite possible not true:

* The xlog backup block records don't need to be synced to disk at the
time of the dirty but they can be synced along with stuff behind it,
although it *needs* to be synced by the time the buffer write() comes
long, otherwise we haven't fixed our torn page probem, so practically,
we may need to sync it for guarentees
* OLAP workloads that handle bulk insert/update/delete are probably
running with full_page_writes off, so don't pay the penalty of extra
xlog writing on all the hint-bits being set
* OLTP workloads with full_page_writes on would have some extra
full_page_writes, but I have a feeling that most dirtied buffers in
OLTP systems are going to get changed by more than just hint-bits soon
eonugh anyways, so it's not a huge net increase
* Slow systems that aren't high-velocity can probably spare a bit more
Xlog bandwith anyways...

But my experience is limitted to my small-scale databases...

--
Aidan Van Dyk Create like a god,
aidan(at)highrise(dot)ca command like a king,
http://www.highrise.ca/ work like a slave.

On Thu, 2008-10-02 at 16:18 -0400, Alvaro Herrera wrote:

> > > Maybe we could mix this with Simon's approach to counting hint bit
> > > setting, and calculate a valid CRC on the page every n-th non-logged
> > > change.
> >
> > I still think we should only calculate checksums on the actual write.
>
> Well, if we could trade off a bit of performance for correctness, I
> would give up on that :-) However, you're right that this tradeoff is
> not what we're having here.
>
> > And, this still seems to have an issue with WAL, unless Simon's
> > original idea somehow included recording hint bit settings/dirtying
> > the page in WAL.
>
> I have to admit I don't remember exactly how it worked :-) I think the
> idea was avoiding setting the page dirty until a certain number of hint
> bit setting operations had been done (which I think means it's not
> useful for the present purpose).

Having read everything so far, the only way I can see to solve the
problem does seem to be to make hint bit setting write WAL. When, is the
question. Every time is definitely the wrong answer.

Hint bit setting approach so far is in two parts: we add code to
separate the concept of "dirty" from "has hint bits set". We already
have some logic that says when to write dirty pages. So we just add some
slightly different logic that says when to write hinted pages.

The main correctness of the idea has been worked out. The difficult part
is the "when to write hinted pages" because its just a heuristic,
subject to argument and lots of performance testing.

--
Simon Riggs www.2ndQuadrant.com
PostgreSQL Training, Services and Support

On Thu, Oct 2, 2008 at 7:42 PM, Jonah H. Harris <jonah(dot)harris(at)gmail(dot)com> wrote:
>> It's not the buffeting it's the checksum. The problem arises if a page is
>> read in but no wal logged modifications are done against it. If a hint bit
>> is modified it won't be wal logged but the page is marked dirty.
>
> Ahhhhh. Thanks Greg. Let me look into this a bit before I respond :)

Hmm, how about, when reading a page:

read the page
if checksum mismatch {
flip the hint bits [1]
if checksum mismatch {
ERROR
} else {
emit a warning, 'found a torn page'
}
}

...that is assuming we know which bit to flip
and that we accept the check will be a bit
weaker. :) OTOH this shouldn't happen too
often, so performance should matter much.

My 0.02

Best regards,
Dawid Kuroczko

[1]: Of course it would be more efficient to flip
the checksum, but it would be tricky. :)
--
.................. ``The essence of real creativity is a certain
: *Dawid Kuroczko* : playfulness, a flitting from idea to idea
: qnex42(at)gmail(dot)com : without getting bogged down by fixated demands.''
`..................' Sherkaner Underhill, A Deepness in the Sky, V. Vinge

On Oct 1, 2008, at 2:03 PM, Sam Mason wrote:
> I know you said detecting memory errors wasn't being attempted, but
> bad memory accounts for a reasonable number of reports of database
> corruption on -general so I was wondering if moving the checks around
> could catch some of these.

Down the road, I would also like to have a sanity check for data
modification that occur while the data is in a buffer, to guard
against memory or CPU errors. But the huge issue there is how to do
it without killing performance. Because there's no obvious solution
to that, I don't want to try and get it in for 8.4.
--
Decibel!, aka Jim C. Nasby, Database Architect decibel(at)decibel(dot)org
Give your computer some brain candy! www.distributed.net Team #1828

"Dawid Kuroczko" <qnex42(at)gmail(dot)com> writes:

> On Thu, Oct 2, 2008 at 7:42 PM, Jonah H. Harris <jonah(dot)harris(at)gmail(dot)com> wrote:
>
> if checksum mismatch {
> flip the hint bits [1]

I did try to make something like that work. But I didn't get anywhere. There
could easily be dozens of bits to flip. The MaxHeapTuplesPerPage is over 200
and you could easily have half of them that don't match the checksum if the
writes happen in 4k chunks. If they happen in 512b chunks then you could have
a lot more. And yes they could easily have all been set at around the same
time because that's often just what a sequential scan does.

And you can't even just set the bits to their "correct" values either before
the checksum or before checking the checksum since the "correct" value changes
over time. By the time you compare the checksum more bits will be settable
than when the page was stored.

--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com
Ask me about EnterpriseDB's PostGIS support!

On Oct 2, 2008, at 3:18 PM, Alvaro Herrera wrote:
> I have to admit I don't remember exactly how it worked :-) I think
> the
> idea was avoiding setting the page dirty until a certain number of
> hint
> bit setting operations had been done (which I think means it's not
> useful for the present purpose).

Well, it would be useful if whenever we magically decided it was time
to write out a page that had only hint-bit updates we generated WAL,
right? Even if it was just a no-op WAL record to ensure we had the
page image in the WAL.

BTW, speaking of torn pages... I've heard that there's some serious
gains to be had by turning full_page_writes to off, but I've never
even dreamed of doing that because I've never seen any real sure-fire
way to check that your hardware can't write torn pages. But if we
have checksums enabled and checked the checksums on a block the first
time we touched it during recovery, we'd be able to detect torn
pages, yet still recover. That would help show that torn pages aren't
possible in a particular environment (though unfortunately I don't
think there's any way to actually prove that they're not).
--
Decibel!, aka Jim C. Nasby, Database Architect decibel(at)decibel(dot)org
Give your computer some brain candy! www.distributed.net Team #1828

OK, I have a stupid question- torn pages are a problem, but only during
recovery. Recovery is (I assume) a fairly rare condition- if data
corruption is going to happen, it's most likely to happen during normal
operation. So why not just turn off CRC checksumming during recovery,
or at least treat it as a much less critical error? During recovery, if
the CRC checksum matches, we can assume the page is good- not only not
corrupt, but not torn either. If the CRC checksum doesn't match, we
don't panic, but maybe we do more careful analysis of the page to make
sure that only the hint bits are wrong. Or maybe not. It's only during
normal operation that a CRC checksum failure would be considered critical.

Feel free to explain to me why I'm an idiot.

Brian

On Fri, Oct 3, 2008 at 3:36 PM, Brian Hurt <bhurt(at)janestcapital(dot)com> wrote:
> OK, I have a stupid question- torn pages are a problem, but only during
> recovery. Recovery is (I assume) a fairly rare condition- if data
> corruption is going to happen, it's most likely to happen during normal
> operation. So why not just turn off CRC checksumming during recovery, or at
> least treat it as a much less critical error? During recovery, if the CRC
> checksum matches, we can assume the page is good- not only not corrupt, but
> not torn either. If the CRC checksum doesn't match, we don't panic, but
> maybe we do more careful analysis of the page to make sure that only the
> hint bits are wrong. Or maybe not. It's only during normal operation that
> a CRC checksum failure would be considered critical.

Well:
1. database half-writes the page X to disk, and there is power outage.
2. we regain the power
2. during recovery database replay all WAL-logged pages. The X page
was not WAL-logged, thus it is not replayed.
3. when replaying is finished, everything looks OK at this point
4. user runs a SELECT which hits page X. Oops, we have a checksum
mismatch.

Best regards,
Dawid Kuroczko
--
.................. ``The essence of real creativity is a certain
: *Dawid Kuroczko* : playfulness, a flitting from idea to idea
: qnex42(at)gmail(dot)com : without getting bogged down by fixated demands.''
`..................' Sherkaner Underhill, A Deepness in the Sky, V. Vinge

Brian Hurt wrote:
> OK, I have a stupid question- torn pages are a problem, but only during
> recovery. Recovery is (I assume) a fairly rare condition- if data
> corruption is going to happen, it's most likely to happen during normal
> operation. So why not just turn off CRC checksumming during recovery,
> or at least treat it as a much less critical error? During recovery, if
> the CRC checksum matches, we can assume the page is good- not only not
> corrupt, but not torn either. If the CRC checksum doesn't match, we
> don't panic, but maybe we do more careful analysis of the page to make
> sure that only the hint bits are wrong. Or maybe not. It's only during
> normal operation that a CRC checksum failure would be considered critical.

Interesting question. The problem is that we don't read all pages in
during recovery. One idea would be to WAL log the page numbers that
might be torn and recompute the checksums on those pages during
recovery.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

* Decibel! <decibel(at)decibel(dot)org> [081002 19:18]:

> Well, it would be useful if whenever we magically decided it was time
> to write out a page that had only hint-bit updates we generated WAL,
> right? Even if it was just a no-op WAL record to ensure we had the
> page image in the WAL.

Well, I'm by no means an expert in the code, but from my looking around
bufmgr and transam yesterady, it really looks like it would be a
modularity nightmare...

But I think that would have the same "total IO" affect as nop WAL record
being generated at the the page being dirtied, which would seem to fit
the code a bit better...

a.

--
Aidan Van Dyk Create like a god,
aidan(at)highrise(dot)ca command like a king,
http://www.highrise.ca/ work like a slave.

So this discussion died with no solution arising to the
hint-bit-setting-invalidates-the-CRC problem.

Apparently the only solution in sight is to WAL-log hint bits. Simon
opines it would be horrible from a performance standpoint to WAL-log
every hint bit set, and I think we all agree with that. So we need to
find an alternative mechanism to WAL log hint bits.

I thought about causing a process that's about to write a page check a
flag that says "this page has been dirtied by someone who didn't bother
to generate WAL". If the flag is set, then the writer process is forced
to write a WAL record containing all hint bits in the page, and only
then it is allowed to write the page (and thus calculate the new CRC).

--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

On Fri, Oct 17, 2008 at 11:26 AM, Alvaro Herrera
<alvherre(at)commandprompt(dot)com> wrote:
> So this discussion died with no solution arising to the
> hint-bit-setting-invalidates-the-CRC problem.

I've been busy.

> Apparently the only solution in sight is to WAL-log hint bits. Simon
> opines it would be horrible from a performance standpoint to WAL-log
> every hint bit set, and I think we all agree with that. So we need to
> find an alternative mechanism to WAL log hint bits.

Agreed.

> I thought about causing a process that's about to write a page check a
> flag that says "this page has been dirtied by someone who didn't bother
> to generate WAL". If the flag is set, then the writer process is forced
> to write a WAL record containing all hint bits in the page, and only
> then it is allowed to write the page (and thus calculate the new CRC).

Interesting idea... let me ponder it for a bit.

--
Jonah H. Harris, Senior DBA
myYearbook.com

I'm far from convinced wal logging hint bits is a non starter. In fact
I doubt the wal log itself I a problem. Having to take the buffer lock
does suck though.

Heikki had a clever idea earlier which was to have two crc checks- one
which skips the hint bits and one dedicated to hint bits. If the
second doesn't match we clear all the hint bits.

The problem with that is that skipping the hint bits for the main crc
would slow it down severely. It would make a lot of sense if the hint
bits were all in a contiguous block of memory but I can't see how to
make that add up.

greg

On 17 Oct 2008, at 05:42 PM, "Jonah H. Harris"
<jonah(dot)harris(at)gmail(dot)com> wrote:

> On Fri, Oct 17, 2008 at 11:26 AM, Alvaro Herrera
> <alvherre(at)commandprompt(dot)com> wrote:
>> So this discussion died with no solution arising to the
>> hint-bit-setting-invalidates-the-CRC problem.
>
> I've been busy.
>
>> Apparently the only solution in sight is to WAL-log hint bits. Simon
>> opines it would be horrible from a performance standpoint to WAL-log
>> every hint bit set, and I think we all agree with that. So we need
>> to
>> find an alternative mechanism to WAL log hint bits.
>
> Agreed.
>
>> I thought about causing a process that's about to write a page
>> check a
>> flag that says "this page has been dirtied by someone who didn't
>> bother
>> to generate WAL". If the flag is set, then the writer process is
>> forced
>> to write a WAL record containing all hint bits in the page, and only
>> then it is allowed to write the page (and thus calculate the new
>> CRC).
>
> Interesting idea... let me ponder it for a bit.
>
> --
> Jonah H. Harris, Senior DBA
> myYearbook.com
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers

On Fri, Oct 17, 2008 at 12:05 PM, Greg Stark
<greg(dot)stark(at)enterprisedb(dot)com> wrote:
> Heikki had a clever idea earlier which was to have two crc checks- one which
> skips the hint bits and one dedicated to hint bits. If the second doesn't
> match we clear all the hint bits.

Sounds overcomplicated to me.

> The problem with that is that skipping the hint bits for the main crc would
> slow it down severely. It would make a lot of sense if the hint bits were
> all in a contiguous block of memory but I can't see how to make that add up.

Agreed.

--
Jonah H. Harris, Senior DBA
myYearbook.com

* Greg Stark <greg(dot)stark(at)enterprisedb(dot)com> [081017 12:05]:
> I'm far from convinced wal logging hint bits is a non starter. In fact
> I doubt the wal log itself I a problem. Having to take the buffer lock
> does suck though.

And remember, you don't even need to WAL all the hint-bit setts:... You
only *need* to get a WAL backup of the block at some *any* point before the
block's written to save from the torn-page on recovery leading to an
inconsistent CRC.

a.
--
Aidan Van Dyk Create like a god,
aidan(at)highrise(dot)ca command like a king,
http://www.highrise.ca/ work like a slave.

On Fri, 2008-10-17 at 12:26 -0300, Alvaro Herrera wrote:

> Apparently the only solution in sight is to WAL-log hint bits. Simon
> opines it would be horrible from a performance standpoint to WAL-log
> every hint bit set, and I think we all agree with that. So we need to
> find an alternative mechanism to WAL log hint bits.

Yes, it's clearly not acceptable bit by bit.

But perhaps writing a single WAL record if you scan whole page and set
all bits at once. Then it makes sense in some cases.

It might be possible to have a partial solution where some blocks have
CRC checks, some not. Most databases have static portions. Any block not
touched for X amount of time (~= to a distance between current LSN and
LSN on block) could have CRC checks added.

Or maybe just make it a table-level option and let users choose if they
want the hit or not.

Or maybe have a new command that you can run whenever you want to set
CRC checks. That way you get to choose. CHECK TABLE?

--
Simon Riggs www.2ndQuadrant.com
PostgreSQL Training, Services and Support

Simon Riggs wrote:
>
> On Fri, 2008-10-17 at 12:26 -0300, Alvaro Herrera wrote:
>
> > Apparently the only solution in sight is to WAL-log hint bits. Simon
> > opines it would be horrible from a performance standpoint to WAL-log
> > every hint bit set, and I think we all agree with that. So we need to
> > find an alternative mechanism to WAL log hint bits.
>
> Yes, it's clearly not acceptable bit by bit.
>
> But perhaps writing a single WAL record if you scan whole page and set
> all bits at once. Then it makes sense in some cases.

Yeah, I thought about that too -- and perhaps give the scan some slop,
so that it will also updates some more hint bits that would be updated
in the next, say, 100 transactions. However this seems more messy than
the other idea.

> It might be possible to have a partial solution where some blocks have
> CRC checks, some not.

That's another idea but it reduces the effectiveness of the check.

--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

On Fri, 2008-10-17 at 13:59 -0300, Alvaro Herrera wrote:

> > It might be possible to have a partial solution where some blocks have
> > CRC checks, some not.
>
> That's another idea but it reduces the effectiveness of the check.

If you put in a GUC to control the check, block by block. 0 = check
every time, with full impact. Other values delay the use of CRC checks.
Kind of like freezing parameters. Let people choose.

--
Simon Riggs www.2ndQuadrant.com
PostgreSQL Training, Services and Support

I don't think that works anyways. No matter how thoroughly you update
all the hint bits there's still a chance someone else comes along and
sets one you missed or is setting hint bits on the same tuple at the
same time and your update gets lost.

greg

On 17 Oct 2008, at 06:59 PM, Alvaro Herrera
<alvherre(at)commandprompt(dot)com> wrote:

> Simon Riggs wrote:
>>
>> On Fri, 2008-10-17 at 12:26 -0300, Alvaro Herrera wrote:
>>
>>> Apparently the only solution in sight is to WAL-log hint bits.
>>> Simon
>>> opines it would be horrible from a performance standpoint to WAL-log
>>> every hint bit set, and I think we all agree with that. So we
>>> need to
>>> find an alternative mechanism to WAL log hint bits.
>>
>> Yes, it's clearly not acceptable bit by bit.
>>
>> But perhaps writing a single WAL record if you scan whole page and
>> set
>> all bits at once. Then it makes sense in some cases.
>
> Yeah, I thought about that too -- and perhaps give the scan some slop,
> so that it will also updates some more hint bits that would be updated
> in the next, say, 100 transactions. However this seems more messy
> than
> the other idea.
>
>> It might be possible to have a partial solution where some blocks
>> have
>> CRC checks, some not.
>
> That's another idea but it reduces the effectiveness of the check.
>
> --
> Alvaro Herrera http://www.CommandPrompt.com/
> The PostgreSQL Company - Command Prompt, Inc.
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers

Hi,

Alvaro Herrera wrote:
> So this discussion died with no solution arising to the
> hint-bit-setting-invalidates-the-CRC problem.

Isn't double-buffering solving this issue? Has somebody checked if it
even helps performance due to being able to release the lock on the
buffer *before* the syscall?

Regards

Markus Wanner

Markus Wanner wrote:
> Alvaro Herrera wrote:
>> So this discussion died with no solution arising to the
>> hint-bit-setting-invalidates-the-CRC problem.
>
> Isn't double-buffering solving this issue? Has somebody checked if it
> even helps performance due to being able to release the lock on the
> buffer *before* the syscall?

Double-buffering helps with the hint bit issues within shared buffers,
but doesn't help with the torn page and hint bits problem. The torn page
problem seems to be the show-stopper.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

Simon Riggs wrote:

> But perhaps writing a single WAL record if you scan whole page and set
> all bits at once. Then it makes sense in some cases.

So this is what I ended up doing; attached.

There are some gotchas in this patch:

1. it does not consider hint bits other than the ones defined in htup.h.
Some index AMs use hint bits to "kill" tuples (LP_DEAD mostly, I think).
This means that CRCs will be broken for such pages when pages are torn.

2. some parts of the code could be considered modularity violations.
For example, tqual.c is setting a bit in a Page structure; bufmgr.c is
later checking that bit to determine when to log.

3. the bgwriter is seen writing WAL entries at checkpoint. At shutdown,
this might cause an error to be reported on how there was not supposed
to be activity on the log. I didn't save the exact error report and I
can't find it in the source :-(

So it "mostly works" at this time. I very much welcome opinions to
improve the weak points.

--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

Alvaro Herrera wrote:

> So this is what I ended up doing; attached.

Oh, another thing. The contents for the WAL log message here is very
simplistic; just store all the t_infomask and t_infomask2 relevant bits,
for all the tuples in the table. A possible optimization to reduce the
WAL traffic is to add another infomask bit which indicates whether a
hint bit has been set since the last time we visited the page. I'm
unsure if this is worth the pain. (Another possibility, even more
painful, is to choose at runtime between the two formats, depending on
the number of tuples that need hint bits logged.)

--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

Alvaro Herrera wrote:

> There are some gotchas in this patch:
>
> 1. it does not consider hint bits other than the ones defined in htup.h.
> Some index AMs use hint bits to "kill" tuples (LP_DEAD mostly, I think).
> This means that CRCs will be broken for such pages when pages are torn.

The "other hint bits" are:

- LP_DEAD as used by the various callers of ItemIdMarkDead.
- PD_PAGE_FULL
- BTPageOpaque->btpo_flags and btpo_cycleid

All of them are changed with only SetBufferCommitInfoNeedsSave being
called afterwards.

--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

Alvaro Herrera wrote:

> 3. the bgwriter is seen writing WAL entries at checkpoint. At shutdown,
> this might cause an error to be reported on how there was not supposed
> to be activity on the log. I didn't save the exact error report and I
> can't find it in the source :-(

LOG: received fast shutdown request
LOG: aborting any active transactions
FATAL: terminating connection due to administrator command
LOG: autovacuum launcher shutting down
LOG: shutting down
LOG: INSERT @ 0/67F05F0: prev 0/67F05C0; xid 0: Heap2 - hintbits: rel 1663/16384/1259; blk 4
CONTEXT: writing block 4 of relation 1663/16384/1259
LOG: xlog flush request 0/67F06C0; write 0/0; flush 0/0
CONTEXT: writing block 4 of relation 1663/16384/1259
LOG: INSERT @ 0/67F06C0: prev 0/67F05F0; xid 0: Heap2 - hintbits: rel 1663/16384/2608; blk 40
CONTEXT: writing block 40 of relation 1663/16384/2608
LOG: xlog flush request 0/67F0708; write 0/67F06C0; flush 0/67F06C0
CONTEXT: writing block 40 of relation 1663/16384/2608
LOG: INSERT @ 0/67F0708: prev 0/67F06C0; xid 0: Heap2 - hintbits: rel 1663/16384/1249; blk 29
CONTEXT: writing block 29 of relation 1663/16384/1249
LOG: xlog flush request 0/67F0808; write 0/67F0708; flush 0/67F0708
CONTEXT: writing block 29 of relation 1663/16384/1249
LOG: INSERT @ 0/67F0808: prev 0/67F0708; xid 0: XLOG - checkpoint: redo 0/67F05F0; tli 1; xid 0/9093; oid 90132; multi 1; offset 0; shutdown
LOG: xlog flush request 0/67F0850; write 0/67F0808; flush 0/67F0808
PANIC: concurrent transaction log activity while database system is shutting down
LOG: background writer process (PID 17411) was terminated by signal 6: Aborted

I am completely at a loss what to do here.

--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

Alvaro Herrera wrote:

> The "other hint bits" are:
>
> - LP_DEAD as used by the various callers of ItemIdMarkDead.
> - PD_PAGE_FULL
> - BTPageOpaque->btpo_flags and btpo_cycleid
>
> All of them are changed with only SetBufferCommitInfoNeedsSave being
> called afterwards.

I think we could get away with WAL-logging LP_DEAD via ItemIdMarkDead
similar to what is done to SetHintBits in the posted patch, and cope
with the rest by marking the page with the invalid checksum; they are
not so frequent anyway so the reliability loss is low.

--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

Alvaro Herrera <alvherre(at)commandprompt(dot)com> writes:

> Alvaro Herrera wrote:
>
>> The "other hint bits" are:
>>
>> - LP_DEAD as used by the various callers of ItemIdMarkDead.
>> - PD_PAGE_FULL
>> - BTPageOpaque->btpo_flags and btpo_cycleid
>>
>> All of them are changed with only SetBufferCommitInfoNeedsSave being
>> called afterwards.
>
> I think we could get away with WAL-logging LP_DEAD via ItemIdMarkDead
> similar to what is done to SetHintBits in the posted patch, and cope
> with the rest by marking the page with the invalid checksum; they are
> not so frequent anyway so the reliability loss is low.

If PD_PAGE_FULL is set and that causes the crc to be set to the invalid sum
will we ever get another chance to set it?

--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com
Ask me about EnterpriseDB's RemoteDBA services!

Alvaro Herrera napsal(a):
> Simon Riggs wrote:
>
>> But perhaps writing a single WAL record if you scan whole page and set
>> all bits at once. Then it makes sense in some cases.
>
> So this is what I ended up doing; attached.
>
> There are some gotchas in this patch:
>

Please, DO NOT MOVE position of page version in PageHeader structure! And
PG_PAGE_LAYOUT_VERSION should be bump to 5.

Thanks Zdenek

--
Zdenek Kotala Sun Microsystems
Prague, Czech Republic http://sun.com/postgresql

On Thu, Oct 30, 2008 at 10:33 AM, Zdenek Kotala <Zdenek(dot)Kotala(at)sun(dot)com> wrote:
> Please, DO NOT MOVE position of page version in PageHeader structure! And
> PG_PAGE_LAYOUT_VERSION should be bump to 5.

Umm, any in-place upgrade should be capable of handling changes to the
page header. Of, did I miss something significant in the in-place
upgrade design?

--
Jonah H. Harris, Senior DBA
myYearbook.com

Jonah H. Harris wrote:
> On Thu, Oct 30, 2008 at 10:33 AM, Zdenek Kotala <Zdenek(dot)Kotala(at)sun(dot)com> wrote:
>> Please, DO NOT MOVE position of page version in PageHeader structure! And
>> PG_PAGE_LAYOUT_VERSION should be bump to 5.
>
> Umm, any in-place upgrade should be capable of handling changes to the
> page header. Of, did I miss something significant in the in-place

I thought that was kind of the point of in place upgrade.

Joshua D. Drake

Zdenek Kotala wrote:
> Alvaro Herrera napsal(a):
>> Simon Riggs wrote:
>>
>>> But perhaps writing a single WAL record if you scan whole page and set
>>> all bits at once. Then it makes sense in some cases.
>>
>> So this is what I ended up doing; attached.
>
> Please, DO NOT MOVE position of page version in PageHeader structure!

Hmm. The only way I see we could do that is to modify the checksum
struct member to a predefined value before calculating the page's
checksum.

Ah, actually there's another alternative -- leave the checksum on its
current position (start of struct) and move other members below
pg_pagesize_version (leaning towards pd_tli and pd_flags). That'd leave
the page version in the same position.

(Hmm, maybe it's better to move pd_lower and pd_upper?)

> And PG_PAGE_LAYOUT_VERSION should be bump to 5.

Easily done; thanks for the reminder.

--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

"Joshua D. Drake" <jd(at)commandprompt(dot)com> writes:

> Jonah H. Harris wrote:
>> On Thu, Oct 30, 2008 at 10:33 AM, Zdenek Kotala <Zdenek(dot)Kotala(at)sun(dot)com> wrote:
>>> Please, DO NOT MOVE position of page version in PageHeader structure! And
>>> PG_PAGE_LAYOUT_VERSION should be bump to 5.
>>
>> Umm, any in-place upgrade should be capable of handling changes to the
>> page header. Of, did I miss something significant in the in-place
>
> I thought that was kind of the point of in place upgrade.

Sure, but he has to have a reliable way to tell what version of the page
header he's looking at...

What I'm wondering though -- are we going to make CRCs mandatory? Or set aside
the 4 bytes even if you're not using them? Because if the size of the page
header varies depending on whether you're using CRCs that sounds like it would
be quite a pain.

--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com
Ask me about EnterpriseDB's 24x7 Postgres support!

"Jonah H. Harris" <jonah(dot)harris(at)gmail(dot)com> writes:
> On Thu, Oct 30, 2008 at 10:33 AM, Zdenek Kotala <Zdenek(dot)Kotala(at)sun(dot)com> wrote:
>> Please, DO NOT MOVE position of page version in PageHeader structure! And
>> PG_PAGE_LAYOUT_VERSION should be bump to 5.

> Umm, any in-place upgrade should be capable of handling changes to the
> page header.

Well, yeah, but it has to be able to tell which version it's dealing
with. I quite agree with Zdenek that keeping the version indicator
in a fixed location is appropriate.

regards, tom lane

On Thu, Oct 30, 2008 at 11:14 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> "Jonah H. Harris" <jonah(dot)harris(at)gmail(dot)com> writes:
>> On Thu, Oct 30, 2008 at 10:33 AM, Zdenek Kotala <Zdenek(dot)Kotala(at)sun(dot)com> wrote:
>>> Please, DO NOT MOVE position of page version in PageHeader structure! And
>>> PG_PAGE_LAYOUT_VERSION should be bump to 5.
>
>> Umm, any in-place upgrade should be capable of handling changes to the
>> page header.
>
> Well, yeah, but it has to be able to tell which version it's dealing
> with. I quite agree with Zdenek that keeping the version indicator
> in a fixed location is appropriate.

Most of the other databases I've worked, which don't have different
types of pages, put the page version as the first element of the page.
That would let us put the crc right after it. Thoughts?

--
Jonah H. Harris, Senior DBA
myYearbook.com

Gregory Stark wrote:
> Alvaro Herrera <alvherre(at)commandprompt(dot)com> writes:
>
> > Alvaro Herrera wrote:
> >
> >> The "other hint bits" are:
> >>
> >> - LP_DEAD as used by the various callers of ItemIdMarkDead.
> >> - PD_PAGE_FULL
> >> - BTPageOpaque->btpo_flags and btpo_cycleid
> >>
> >> All of them are changed with only SetBufferCommitInfoNeedsSave being
> >> called afterwards.
> >
> > I think we could get away with WAL-logging LP_DEAD via ItemIdMarkDead
> > similar to what is done to SetHintBits in the posted patch, and cope
> > with the rest by marking the page with the invalid checksum; they are
> > not so frequent anyway so the reliability loss is low.
>
> If PD_PAGE_FULL is set and that causes the crc to be set to the invalid sum
> will we ever get another chance to set it?

I should have qualified that a bit more. It's not setting PD_FULL
that's not logged, but clearing it (heap_prune_page, line 282). It's
set in heap_update.

Hmm, oh I see another problem here -- the bit is not restored when
replayed heap_update's WAL record. I'm now wondering what other bits
are set without much care about correctly restoring them during replay.

--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

"Jonah H. Harris" <jonah(dot)harris(at)gmail(dot)com> writes:
> On Thu, Oct 30, 2008 at 11:14 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> Well, yeah, but it has to be able to tell which version it's dealing
>> with. I quite agree with Zdenek that keeping the version indicator
>> in a fixed location is appropriate.

> Most of the other databases I've worked, which don't have different
> types of pages, put the page version as the first element of the page.
> That would let us put the crc right after it. Thoughts?

"Fixed location" does not mean "let's move it".

regards, tom lane

Jonah H. Harris napsal(a):
> On Thu, Oct 30, 2008 at 10:33 AM, Zdenek Kotala <Zdenek(dot)Kotala(at)sun(dot)com> wrote:
>> Please, DO NOT MOVE position of page version in PageHeader structure! And
>> PG_PAGE_LAYOUT_VERSION should be bump to 5.
>
> Umm, any in-place upgrade should be capable of handling changes to the
> page header. Of, did I miss something significant in the in-place
> upgrade design?

Not any change. If you move page header version field to another position it
will require kind of magic to detect what version it is. Other field you can
place everywhere :-), but do not touch page version. It will brings a lot of
problems...

Zdenek

--
Zdenek Kotala Sun Microsystems
Prague, Czech Republic http://sun.com/postgresql

On Thu, Oct 30, 2008 at 11:27 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> "Jonah H. Harris" <jonah(dot)harris(at)gmail(dot)com> writes:
>> On Thu, Oct 30, 2008 at 11:14 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>> Well, yeah, but it has to be able to tell which version it's dealing
>>> with. I quite agree with Zdenek that keeping the version indicator
>>> in a fixed location is appropriate.
>
>> Most of the other databases I've worked, which don't have different
>> types of pages, put the page version as the first element of the page.
>> That would let us put the crc right after it. Thoughts?
>
> "Fixed location" does not mean "let's move it".

Just trying to be helpful. Just thought I might give some insight as
to what others, who had implemented in-place upgrade functionality
years before Postgres' existence, had done.

--
Jonah H. Harris, Senior DBA
myYearbook.com

Alvaro Herrera <alvherre(at)commandprompt(dot)com> writes:

> Ah, actually there's another alternative -- leave the checksum on its
> current position (start of struct) and move other members below
> pg_pagesize_version (leaning towards pd_tli and pd_flags). That'd leave
> the page version in the same position.

I don't understand why the position of anything matters here. Look at TCP
packets for instance, the checksum is not at the beginning or end of anything.

The CRC is chosen such that if you CRC the resulting packet including the CRC
you get a CRC of 0. That can be done for whatever offset the CRC appears at I
believe.

--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com
Ask me about EnterpriseDB's 24x7 Postgres support!

Alvaro Herrera napsal(a):
> Zdenek Kotala wrote:
>> Alvaro Herrera napsal(a):
>>> Simon Riggs wrote:
>>>
>>>> But perhaps writing a single WAL record if you scan whole page and set
>>>> all bits at once. Then it makes sense in some cases.
>>> So this is what I ended up doing; attached.
>> Please, DO NOT MOVE position of page version in PageHeader structure!
>
> Hmm. The only way I see we could do that is to modify the checksum
> struct member to a predefined value before calculating the page's
> checksum.
>
> Ah, actually there's another alternative -- leave the checksum on its
> current position (start of struct) and move other members below
> pg_pagesize_version (leaning towards pd_tli and pd_flags). That'd leave
> the page version in the same position.
>
> (Hmm, maybe it's better to move pd_lower and pd_upper?)

No, please, keep pd_lower and pd_upper on same position. They are accessed more
often than pd_tli and pd_flags. It is better for optimization.

By the way, do you need CRC as a first page member? Is it for future development
like CLOG integration into buffers? Why not put it on the end as and mark it as
a special? It will reduce space requirement when CRC is not enabled.

Zdenek

--
Zdenek Kotala Sun Microsystems
Prague, Czech Republic http://sun.com/postgresql

Zdenek Kotala <Zdenek(dot)Kotala(at)Sun(dot)COM> writes:
> By the way, do you need CRC as a first page member? Is it for future development
> like CLOG integration into buffers? Why not put it on the end as and mark it as
> a special? It will reduce space requirement when CRC is not enabled.

... and make life tremendously more complex for indexes, plus turning
CRC checking on or off on-the-fly would be problematic. I think Alvaro
has the right idea: just put the field there all the time.

regards, tom lane