Online enabling of checksums

Re-sending this one with proper formatting. Apologies for the horrible
gmail-screws-up-the-text-part of the last one!

No change to patch or text, just the formatting.

//Magnus

Once more, here is an attempt to solve the problem of on-line enabling of
checksums that me and Daniel have been hacking on for a bit. See for
example
/messages/by-id/CABUevEx8KWhZE_XkZQpzEkZypZmBp3GbM9W90JLp=-7OJWBbcg@mail.gmail.com
and
/messages/by-id/FF393672-5608-46D6-9224-6620EC532693@endpoint.com
for some previous discussions.

Base design:

Change the checksum flag to instead of on and off be an enum.
off/inprogress/on. When checksums are off and on, they work like today.
When checksums are in progress, checksums are *written* but not verified.
State can go from “off” to “inprogress”, from “inprogress” to either “on”
or “off”, or from “on” to “off”.

Two new functions are added, pg_enable_data_checksums() and
pg_disable_data_checksums(). The disable one is easy -- it just changes to
disable. The enable one will change the state to inprogress, and then start
a background worker (the “checksumhelper launcher”). This worker in turn
will start one sub-worker (“checksumhelper worker”) in each database
(currently all done sequentially). This worker will enumerate all
tables/indexes/etc in the database and validate their checksums. If there
is no checksum, or the checksum is incorrect, it will compute a new
checksum and write it out. When all databases have been processed, the
checksum state changes to “on” and the launcher shuts down. At this point,
the cluster has checksums enabled as if it was initdb’d with checksums
turned on.

If the cluster shuts down while “inprogress”, the DBA will have to manually
either restart the worker (by calling pg_enable_checksums()) or turn
checksums off again. Checksums “in progress” only carries a cost and no
benefit.

The change of the checksum state is WAL logged with a new xlog record. All
the buffers written by the background worker are forcibly enabled full page
writes to make sure the checksum is fully updated on the standby even if no
actual contents of the buffer changed.

We’ve also included a small commandline tool, bin/pg_verify_checksums, that
can be run against an offline cluster to validate all checksums. Future
improvements includes being able to use the background worker/launcher to
perform an online check as well. Being able to run more parallel workers in
the checksumhelper might also be of interest.

The patch includes two sets of tests, an isolation test turning on
checksums while one session is writing to the cluster and another is
continuously reading, to simulate turning on checksums in a production
database. There is also a TAP test which enables checksums with streaming
replication turned on to test the new xlog record. The isolation test ran
into the 1024 character limit of the isolation test lexer, with a separate
patch and discussion at
/messages/by-id/8D628BE4-6606-4FF6-A3FF-8B2B0E9B43D0@yesql.se

On 2/21/18 15:53, Magnus Hagander wrote:

*Two new functions are added, pg_enable_data_checksums() and
pg_disable_data_checksums(). The disable one is easy -- it just changes
to disable. The enable one will change the state to inprogress, and then
start a background worker (the “checksumhelper launcher”). This worker
in turn will start one sub-worker (“checksumhelper worker”) in each
database (currently all done sequentially).*

This is at least the fourth version of the pattern launcher plus worker
background workers. I wonder whether we can do something to make this
easier and less repetitive. Not in this patch, of course.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Hello, Magnus, Peter!

I'm excited that this feature emerged, thanks for the patch. Hope it will help to fix some mistakes made during initdb long time ago...

22 февр. 2018 г., в 18:22, Peter Eisentraut <peter.eisentraut@2ndquadrant.com> написал(а):

On 2/21/18 15:53, Magnus Hagander wrote:

*Two new functions are added, pg_enable_data_checksums() and
pg_disable_data_checksums(). The disable one is easy -- it just changes
to disable. The enable one will change the state to inprogress, and then
start a background worker (the “checksumhelper launcher”). This worker
in turn will start one sub-worker (“checksumhelper worker”) in each
database (currently all done sequentially).*

This is at least the fourth version of the pattern launcher plus worker
background workers. I wonder whether we can do something to make this
easier and less repetitive. Not in this patch, of course.

Peter, can I ask for some pointers in searching for previous versions?
I want to review patch this patch and some code comparision could be handy....

So far I've found only this [0,1] (without code) and threads mentioned by Magnus [2,3]

Or do you mean extracting "worker+lancher" for reuse for other purposes?

Best regards, Andrey Borodin.

[0]: /messages/by-id/E2B195BF-7AA1-47AF-85BE-0E936D157902@endpoint.com </messages/by-id/E2B195BF-7AA1-47AF-85BE-0E936D157902@endpoint.com
[1]: /messages/by-id/7A00D9D1-535A-4C37-94C7-02296AAF063F@endpoint.com </messages/by-id/7A00D9D1-535A-4C37-94C7-02296AAF063F@endpoint.com
[2]: /messages/by-id/CABUevEx8KWhZE_XkZQpzEkZypZmBp3GbM9W90JLp=-7OJWBbcg@mail.gmail.com </messages/by-id/CABUevEx8KWhZE_XkZQpzEkZypZmBp3GbM9W90JLp=-7OJWBbcg@mail.gmail.com
[3]: /messages/by-id/FF393672-5608-46D6-9224-6620EC532693@endpoint.com

On Thu, Feb 22, 2018 at 4:47 PM, Andrey Borodin <x4mmm@yandex-team.ru>
wrote:

Hello, Magnus, Peter!

I'm excited that this feature emerged, thanks for the patch. Hope it will
help to fix some mistakes made during initdb long time ago...

22 февр. 2018 г., в 18:22, Peter Eisentraut <peter.eisentraut@2ndquadrant.
com> написал(а):

On 2/21/18 15:53, Magnus Hagander wrote:

*Two new functions are added, pg_enable_data_checksums() and
pg_disable_data_checksums(). The disable one is easy -- it just changes
to disable. The enable one will change the state to inprogress, and then
start a background worker (the “checksumhelper launcher”). This worker
in turn will start one sub-worker (“checksumhelper worker”) in each
database (currently all done sequentially).*

This is at least the fourth version of the pattern launcher plus worker
background workers. I wonder whether we can do something to make this
easier and less repetitive. Not in this patch, of course.

Peter, can I ask for some pointers in searching for previous versions?
I want to review patch this patch and some code comparision could be
handy....

So far I've found only this [0,1] (without code) and threads mentioned by
Magnus [2,3]

Or do you mean extracting "worker+lancher" for reuse for other purposes?

I'm pretty sure Peter means the second. Which could be interesting, but as
he says, not the topic for this patch.

I'm not entirely sure which the others ones are. Auto-Vacuum obviously is
one, which doesn't use the worker infrastructure. But I'm not sure which
the others are referring to?

--
Magnus Hagander
Me: https://www.hagander.net/ <http://www.hagander.net/&gt;
Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/&gt;

On 2018-02-22 08:22:48 -0500, Peter Eisentraut wrote:

On 2/21/18 15:53, Magnus Hagander wrote:

*Two new functions are added, pg_enable_data_checksums() and
pg_disable_data_checksums(). The disable one is easy -- it just changes
to disable. The enable one will change the state to inprogress, and then
start a background worker (the “checksumhelper launcher”). This worker
in turn will start one sub-worker (“checksumhelper worker”) in each
database (currently all done sequentially).*

This is at least the fourth version of the pattern launcher plus worker
background workers. I wonder whether we can do something to make this
easier and less repetitive. Not in this patch, of course.

I suspect I'm going to get some grief for this, but I think the time has
come to bite the bullet and support changing databases in the same
process...

Greetings,

Andres Freund

On Thu, Feb 22, 2018 at 8:24 PM, Andres Freund <andres@anarazel.de> wrote:

On 2018-02-22 08:22:48 -0500, Peter Eisentraut wrote:

On 2/21/18 15:53, Magnus Hagander wrote:

*Two new functions are added, pg_enable_data_checksums() and
pg_disable_data_checksums(). The disable one is easy -- it just changes
to disable. The enable one will change the state to inprogress, and

then

start a background worker (the “checksumhelper launcher”). This worker
in turn will start one sub-worker (“checksumhelper worker”) in each
database (currently all done sequentially).*

This is at least the fourth version of the pattern launcher plus worker
background workers. I wonder whether we can do something to make this
easier and less repetitive. Not in this patch, of course.

I suspect I'm going to get some grief for this, but I think the time has
come to bite the bullet and support changing databases in the same
process...

Hey, I can't even see the goalposts anymore :P

Are you saying this should be done *in general*, or specifically for
background workers? I'm assuming you mean the general case? That would be
very useful, but is probably a fairly non-trivial task (TM).

--
Magnus Hagander
Me: https://www.hagander.net/ <http://www.hagander.net/&gt;
Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/&gt;

Hi,

On 2018-02-22 20:30:52 +0100, Magnus Hagander wrote:

On Thu, Feb 22, 2018 at 8:24 PM, Andres Freund <andres@anarazel.de> wrote:

I suspect I'm going to get some grief for this, but I think the time has
come to bite the bullet and support changing databases in the same
process...

Hey, I can't even see the goalposts anymore :P

Hah. I vote for making this a hard requirement :P

Are you saying this should be done *in general*, or specifically for
background workers? I'm assuming you mean the general case?

I'd say bgworkers first. It's a lot clearer how to exactly do it
there. Refactoring the mainloop handling in PostgresMain() would be a
bigger task.

That would be very useful, but is probably a fairly non-trivial task
(TM).

I'm not actually that sure it is. We have nearly all the code, I
think. Syscache inval, ProcKill(), and then you're nearly ready to do
the normal connection dance again.

Greetings,

Andres Freund

On Thu, Feb 22, 2018 at 8:41 PM, Andres Freund <andres@anarazel.de> wrote:

On 2018-02-22 20:30:52 +0100, Magnus Hagander wrote:

On Thu, Feb 22, 2018 at 8:24 PM, Andres Freund <andres@anarazel.de>

wrote:

I suspect I'm going to get some grief for this, but I think the time

has

come to bite the bullet and support changing databases in the same
process...

Hey, I can't even see the goalposts anymore :P

Hah. I vote for making this a hard requirement :P

Hah! Are you handing out binoculars? :)

Are you saying this should be done *in general*, or specifically for
background workers? I'm assuming you mean the general case?

I'd say bgworkers first. It's a lot clearer how to exactly do it
there. Refactoring the mainloop handling in PostgresMain() would be a
bigger task.

Yeah, it'd probably be easier. I don't know exactly what it'd involve but
clearly less.

In this particular case that would at least phase 1 simplify it because
we'd only need one process instead of worker/launcher. However, if we'd
ever want to parallellize it -- or any other process of the style, like
autovacuum -- you'd still need a launcher+worker combo. So making that
particular scenario simpler might be worthwhile on it's own.

That would be very useful, but is probably a fairly non-trivial task
(TM).

I'm not actually that sure it is. We have nearly all the code, I
think. Syscache inval, ProcKill(), and then you're nearly ready to do
the normal connection dance again.

I'll take your word for it :) I haven't dug into that part.

--
Magnus Hagander
Me: https://www.hagander.net/ <http://www.hagander.net/&gt;
Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/&gt;

On February 22, 2018 11:44:17 AM PST, Magnus Hagander <magnus@hagander.net> wrote:

On Thu, Feb 22, 2018 at 8:41 PM, Andres Freund <andres@anarazel.de>
wrote:
In this particular case that would at least phase 1 simplify it because
we'd only need one process instead of worker/launcher. However, if we'd
ever want to parallellize it -- or any other process of the style, like
autovacuum -- you'd still need a launcher+worker combo. So making that
particular scenario simpler might be worthwhile on it's own.

Why is that needed? You can just start two bgworkers and process a list of items stored in shared memory. Or even just check, I assume there'd be a catalog flag somewhere, whether a database / table / object of granularity has already been processed and use locking to prevent concurrent access.

Andres
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.

On 2/22/18 12:38, Magnus Hagander wrote:

I'm not entirely sure which the others ones are. Auto-Vacuum obviously
is one, which doesn't use the worker infrastructure. But I'm not sure
which the others are referring to? 

autovacuum, subscription workers, auto prewarm

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

On Thu, Feb 22, 2018 at 8:52 PM, Andres Freund <andres@anarazel.de> wrote:

On February 22, 2018 11:44:17 AM PST, Magnus Hagander <magnus@hagander.net>
wrote:

On Thu, Feb 22, 2018 at 8:41 PM, Andres Freund <andres@anarazel.de>
wrote:
In this particular case that would at least phase 1 simplify it because
we'd only need one process instead of worker/launcher. However, if we'd
ever want to parallellize it -- or any other process of the style, like
autovacuum -- you'd still need a launcher+worker combo. So making that
particular scenario simpler might be worthwhile on it's own.

Why is that needed? You can just start two bgworkers and process a list of
items stored in shared memory. Or even just check, I assume there'd be a
catalog flag somewhere, whether a database / table / object of granularity
has already been processed and use locking to prevent concurrent access.

You could do that, but then you've moving the complexity to managing that
list in shared memory instead. I'm not sure that's any easier... And
certainly adding a catalog flag for a usecase like this one is not making
it easier.

--
Magnus Hagander
Me: https://www.hagander.net/ <http://www.hagander.net/&gt;
Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/&gt;

Hi,

On 2018-02-22 21:16:02 +0100, Magnus Hagander wrote:

You could do that, but then you've moving the complexity to managing that
list in shared memory instead.

Maybe I'm missing something, but how are you going to get quick parallel
processing if you don't have a shmem piece? You can't assign one
database per worker because commonly there's only one database. You
don't want to start/stop a worker for each relation because that'd be
extremely slow for databases with a lot of tables. Without shmem you
can't pass more than an oid to a bgworker. To me the combination of
these things imply that you need some other synchronization mechanism
*anyway*.

I'm not sure that's any easier... And
certainly adding a catalog flag for a usecase like this one is not making
it easier.

Hm, I imagined you'd need that anyway. Imagine a 10TB database that's
online converted to checksums. I assume you'd not want to reread 9TB if
you crash after processing most of the cluster already?

Regards,

Andres Freund

On Thu, Feb 22, 2018 at 9:23 PM, Andres Freund <andres@anarazel.de> wrote:

Hi,

On 2018-02-22 21:16:02 +0100, Magnus Hagander wrote:

You could do that, but then you've moving the complexity to managing that
list in shared memory instead.

Maybe I'm missing something, but how are you going to get quick parallel
processing if you don't have a shmem piece? You can't assign one
database per worker because commonly there's only one database. You
don't want to start/stop a worker for each relation because that'd be
extremely slow for databases with a lot of tables. Without shmem you
can't pass more than an oid to a bgworker. To me the combination of
these things imply that you need some other synchronization mechanism
*anyway*.

Yes, you probably need something like that if you want to be able to
parallelize on things inside each database. If you are OK parallelizing
things on a per-database level, you don't need it.

I'm not sure that's any easier... And
certainly adding a catalog flag for a usecase like this one is not making
it easier.

Hm, I imagined you'd need that anyway. Imagine a 10TB database that's
online converted to checksums. I assume you'd not want to reread 9TB if
you crash after processing most of the cluster already?

I would prefer that yes. But having to re-read 9TB is still significantly
better than not being able to turn on checksums at all (state today). And
adding a catalog column for it will carry the cost of the migration
*forever*, both for clusters that never have checksums and those that had
it from the beginning.

Accepting that the process will start over (but only read, not re-write,
the blocks that have already been processed) in case of a crash does
significantly simplify the process, and reduce the long-term cost of it in
the form of entries in the catalogs. Since this is a on-time operation (or
for many people, a zero-time operation), paying that cost that one time is
probably better than paying a much smaller cost but constantly.

--
Magnus Hagander
Me: https://www.hagander.net/ <http://www.hagander.net/&gt;
Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/&gt;

On Thu, Feb 22, 2018 at 11:24:37AM -0800, Andres Freund wrote:

I suspect I'm going to get some grief for this, but I think the time has
come to bite the bullet and support changing databases in the same
process...

I'd like to see that. Last time this has been discussed, and Robert
complained to me immediately when I suggested it, is that this is not
worth it with the many complications around syscache handling and
resource cleanup. It is in the long term more stable to use a model
where a parent process handles a set of children and decides to which
database each child should spawn, which is what autovacuum does.
--
Michael

How does:

On 2018-02-23 11:48:16 +0900, Michael Paquier wrote:

On Thu, Feb 22, 2018 at 11:24:37AM -0800, Andres Freund wrote:

I suspect I'm going to get some grief for this, but I think the time has
come to bite the bullet and support changing databases in the same
process...

I'd like to see that. Last time this has been discussed, and Robert
complained to me immediately when I suggested it, is that this is not
worth it with the many complications around syscache handling and
resource cleanup.

relate to:

It is in the long term more stable to use a model
where a parent process handles a set of children and decides to which
database each child should spawn, which is what autovacuum does.

?

On Thu, Feb 22, 2018 at 9:48 PM, Michael Paquier <michael@paquier.xyz> wrote:

On Thu, Feb 22, 2018 at 11:24:37AM -0800, Andres Freund wrote:

I suspect I'm going to get some grief for this, but I think the time has
come to bite the bullet and support changing databases in the same
process...

I'd like to see that. Last time this has been discussed, and Robert
complained to me immediately when I suggested it, is that this is not
worth it with the many complications around syscache handling and
resource cleanup. It is in the long term more stable to use a model
where a parent process handles a set of children and decides to which
database each child should spawn, which is what autovacuum does.

My position is that allowing processes to change databases is a good
idea but (1) it will probably take some work to get correct and (2) it
probably won't be super-fast due to the need to flush absolutely every
bit of state in sight that might've been influenced by the choice of
database.

I also agree with Andres that this email is not very easy to
understand, although my complaint is not so much that I don't see how
the parts relate as that you seem to be contradicting yourself.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On Thu, Feb 22, 2018 at 9:09 PM, Peter Eisentraut <
peter.eisentraut@2ndquadrant.com> wrote:

On 2/22/18 12:38, Magnus Hagander wrote:

I'm not entirely sure which the others ones are. Auto-Vacuum obviously
is one, which doesn't use the worker infrastructure. But I'm not sure
which the others are referring to?

autovacuum, subscription workers, auto prewarm

Oh, for some reason I thought you were thinking in pending patches. Yeah,
for those it makes sense -- though autovacuum isn't (currently) using
background workers for what it does, the rest certainly makes sense to do
something with.

But as you say, that's a separate patch :)

--
Magnus Hagander
Me: https://www.hagander.net/ <http://www.hagander.net/&gt;
Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/&gt;

On Thu, Feb 22, 2018 at 3:28 PM, Magnus Hagander <magnus@hagander.net> wrote:

I would prefer that yes. But having to re-read 9TB is still significantly
better than not being able to turn on checksums at all (state today). And
adding a catalog column for it will carry the cost of the migration
*forever*, both for clusters that never have checksums and those that had it
from the beginning.

Accepting that the process will start over (but only read, not re-write, the
blocks that have already been processed) in case of a crash does
significantly simplify the process, and reduce the long-term cost of it in
the form of entries in the catalogs. Since this is a on-time operation (or
for many people, a zero-time operation), paying that cost that one time is
probably better than paying a much smaller cost but constantly.

That's not totally illogical, but to be honest I'm kinda surprised
that you're approaching it that way. I would have thought that
relchecksums and datchecksums columns would have been a sort of
automatic design choice for this feature. The thing to keep in mind
is that nobody's going to notice the overhead of adding those columns
in practice, but someone will surely notice the pain that comes from
having to restart the whole operation. You're talking about trading
an effectively invisible overhead for a very noticeable operational
problem.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On 02/24/2018 01:34 AM, Robert Haas wrote:

On Thu, Feb 22, 2018 at 3:28 PM, Magnus Hagander <magnus@hagander.net> wrote:

I would prefer that yes. But having to re-read 9TB is still significantly
better than not being able to turn on checksums at all (state today). And
adding a catalog column for it will carry the cost of the migration
*forever*, both for clusters that never have checksums and those that had it
from the beginning.

Accepting that the process will start over (but only read, not re-write, the
blocks that have already been processed) in case of a crash does
significantly simplify the process, and reduce the long-term cost of it in
the form of entries in the catalogs. Since this is a on-time operation (or
for many people, a zero-time operation), paying that cost that one time is
probably better than paying a much smaller cost but constantly.

That's not totally illogical, but to be honest I'm kinda surprised
that you're approaching it that way. I would have thought that
relchecksums and datchecksums columns would have been a sort of
automatic design choice for this feature. The thing to keep in mind
is that nobody's going to notice the overhead of adding those columns
in practice, but someone will surely notice the pain that comes from
having to restart the whole operation. You're talking about trading
an effectively invisible overhead for a very noticeable operational
problem.

I agree having to restart the whole operation after a crash is not
ideal, but I don't see how adding a flag actually solves it. The problem
is the large databases often store most of the data (>80%) in one or two
central tables (think fact tables in star schema, etc.). So if you
crash, it's likely half-way while processing this table, so the whole
table would still have relchecksums=false and would have to be processed
from scratch.

But perhaps you meant something like "position" instead of just a simple
true/false flag?

regards

--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services