Logging WAL when updating hintbit

On Thu, Nov 14, 2013 at 3:02 PM, Sawada Masahiko <sawada.mshk@gmail.com> wrote:

I attached patch adds new wal_level 'all'.
If wal_level is set 'all', the server logs WAL not only when wal_level
is set 'hot_standby' ,but also when updating hint bit.
That is, we will be able to know all of the changed block number by
reading the WALs.

Is 'all' a name really suited? It looks too general. I don't have a
name on top of my mind but what about something like full_pages or
something similar...

This wal_level is infrastructure for fast failback. (i.g., without fresh backup)
It need to cooperate with pg_rewind.

I am not sure that using as reason the possible interactions of a
contrib module not in core is a reason sufficient to justify the
presence of a new wal_level, and pg_rewind is still a young solution
that needs to be improved. So such a patch looks premature IMO, but
the idea is interesting and might cover many needs for external
projects.

Not only that, I think it will be profitable infrastructure for
differential backup.

Yep, agreed. This might help some projects in this area.

And it leads to improve performance at standby server side. Because
the standby server doesn't update hintbit by itself, but FPW is
replicated to standby server and applied.

It would be interesting to see some numbers here.

This is clearly a WIP patch so it does not matter now, but if you
submit it later on, be sure to add some comments in bufmgr.c as well
as documentation for the new option.
Regards,
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 11/14/2013 07:02 AM, Sawada Masahiko wrote:

I attached patch adds new wal_level 'all'.

Shouldn't this be a separate setting? It's useful for storage which
requires rewriting a partially written sector before it can be read again.

--
Florian Weimer / Red Hat Product Security Team

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Thu, Nov 14, 2013 at 3:53 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:

On Thu, Nov 14, 2013 at 3:02 PM, Sawada Masahiko <sawada.mshk@gmail.com> wrote:

I attached patch adds new wal_level 'all'.
If wal_level is set 'all', the server logs WAL not only when wal_level
is set 'hot_standby' ,but also when updating hint bit.
That is, we will be able to know all of the changed block number by
reading the WALs.

Is 'all' a name really suited? It looks too general. I don't have a
name on top of my mind but what about something like full_pages or
something similar...

Yes, I'm worried about name of value.
But 'full_pages' sounds good for me.

This wal_level is infrastructure for fast failback. (i.g., without fresh backup)
It need to cooperate with pg_rewind.

I am not sure that using as reason the possible interactions of a
contrib module not in core is a reason sufficient to justify the
presence of a new wal_level, and pg_rewind is still a young solution
that needs to be improved. So such a patch looks premature IMO, but
the idea is interesting and might cover many needs for external
projects.

Not only that, I think it will be profitable infrastructure for
differential backup.

Yep, agreed. This might help some projects in this area.

And it leads to improve performance at standby server side. Because
the standby server doesn't update hintbit by itself, but FPW is
replicated to standby server and applied.

It would be interesting to see some numbers here.

I think this patch provide several benefit for feature. The fast
failback with pg_rewind is one of them.
If I want to provide only the fast failback with pg_rewind, this patch
logs too much information.
Just logging changed block number is enough for that.

As you said pg_rewind is still a young solution. But It very cool and
flexible solution.
I'm going to improve pg_rewind actively.

This is clearly a WIP patch so it does not matter now, but if you
submit it later on, be sure to add some comments in bufmgr.c as well
as documentation for the new option.

Yes, will do.

--
Regards,

-------
Sawada Masahiko

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Thu, Nov 14, 2013 at 7:51 PM, Florian Weimer <fweimer@redhat.com> wrote:

On 11/14/2013 07:02 AM, Sawada Masahiko wrote:

I attached patch adds new wal_level 'all'.

Shouldn't this be a separate setting? It's useful for storage which
requires rewriting a partially written sector before it can be read again.

Thank you for comment.
Actually, I had thought to add separate parameter.
If so, this feature logs enough information with all of the wal_level
(e.g., minimal) ?
And I thought that relation between wal_lvel and new feature would be
confuse user.

Regards,

-------
Sawada Masahiko

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 11/14/13, 1:02 AM, Sawada Masahiko wrote:

I attached patch adds new wal_level 'all'.

Compiler warning:

pg_controldata.c: In function ‘wal_level_str’:
pg_controldata.c:72:2: warning: enumeration value ‘WAL_LEVEL_ALL’ not handled in switch [-Wswitch]

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Fri, Nov 15, 2013 at 11:33 PM, Peter Eisentraut <peter_e@gmx.net> wrote:

On 11/14/13, 1:02 AM, Sawada Masahiko wrote:

I attached patch adds new wal_level 'all'.

Compiler warning:

pg_controldata.c: In function ‘wal_level_str’:
pg_controldata.c:72:2: warning: enumeration value ‘WAL_LEVEL_ALL’ not handled in switch [-Wswitch]

Thank you for report!
I have fixed it.

--
Regards,

-------
Sawada Masahiko

(2013/11/15 19:27), Sawada Masahiko wrote:

On Thu, Nov 14, 2013 at 7:51 PM, Florian Weimer <fweimer@redhat.com> wrote:

On 11/14/2013 07:02 AM, Sawada Masahiko wrote:

I attached patch adds new wal_level 'all'.

Shouldn't this be a separate setting? It's useful for storage which
requires rewriting a partially written sector before it can be read again.

Thank you for comment.
Actually, I had thought to add separate parameter.

I think that he said that if you can proof that amount of WAL is almost same and
without less performance same as before, you might not need to separate
parameter in your patch.

Did you test about amount of WAL size in your patch?
I'd like to know it.

Regards,
--
Mitsumasa KONDO
NTT Open Source Software Center

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Thu, Nov 14, 2013 at 1:02 AM, Sawada Masahiko <sawada.mshk@gmail.com> wrote:

I attached patch adds new wal_level 'all'.
If wal_level is set 'all', the server logs WAL not only when wal_level
is set 'hot_standby' ,but also when updating hint bit.
That is, we will be able to know all of the changed block number by
reading the WALs.
This wal_level is infrastructure for fast failback. (i.g., without fresh backup)
It need to cooperate with pg_rewind.

This needs to be a separate parameter, not something that gets jammed
into wal_level.

I'm also not 100% sure we want it, but let's hear what others think.

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Tue, Nov 19, 2013 at 3:54 PM, KONDO Mitsumasa
<kondo.mitsumasa@lab.ntt.co.jp> wrote:

(2013/11/15 19:27), Sawada Masahiko wrote:

On Thu, Nov 14, 2013 at 7:51 PM, Florian Weimer <fweimer@redhat.com>
wrote:

On 11/14/2013 07:02 AM, Sawada Masahiko wrote:

I attached patch adds new wal_level 'all'.

Shouldn't this be a separate setting? It's useful for storage which
requires rewriting a partially written sector before it can be read
again.

Thank you for comment.
Actually, I had thought to add separate parameter.

I think that he said that if you can proof that amount of WAL is almost same
and
without less performance same as before, you might not need to separate
parameter in your patch.

Thanks!
I took it wrong.
I think that there are quite a few difference amount of WAL.

Did you test about amount of WAL size in your patch?

Not yet. I will do that.

Regards,

-------
Sawada Masahiko

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 19 November 2013 22:19, Sawada Masahiko Wrote

Thank you for comment.
Actually, I had thought to add separate parameter.

I think that he said that if you can proof that amount of WAL is
almost same and without less performance same as before, you might

not

need to separate parameter in your patch.

Thanks!
I took it wrong.
I think that there are quite a few difference amount of WAL.

Did you test about amount of WAL size in your patch?

Not yet. I will do that.

1. Patch applies cleanly to master HEAD.
2. No Compilation Warning.
3. It works as per the patch expectation.

Some Suggestion:
1. Add new WAL level ("all") in comment in postgresql.conf
wal_level = hot_standby # minimal, archive, or hot_standby

Performance Test Result:
Run with pgbench for 300 seconds

WAL level : hot_standby
WAL Size : 111BF8A8
TPS : 125

WAL level : all
WAL Size : 11DB5AF8
TPS : 122

* TPS is almost constant but WAL size is increased around 11M.

This is the first level of observation, I will continue to test few more scenarios including performance test on standby.

Regards,
Dilip Kumar

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Wed, Nov 20, 2013 at 9:19 PM, Dilip kumar <dilip.kumar@huawei.com> wrote:

On 19 November 2013 22:19, Sawada Masahiko Wrote

Thanks!
I took it wrong.
I think that there are quite a few difference amount of WAL.

Did you test about amount of WAL size in your patch?

Not yet. I will do that.

1. Patch applies cleanly to master HEAD.
2. No Compilation Warning.
3. It works as per the patch expectation.

Some Suggestion:
1. Add new WAL level ("all") in comment in postgresql.conf
wal_level = hot_standby # minimal, archive, or hot_standby

Thank you for reviewing the patch.
Yes, I will do that. And I'm going to implement documentation patch.

Performance Test Result:
Run with pgbench for 300 seconds

WAL level : hot_standby
WAL Size : 111BF8A8
TPS : 125

WAL level : all
WAL Size : 11DB5AF8
TPS : 122

* TPS is almost constant but WAL size is increased around 11M.

This is the first level of observation, I will continue to test few more scenarios including performance test on standby.

Thank you for performance testing.
According of test result, TPS of 'all' lower than 'hot_standby',but
WAL size is increased.
I think that it should be separate parameter.
And TPS on master side is is almost constant. so this patch might have
several benefit for performance
improvement on standby side if the result of performance test on
standby side is improved.

Regards,

-------
Sawada Masahiko

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 20 November 2013 22:12, Sawada Masahiko Wrote

1. Patch applies cleanly to master HEAD.
2. No Compilation Warning.
3. It works as per the patch expectation.

Some Suggestion:
1. Add new WAL level ("all") in comment in postgresql.conf
wal_level = hot_standby # minimal, archive,

or hot_standby

Thank you for reviewing the patch.
Yes, I will do that. And I'm going to implement documentation patch.

OK, once I get it, I will review the same.

Performance Test Result:
Run with pgbench for 300 seconds

WAL level : hot_standby
WAL Size : 111BF8A8
TPS : 125

WAL level : all
WAL Size : 11DB5AF8
TPS : 122

* TPS is almost constant but WAL size is increased around 11M.

This is the first level of observation, I will continue to test few

more scenarios including performance test on standby.

Thank you for performance testing.
According of test result, TPS of 'all' lower than 'hot_standby',but
WAL size is increased.
I think that it should be separate parameter.
And TPS on master side is is almost constant. so this patch might have
several benefit for performance improvement on standby side if the
result of performance test on standby side is improved.

[Performance test on standby:]

I have executed pgbench on master with WAL LEVEL "hot_stanby" and "all" option, and after that run pgbench on standby with "select-only" option.

WAL LEVEL (on master) : hot_standby
Select TPS (on standby) : 4098

WAL LEVEL (on master) : all
Select TPS (on standby) : 4115

Regards,
Dilip

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Thu, Nov 21, 2013 at 8:59 PM, Dilip kumar <dilip.kumar@huawei.com> wrote:

On 20 November 2013 22:12, Sawada Masahiko Wrote

1. Patch applies cleanly to master HEAD.
2. No Compilation Warning.
3. It works as per the patch expectation.

Some Suggestion:
1. Add new WAL level ("all") in comment in postgresql.conf
wal_level = hot_standby # minimal, archive,

or hot_standby

Thank you for reviewing the patch.
Yes, I will do that. And I'm going to implement documentation patch.

OK, once I get it, I will review the same.

Performance Test Result:
Run with pgbench for 300 seconds

WAL level : hot_standby
WAL Size : 111BF8A8
TPS : 125

WAL level : all
WAL Size : 11DB5AF8
TPS : 122

* TPS is almost constant but WAL size is increased around 11M.

This is the first level of observation, I will continue to test few

more scenarios including performance test on standby.

Thank you for performance testing.
According of test result, TPS of 'all' lower than 'hot_standby',but
WAL size is increased.
I think that it should be separate parameter.
And TPS on master side is is almost constant. so this patch might have
several benefit for performance improvement on standby side if the
result of performance test on standby side is improved.

[Performance test on standby:]

I have executed pgbench on master with WAL LEVEL "hot_stanby" and "all" option, and after that run pgbench on standby with "select-only" option.

WAL LEVEL (on master) : hot_standby
Select TPS (on standby) : 4098

WAL LEVEL (on master) : all
Select TPS (on standby) : 4115

Thank you for testing!

It seems to have a little benefit for performance improvement on standby side.
It need to more test to write such thing into the documentation patch.

And I'm going to implement the patch as separated parameter now.
I think that this parameter should allow to use something together
with 'archive' and 'hot_standby'.
IWO not allow with 'minimal'.
Thought?

Regards,

-------
Sawada Masahiko

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Tue, 2013-11-19 at 11:42 -0500, Robert Haas wrote:

On Thu, Nov 14, 2013 at 1:02 AM, Sawada Masahiko <sawada.mshk@gmail.com> wrote:

I attached patch adds new wal_level 'all'.
If wal_level is set 'all', the server logs WAL not only when wal_level
is set 'hot_standby' ,but also when updating hint bit.
That is, we will be able to know all of the changed block number by
reading the WALs.
This wal_level is infrastructure for fast failback. (i.g., without fresh backup)
It need to cooperate with pg_rewind.

I'm also not 100% sure we want it, but let's hear what others think.

My take is that configuration options should be used to serve different
use cases. One thing I like about postgres is that it doesn't have
options for the sake of options.

The trade-off here seems to be: if you want fast failback, you have to
write more WAL during normal operation. But it's not clear to me which
one I should choose for a given installation. If there's a lot of data,
then fast failback is nice, but then again, logging the hint bits on a
large amount of data might be painful during normal operation. The only
time the choice is easy is when you already have checksums enabled.

I think we should work some more in this area first so we can end up
with something that works for everyone; or at least a more clear choice
to offer users. My hope is that it will go something like:

1. We get more reports from the field about checksum performance
2. pg_rewind gets some more attention
3. we find a way to upgrade a replica set using pg_upgrade and pg_rewind
so that the replicas do not all need a new basebackup after an upgrade
4. We further mitigate the performance impact of logging all page
modifications
5. We eventually see that the benefits of logging all page modifications
outweigh the performance cost for most people, and start recommending to
most people
6. The other WAL levels become more specialized for single, unreplicated
instances

That's just a hope, of course, but we've made some progress and I think
it's a plausible outcome. As it stands, the replica re-sync after any
failover or upgrade seems to be a design gap.

All that being said, I don't object to this option -- I just want it to
lead us somewhere. Not be another option that we keep around forever
with conflicting recommendations about how to set it.

Regards,
Jeff Davis

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Sun, Nov 24, 2013 at 6:02 AM, Jeff Davis <pgsql@j-davis.com> wrote:

On Tue, 2013-11-19 at 11:42 -0500, Robert Haas wrote:
My take is that configuration options should be used to serve different
use cases. One thing I like about postgres is that it doesn't have
options for the sake of options.

The trade-off here seems to be: if you want fast failback, you have to
write more WAL during normal operation. But it's not clear to me which
one I should choose for a given installation. If there's a lot of data,
then fast failback is nice, but then again, logging the hint bits on a
large amount of data might be painful during normal operation. The only
time the choice is easy is when you already have checksums enabled.

I think we should work some more in this area first so we can end up
with something that works for everyone; or at least a more clear choice
to offer users. My hope is that it will go something like:

1. We get more reports from the field about checksum performance
2. pg_rewind gets some more attention
3. we find a way to upgrade a replica set using pg_upgrade and pg_rewind
so that the replicas do not all need a new basebackup after an upgrade
4. We further mitigate the performance impact of logging all page
modifications
5. We eventually see that the benefits of logging all page modifications
outweigh the performance cost for most people, and start recommending to
most people
6. The other WAL levels become more specialized for single, unreplicated
instances

That's just a hope, of course, but we've made some progress and I think
it's a plausible outcome. As it stands, the replica re-sync after any
failover or upgrade seems to be a design gap.

All that being said, I don't object to this option -- I just want it to
lead us somewhere. Not be another option that we keep around forever
with conflicting recommendations about how to set it.

Thank you for feedback.

I agree with you.
We need to more report regarding checksum performance first. I will do that.

I attached the new version patch which adds separated parameter
'enable_logging_hintbit'.
It works same as previous patch, just independence from wal_level and
name is changed.
One changed behave is that it doesn't work together with 'minimal' wal_level.

Regards,

-------
Sawada Masahiko

On Mon, Nov 25, 2013 at 9:02 PM, Sawada Masahiko <sawada.mshk@gmail.com> wrote:

On Sun, Nov 24, 2013 at 6:02 AM, Jeff Davis <pgsql@j-davis.com> wrote:

On Tue, 2013-11-19 at 11:42 -0500, Robert Haas wrote:
My take is that configuration options should be used to serve different
use cases. One thing I like about postgres is that it doesn't have
options for the sake of options.

The trade-off here seems to be: if you want fast failback, you have to
write more WAL during normal operation. But it's not clear to me which
one I should choose for a given installation. If there's a lot of data,
then fast failback is nice, but then again, logging the hint bits on a
large amount of data might be painful during normal operation. The only
time the choice is easy is when you already have checksums enabled.

I think we should work some more in this area first so we can end up
with something that works for everyone; or at least a more clear choice
to offer users. My hope is that it will go something like:

1. We get more reports from the field about checksum performance
2. pg_rewind gets some more attention
3. we find a way to upgrade a replica set using pg_upgrade and pg_rewind
so that the replicas do not all need a new basebackup after an upgrade
4. We further mitigate the performance impact of logging all page
modifications
5. We eventually see that the benefits of logging all page modifications
outweigh the performance cost for most people, and start recommending to
most people
6. The other WAL levels become more specialized for single, unreplicated
instances

That's just a hope, of course, but we've made some progress and I think
it's a plausible outcome. As it stands, the replica re-sync after any
failover or upgrade seems to be a design gap.

All that being said, I don't object to this option -- I just want it to
lead us somewhere. Not be another option that we keep around forever
with conflicting recommendations about how to set it.

Thank you for feedback.

I agree with you.
We need to more report regarding checksum performance first. I will do that.

I did performance test on my environment.
Performance test result:

pgbench -T 600

Plane postgres : 460
Plane postgres with checksum : 450
Logging hintbit : 456

There is not huge performance degradation between three cases.
I think that It is better point that user can change the values
without creating database cluster newly.

Regards,

-------
Sawada Masahiko

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 11/25/13, 7:02 AM, Sawada Masahiko wrote:

I attached the new version patch which adds separated parameter
'enable_logging_hintbit'.

Let's not add more parameters named enable_*. Every parameter enables
something.

Also, I'd be worried about confusion with other log_* and logging_*
parameters, which are about something other than WAL. Maybe it should
be called wal_log_hintbits (or walog_hintbits?).

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Peter Eisentraut <peter_e@gmx.net> writes:

On 11/25/13, 7:02 AM, Sawada Masahiko wrote:

I attached the new version patch which adds separated parameter
'enable_logging_hintbit'.

Let's not add more parameters named enable_*. Every parameter enables
something.

More to the point: there's a convention that we use enable_foo
for planner control parameters that gate usage of a "foo" plan type.
This is not in that category.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Wed, Nov 27, 2013 at 2:28 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Peter Eisentraut <peter_e@gmx.net> writes:

On 11/25/13, 7:02 AM, Sawada Masahiko wrote:

I attached the new version patch which adds separated parameter
'enable_logging_hintbit'.

Let's not add more parameters named enable_*. Every parameter enables
something.

More to the point: there's a convention that we use enable_foo
for planner control parameters that gate usage of a "foo" plan type.
This is not in that category.

Thank you for feedback.
I attached the v4 patch which have modified. Just name changed to
'wal_log_hintbtis'.
And i'm also implementing documentation patch.

Regards,

-------
Sawada Masahiko