Patch: raise default for max_wal_segments to 1GB

Started by Josh Berkusalmost 11 years ago25 messages
#1Josh Berkus
josh@agliodbs.com
1 attachment(s)

Attached.

Per discussion on the thread "Redesigning checkpoint_segments" this
raises the default for the new parameter "max_wal_size" to 1GB.

Seems too small to add it to the CF, but if you want me to, I will.

--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

Attachments:

max_wal_1gb.patchtext/x-patch; name=max_wal_1gb.patchDownload
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
new file mode 100644
index d84dba7..cc4654a
*** a/src/backend/utils/misc/guc.c
--- b/src/backend/utils/misc/guc.c
*************** static struct config_int ConfigureNamesI
*** 2171,2177 ****
  			GUC_UNIT_XSEGS
  		},
  		&max_wal_size,
! 		8, 2, INT_MAX,
  		NULL, assign_max_wal_size, NULL
  	},
  
--- 2171,2177 ----
  			GUC_UNIT_XSEGS
  		},
  		&max_wal_size,
! 		64, 2, INT_MAX,
  		NULL, assign_max_wal_size, NULL
  	},
  
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
new file mode 100644
index f8f9ce1..b4bc29a
*** a/src/backend/utils/misc/postgresql.conf.sample
--- b/src/backend/utils/misc/postgresql.conf.sample
***************
*** 198,204 ****
  # - Checkpoints -
  
  #checkpoint_timeout = 5min		# range 30s-1h
! #max_wal_size = 128MB			# in logfile segments
  #min_wal_size = 80MB
  #checkpoint_completion_target = 0.5	# checkpoint target duration, 0.0 - 1.0
  #checkpoint_warning = 30s		# 0 disables
--- 198,204 ----
  # - Checkpoints -
  
  #checkpoint_timeout = 5min		# range 30s-1h
! #max_wal_size = 1GB			# in logfile segments
  #min_wal_size = 80MB
  #checkpoint_completion_target = 0.5	# checkpoint target duration, 0.0 - 1.0
  #checkpoint_warning = 30s		# 0 disables
#2Josh Berkus
josh@agliodbs.com
In reply to: Josh Berkus (#1)
1 attachment(s)
Re: Patch: raise default for max_wal_segments to 1GB

On 03/02/2015 03:18 PM, Josh Berkus wrote:

Attached.

Per discussion on the thread "Redesigning checkpoint_segments" this
raises the default for the new parameter "max_wal_size" to 1GB.

Seems too small to add it to the CF, but if you want me to, I will.

Ooops, patch didn't include the docs update for some reason. Fixed.

Also, link to relevant post:

/messages/by-id/54F4EF49.1010002@agliodbs.com

--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

Attachments:

max_wal_1gb_b.patchtext/x-patch; name=max_wal_1gb_b.patchDownload
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
new file mode 100644
index 9261e7f..26214ec
*** a/doc/src/sgml/config.sgml
--- b/doc/src/sgml/config.sgml
*************** include_dir 'conf.d'
*** 2406,2412 ****
          checkpoints. This is a soft limit; WAL size can exceed
          <varname>max_wal_size</> under special circumstances, like
          under heavy load, a failing <varname>archive_command</>, or a high
!         <varname>wal_keep_segments</> setting. The default is 128 MB.
          Increasing this parameter can increase the amount of time needed for
          crash recovery.
          This parameter can only be set in the <filename>postgresql.conf</>
--- 2406,2412 ----
          checkpoints. This is a soft limit; WAL size can exceed
          <varname>max_wal_size</> under special circumstances, like
          under heavy load, a failing <varname>archive_command</>, or a high
!         <varname>wal_keep_segments</> setting. The default is 1 GB.
          Increasing this parameter can increase the amount of time needed for
          crash recovery.
          This parameter can only be set in the <filename>postgresql.conf</>
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
new file mode 100644
index d84dba7..cc4654a
*** a/src/backend/utils/misc/guc.c
--- b/src/backend/utils/misc/guc.c
*************** static struct config_int ConfigureNamesI
*** 2171,2177 ****
  			GUC_UNIT_XSEGS
  		},
  		&max_wal_size,
! 		8, 2, INT_MAX,
  		NULL, assign_max_wal_size, NULL
  	},
  
--- 2171,2177 ----
  			GUC_UNIT_XSEGS
  		},
  		&max_wal_size,
! 		64, 2, INT_MAX,
  		NULL, assign_max_wal_size, NULL
  	},
  
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
new file mode 100644
index f8f9ce1..b4bc29a
*** a/src/backend/utils/misc/postgresql.conf.sample
--- b/src/backend/utils/misc/postgresql.conf.sample
***************
*** 198,204 ****
  # - Checkpoints -
  
  #checkpoint_timeout = 5min		# range 30s-1h
! #max_wal_size = 128MB			# in logfile segments
  #min_wal_size = 80MB
  #checkpoint_completion_target = 0.5	# checkpoint target duration, 0.0 - 1.0
  #checkpoint_warning = 30s		# 0 disables
--- 198,204 ----
  # - Checkpoints -
  
  #checkpoint_timeout = 5min		# range 30s-1h
! #max_wal_size = 1GB			# in logfile segments
  #min_wal_size = 80MB
  #checkpoint_completion_target = 0.5	# checkpoint target duration, 0.0 - 1.0
  #checkpoint_warning = 30s		# 0 disables
#3Andres Freund
andres@2ndquadrant.com
In reply to: Josh Berkus (#2)
Re: Patch: raise default for max_wal_segments to 1GB

Hi,

On 2015-03-02 15:40:27 -0800, Josh Berkus wrote:

! #max_wal_size = 1GB # in logfile segments

Independent of the rest of the changes, the "in logfile segments" bit
should probably be changed.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

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

#4Josh Berkus
josh@agliodbs.com
In reply to: Josh Berkus (#1)
1 attachment(s)
Re: Patch: raise default for max_wal_segments to 1GB

On 03/02/2015 03:43 PM, Andres Freund wrote:

Hi,

On 2015-03-02 15:40:27 -0800, Josh Berkus wrote:

! #max_wal_size = 1GB # in logfile segments

Independent of the rest of the changes, the "in logfile segments" bit
should probably be changed.

Point! Although I think it's fair to point out that that wasn't my
omission, but Heikki's.

Version C, with that correction, attached.

--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

Attachments:

max_wal_1gb_c.patchtext/x-patch; name=max_wal_1gb_c.patchDownload
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
new file mode 100644
index 9261e7f..26214ec
*** a/doc/src/sgml/config.sgml
--- b/doc/src/sgml/config.sgml
*************** include_dir 'conf.d'
*** 2406,2412 ****
          checkpoints. This is a soft limit; WAL size can exceed
          <varname>max_wal_size</> under special circumstances, like
          under heavy load, a failing <varname>archive_command</>, or a high
!         <varname>wal_keep_segments</> setting. The default is 128 MB.
          Increasing this parameter can increase the amount of time needed for
          crash recovery.
          This parameter can only be set in the <filename>postgresql.conf</>
--- 2406,2412 ----
          checkpoints. This is a soft limit; WAL size can exceed
          <varname>max_wal_size</> under special circumstances, like
          under heavy load, a failing <varname>archive_command</>, or a high
!         <varname>wal_keep_segments</> setting. The default is 1 GB.
          Increasing this parameter can increase the amount of time needed for
          crash recovery.
          This parameter can only be set in the <filename>postgresql.conf</>
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
new file mode 100644
index d84dba7..cc4654a
*** a/src/backend/utils/misc/guc.c
--- b/src/backend/utils/misc/guc.c
*************** static struct config_int ConfigureNamesI
*** 2171,2177 ****
  			GUC_UNIT_XSEGS
  		},
  		&max_wal_size,
! 		8, 2, INT_MAX,
  		NULL, assign_max_wal_size, NULL
  	},
  
--- 2171,2177 ----
  			GUC_UNIT_XSEGS
  		},
  		&max_wal_size,
! 		64, 2, INT_MAX,
  		NULL, assign_max_wal_size, NULL
  	},
  
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
new file mode 100644
index f8f9ce1..d3ef3e9
*** a/src/backend/utils/misc/postgresql.conf.sample
--- b/src/backend/utils/misc/postgresql.conf.sample
***************
*** 198,204 ****
  # - Checkpoints -
  
  #checkpoint_timeout = 5min		# range 30s-1h
! #max_wal_size = 128MB			# in logfile segments
  #min_wal_size = 80MB
  #checkpoint_completion_target = 0.5	# checkpoint target duration, 0.0 - 1.0
  #checkpoint_warning = 30s		# 0 disables
--- 198,204 ----
  # - Checkpoints -
  
  #checkpoint_timeout = 5min		# range 30s-1h
! #max_wal_size = 1GB
  #min_wal_size = 80MB
  #checkpoint_completion_target = 0.5	# checkpoint target duration, 0.0 - 1.0
  #checkpoint_warning = 30s		# 0 disables
#5Heikki Linnakangas
hlinnaka@iki.fi
In reply to: Andres Freund (#3)
Re: Patch: raise default for max_wal_segments to 1GB

On 03/03/2015 01:43 AM, Andres Freund wrote:

On 2015-03-02 15:40:27 -0800, Josh Berkus wrote:

! #max_wal_size = 1GB # in logfile segments

Independent of the rest of the changes, the "in logfile segments" bit
should probably be changed.

The "base unit" is still logfile segments, so if you write just
"max_wal_size = 10" without a unit, that means 160MB. That's what the
comment means.

Is it needed? Many settings have a similar comment, but most don't. Most
of the ones that do have a magic value 0, -1 as default, so that it's
not obvious from the default what the unit is. For example:

#tcp_keepalives_idle = 0 # TCP_KEEPIDLE, in seconds;
# 0 selects the system default
#tcp_keepalives_interval = 0 # TCP_KEEPINTVL, in seconds;
# 0 selects the system default
#temp_file_limit = -1 # limits per-session temp file space
# in kB, or -1 for no limit
#commit_delay = 0 # range 0-100000, in microseconds
#log_temp_files = -1 # log temporary files equal or larger
# than the specified size in kilobytes;
# -1 disables, 0 logs all temp files
#statement_timeout = 0 # in milliseconds, 0 is disabled
#lock_timeout = 0 # in milliseconds, 0 is disabled
#wal_keep_segments = 0 # in logfile segments, 16MB each; 0 disables

But there are also:

#wal_receiver_timeout = 60s # time that receiver waits for
# communication from master
# in milliseconds; 0 disables
#autovacuum_vacuum_cost_delay = 20ms # default vacuum cost delay for
# autovacuum, in milliseconds;
# -1 means use vacuum_cost_delay

I propose that we remove the comment from max_wal_size, and also remove
the "in milliseconds" from wal_receiver_timeout and
autovacuum_vacuum_cost_delay.

(and we should also change wal_keep_segments to accept MB/GB, as
discussed already)

- Heikki

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

#6Fujii Masao
masao.fujii@gmail.com
In reply to: Josh Berkus (#4)
Re: Patch: raise default for max_wal_segments to 1GB

On Tue, Mar 3, 2015 at 8:51 AM, Josh Berkus <josh@agliodbs.com> wrote:

On 03/02/2015 03:43 PM, Andres Freund wrote:

Hi,

On 2015-03-02 15:40:27 -0800, Josh Berkus wrote:

! #max_wal_size = 1GB # in logfile segments

Independent of the rest of the changes, the "in logfile segments" bit
should probably be changed.

Point! Although I think it's fair to point out that that wasn't my
omission, but Heikki's.

Version C, with that correction, attached.

Minor comments:

"The default settings are 5 minutes and 128 MB, respectively." in wal.sgml
needs to be updated.

int max_wal_size = 8; /* 128 MB */

It's better to update the above code in xlog.c. That's not essential, though.

Regards,

--
Fujii Masao

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

#7Fujii Masao
masao.fujii@gmail.com
In reply to: Heikki Linnakangas (#5)
Re: Patch: raise default for max_wal_segments to 1GB

On Tue, Mar 3, 2015 at 4:25 PM, Heikki Linnakangas <hlinnaka@iki.fi> wrote:

On 03/03/2015 01:43 AM, Andres Freund wrote:

On 2015-03-02 15:40:27 -0800, Josh Berkus wrote:

! #max_wal_size = 1GB # in logfile segments

Independent of the rest of the changes, the "in logfile segments" bit
should probably be changed.

The "base unit" is still logfile segments, so if you write just
"max_wal_size = 10" without a unit, that means 160MB. That's what the
comment means.

Is it needed? Many settings have a similar comment, but most don't. Most of
the ones that do have a magic value 0, -1 as default, so that it's not
obvious from the default what the unit is. For example:

#tcp_keepalives_idle = 0 # TCP_KEEPIDLE, in seconds;
# 0 selects the system default
#tcp_keepalives_interval = 0 # TCP_KEEPINTVL, in seconds;
# 0 selects the system default
#temp_file_limit = -1 # limits per-session temp file space
# in kB, or -1 for no limit
#commit_delay = 0 # range 0-100000, in microseconds
#log_temp_files = -1 # log temporary files equal or
larger
# than the specified size in
kilobytes;
# -1 disables, 0 logs all temp files
#statement_timeout = 0 # in milliseconds, 0 is disabled
#lock_timeout = 0 # in milliseconds, 0 is disabled
#wal_keep_segments = 0 # in logfile segments, 16MB each; 0 disables

But there are also:

#wal_receiver_timeout = 60s # time that receiver waits for
# communication from master
# in milliseconds; 0 disables
#autovacuum_vacuum_cost_delay = 20ms # default vacuum cost delay for
# autovacuum, in milliseconds;
# -1 means use vacuum_cost_delay

I propose that we remove the comment from max_wal_size, and also remove the
"in milliseconds" from wal_receiver_timeout and
autovacuum_vacuum_cost_delay.

Seems OK to me. BTW, we can also remove "in milliseconds" from
wal_sender_timeout.

(and we should also change wal_keep_segments to accept MB/GB, as discussed
already)

+1

Regards,

--
Fujii Masao

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

#8Josh Berkus
josh@agliodbs.com
In reply to: Josh Berkus (#1)
Re: Patch: raise default for max_wal_segments to 1GB

On 03/02/2015 11:25 PM, Heikki Linnakangas wrote:

I propose that we remove the comment from max_wal_size, and also remove
the "in milliseconds" from wal_receiver_timeout and
autovacuum_vacuum_cost_delay.

+1

--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

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

#9Josh Berkus
josh@agliodbs.com
In reply to: Josh Berkus (#1)
Re: Patch: raise default for max_wal_segments to 1GB

On 03/03/2015 10:15 AM, Josh Berkus wrote:

On 03/02/2015 11:25 PM, Heikki Linnakangas wrote:

I propose that we remove the comment from max_wal_size, and also remove
the "in milliseconds" from wal_receiver_timeout and
autovacuum_vacuum_cost_delay.

+1

Actually, let's be consistent about this. It makes no sense to remove
unit comments from some settings which accept ms but not others.

Do we want to remove unit comments from all settings which accept
"MB,GB" or "ms,s,min"? There's more than a few. I'd be in favor of
this, but seems like (a) it should be universal, and (b) its own patch.

--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

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

#10Heikki Linnakangas
hlinnaka@iki.fi
In reply to: Josh Berkus (#9)
Re: Patch: raise default for max_wal_segments to 1GB

On 03/03/2015 08:21 PM, Josh Berkus wrote:

On 03/03/2015 10:15 AM, Josh Berkus wrote:

On 03/02/2015 11:25 PM, Heikki Linnakangas wrote:

I propose that we remove the comment from max_wal_size, and also remove
the "in milliseconds" from wal_receiver_timeout and
autovacuum_vacuum_cost_delay.

+1

Actually, let's be consistent about this. It makes no sense to remove
unit comments from some settings which accept ms but not others.

Do we want to remove unit comments from all settings which accept
"MB,GB" or "ms,s,min"? There's more than a few. I'd be in favor of
this, but seems like (a) it should be universal, and (b) its own patch.

I think it's a good rule that if the commented-out default in the sample
file does not contain a unit, then the base unit is in the comment.
Otherwise it's not. For example:

#shared_buffers = 32MB # min 128kB
# (change requires restart)

The base unit is BLCKSZ, i.e. 8k, but usually people will usually use
MB/GB. And that is evident from the default value, 32MB, so there's no
need to mention it in the comment.

#tcp_keepalives_idle = 0 # TCP_KEEPIDLE, in seconds;
# 0 selects the system default

Here it's not obvious what the unit should be from the default itself.
So the comment says it's "in seconds".

- Heikki

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

#11Josh Berkus
josh@agliodbs.com
In reply to: Josh Berkus (#1)
Re: Patch: raise default for max_wal_segments to 1GB

On 03/03/2015 10:29 AM, Heikki Linnakangas wrote:

On 03/03/2015 08:21 PM, Josh Berkus wrote:

On 03/03/2015 10:15 AM, Josh Berkus wrote:

On 03/02/2015 11:25 PM, Heikki Linnakangas wrote:

I propose that we remove the comment from max_wal_size, and also remove
the "in milliseconds" from wal_receiver_timeout and
autovacuum_vacuum_cost_delay.

+1

Actually, let's be consistent about this. It makes no sense to remove
unit comments from some settings which accept ms but not others.

Do we want to remove unit comments from all settings which accept
"MB,GB" or "ms,s,min"? There's more than a few. I'd be in favor of
this, but seems like (a) it should be universal, and (b) its own patch.

I think it's a good rule that if the commented-out default in the sample
file does not contain a unit, then the base unit is in the comment.
Otherwise it's not. For example:

#shared_buffers = 32MB # min 128kB
# (change requires restart)

The base unit is BLCKSZ, i.e. 8k, but usually people will usually use
MB/GB. And that is evident from the default value, 32MB, so there's no
need to mention it in the comment.

#tcp_keepalives_idle = 0 # TCP_KEEPIDLE, in seconds;
# 0 selects the system default

Here it's not obvious what the unit should be from the default itself.
So the comment says it's "in seconds".

Sure. Although, do we take (s) for tcp_keepalives_idle? Or only an INT?

--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

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

#12Heikki Linnakangas
hlinnaka@iki.fi
In reply to: Josh Berkus (#11)
Re: Patch: raise default for max_wal_segments to 1GB

On 03/03/2015 08:31 PM, Josh Berkus wrote:

On 03/03/2015 10:29 AM, Heikki Linnakangas wrote:

On 03/03/2015 08:21 PM, Josh Berkus wrote:

On 03/03/2015 10:15 AM, Josh Berkus wrote:

On 03/02/2015 11:25 PM, Heikki Linnakangas wrote:

I propose that we remove the comment from max_wal_size, and also remove
the "in milliseconds" from wal_receiver_timeout and
autovacuum_vacuum_cost_delay.

+1

Actually, let's be consistent about this. It makes no sense to remove
unit comments from some settings which accept ms but not others.

Do we want to remove unit comments from all settings which accept
"MB,GB" or "ms,s,min"? There's more than a few. I'd be in favor of
this, but seems like (a) it should be universal, and (b) its own patch.

I think it's a good rule that if the commented-out default in the sample
file does not contain a unit, then the base unit is in the comment.
Otherwise it's not. For example:

#shared_buffers = 32MB # min 128kB
# (change requires restart)

The base unit is BLCKSZ, i.e. 8k, but usually people will usually use
MB/GB. And that is evident from the default value, 32MB, so there's no
need to mention it in the comment.

#tcp_keepalives_idle = 0 # TCP_KEEPIDLE, in seconds;
# 0 selects the system default

Here it's not obvious what the unit should be from the default itself.
So the comment says it's "in seconds".

Sure. Although, do we take (s) for tcp_keepalives_idle? Or only an INT?

It's a "time unit", so you can say "10s" or "10000ms". If you don't
specify a unit, it implies seconds.

- Heikki

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

#13Josh Berkus
josh@agliodbs.com
In reply to: Josh Berkus (#1)
Re: Normalizing units for postgresql.conf WAS: Patch: raise default for max_wal_segments to 1GB

On 03/03/2015 10:37 AM, Heikki Linnakangas wrote:

On 03/03/2015 08:31 PM, Josh Berkus wrote:

On 03/03/2015 10:29 AM, Heikki Linnakangas wrote:

On 03/03/2015 08:21 PM, Josh Berkus wrote:

On 03/03/2015 10:15 AM, Josh Berkus wrote:

On 03/02/2015 11:25 PM, Heikki Linnakangas wrote:

I propose that we remove the comment from max_wal_size, and also
remove
the "in milliseconds" from wal_receiver_timeout and
autovacuum_vacuum_cost_delay.

+1

Actually, let's be consistent about this. It makes no sense to remove
unit comments from some settings which accept ms but not others.

Do we want to remove unit comments from all settings which accept
"MB,GB" or "ms,s,min"? There's more than a few. I'd be in favor of
this, but seems like (a) it should be universal, and (b) its own patch.

I think it's a good rule that if the commented-out default in the sample
file does not contain a unit, then the base unit is in the comment.
Otherwise it's not. For example:

#shared_buffers = 32MB # min 128kB
# (change requires restart)

The base unit is BLCKSZ, i.e. 8k, but usually people will usually use
MB/GB. And that is evident from the default value, 32MB, so there's no
need to mention it in the comment.

#tcp_keepalives_idle = 0 # TCP_KEEPIDLE, in seconds;
# 0 selects the system default

Here it's not obvious what the unit should be from the default itself.
So the comment says it's "in seconds".

Sure. Although, do we take (s) for tcp_keepalives_idle? Or only an INT?

It's a "time unit", so you can say "10s" or "10000ms". If you don't
specify a unit, it implies seconds.

So if we're going to make this consistent, let's make it consistent.

1. All GUCs which accept time/size units will have them on the default
setting.

2. Time/size comments will be removed, *except* from GUCs which do not
accept (ms/s/min) or (kB/MB/GB).

Argument for: the current inconsistency confuses new users and his
entirely historical in nature.

Argument Against: will create unnecessary diff changes between 9.4's
pg.conf and 9.5's pg.conf.

--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

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

#14Corey Huinker
corey.huinker@gmail.com
In reply to: Heikki Linnakangas (#10)
Re: Patch: raise default for max_wal_segments to 1GB

Naive question: would it be /possible/ to change configuration to accept
percentages, and have a percent mean "of existing RAM at startup time"?

I ask because most of the tuning guidelines I see suggest setting memory
parameters as a % of RAM available.

On Tue, Mar 3, 2015 at 1:29 PM, Heikki Linnakangas <hlinnaka@iki.fi> wrote:

Show quoted text

On 03/03/2015 08:21 PM, Josh Berkus wrote:

On 03/03/2015 10:15 AM, Josh Berkus wrote:

On 03/02/2015 11:25 PM, Heikki Linnakangas wrote:

I propose that we remove the comment from max_wal_size, and also remove
the "in milliseconds" from wal_receiver_timeout and
autovacuum_vacuum_cost_delay.

+1

Actually, let's be consistent about this. It makes no sense to remove
unit comments from some settings which accept ms but not others.

Do we want to remove unit comments from all settings which accept
"MB,GB" or "ms,s,min"? There's more than a few. I'd be in favor of
this, but seems like (a) it should be universal, and (b) its own patch.

I think it's a good rule that if the commented-out default in the sample
file does not contain a unit, then the base unit is in the comment.
Otherwise it's not. For example:

#shared_buffers = 32MB # min 128kB
# (change requires restart)

The base unit is BLCKSZ, i.e. 8k, but usually people will usually use
MB/GB. And that is evident from the default value, 32MB, so there's no need
to mention it in the comment.

#tcp_keepalives_idle = 0 # TCP_KEEPIDLE, in seconds;
# 0 selects the system default

Here it's not obvious what the unit should be from the default itself. So
the comment says it's "in seconds".

- Heikki

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

#15Josh Berkus
josh@agliodbs.com
In reply to: Josh Berkus (#1)
1 attachment(s)
Re: Patch: raise default for max_wal_segments to 1GB

On 03/03/2015 02:12 AM, Fujii Masao wrote:

On Tue, Mar 3, 2015 at 8:51 AM, Josh Berkus <josh@agliodbs.com> wrote:

On 03/02/2015 03:43 PM, Andres Freund wrote:

Hi,

On 2015-03-02 15:40:27 -0800, Josh Berkus wrote:

! #max_wal_size = 1GB # in logfile segments

Independent of the rest of the changes, the "in logfile segments" bit
should probably be changed.

Point! Although I think it's fair to point out that that wasn't my
omission, but Heikki's.

Version C, with that correction, attached.

Minor comments:

"The default settings are 5 minutes and 128 MB, respectively." in wal.sgml
needs to be updated.

int max_wal_size = 8; /* 128 MB */

It's better to update the above code in xlog.c. That's not essential, though.

Attached is version D, which incorporates the above two changes, but NOT
a general unit comment cleanup of postgresql.conf, which needs to be an
entirely different patch.

--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

Attachments:

max_wal_1gb_d.patchtext/x-patch; name=max_wal_1gb_d.patchDownload
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
new file mode 100644
index 9261e7f..26214ec
*** a/doc/src/sgml/config.sgml
--- b/doc/src/sgml/config.sgml
*************** include_dir 'conf.d'
*** 2406,2412 ****
          checkpoints. This is a soft limit; WAL size can exceed
          <varname>max_wal_size</> under special circumstances, like
          under heavy load, a failing <varname>archive_command</>, or a high
!         <varname>wal_keep_segments</> setting. The default is 128 MB.
          Increasing this parameter can increase the amount of time needed for
          crash recovery.
          This parameter can only be set in the <filename>postgresql.conf</>
--- 2406,2412 ----
          checkpoints. This is a soft limit; WAL size can exceed
          <varname>max_wal_size</> under special circumstances, like
          under heavy load, a failing <varname>archive_command</>, or a high
!         <varname>wal_keep_segments</> setting. The default is 1 GB.
          Increasing this parameter can increase the amount of time needed for
          crash recovery.
          This parameter can only be set in the <filename>postgresql.conf</>
diff --git a/doc/src/sgml/wal.sgml b/doc/src/sgml/wal.sgml
new file mode 100644
index b57749f..f4083c3
*** a/doc/src/sgml/wal.sgml
--- b/doc/src/sgml/wal.sgml
***************
*** 475,481 ****
     linkend="guc-checkpoint-timeout"> seconds, or if
     <xref linkend="guc-max-wal-size"> is about to be exceeded,
     whichever comes first.
!    The default settings are 5 minutes and 128 MB, respectively.
     If no WAL has been written since the previous checkpoint, new checkpoints
     will be skipped even if <varname>checkpoint_timeout</> has passed.
     (If WAL archiving is being used and you want to put a lower limit on how
--- 475,481 ----
     linkend="guc-checkpoint-timeout"> seconds, or if
     <xref linkend="guc-max-wal-size"> is about to be exceeded,
     whichever comes first.
!    The default settings are 5 minutes and 1 GB, respectively.
     If no WAL has been written since the previous checkpoint, new checkpoints
     will be skipped even if <varname>checkpoint_timeout</> has passed.
     (If WAL archiving is being used and you want to put a lower limit on how
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
new file mode 100644
index a28155f..c3820fa
*** a/src/backend/access/transam/xlog.c
--- b/src/backend/access/transam/xlog.c
*************** extern uint32 bootstrap_data_checksum_ve
*** 79,85 ****
  
  
  /* User-settable parameters */
! int			max_wal_size = 8;		/* 128 MB */
  int			min_wal_size = 5;		/* 80 MB */
  int			wal_keep_segments = 0;
  int			XLOGbuffers = -1;
--- 79,85 ----
  
  
  /* User-settable parameters */
! int			max_wal_size = 64;		/* 1 GB */
  int			min_wal_size = 5;		/* 80 MB */
  int			wal_keep_segments = 0;
  int			XLOGbuffers = -1;
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
new file mode 100644
index d84dba7..cc4654a
*** a/src/backend/utils/misc/guc.c
--- b/src/backend/utils/misc/guc.c
*************** static struct config_int ConfigureNamesI
*** 2171,2177 ****
  			GUC_UNIT_XSEGS
  		},
  		&max_wal_size,
! 		8, 2, INT_MAX,
  		NULL, assign_max_wal_size, NULL
  	},
  
--- 2171,2177 ----
  			GUC_UNIT_XSEGS
  		},
  		&max_wal_size,
! 		64, 2, INT_MAX,
  		NULL, assign_max_wal_size, NULL
  	},
  
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
new file mode 100644
index f8f9ce1..d3ef3e9
*** a/src/backend/utils/misc/postgresql.conf.sample
--- b/src/backend/utils/misc/postgresql.conf.sample
***************
*** 198,204 ****
  # - Checkpoints -
  
  #checkpoint_timeout = 5min		# range 30s-1h
! #max_wal_size = 128MB			# in logfile segments
  #min_wal_size = 80MB
  #checkpoint_completion_target = 0.5	# checkpoint target duration, 0.0 - 1.0
  #checkpoint_warning = 30s		# 0 disables
--- 198,204 ----
  # - Checkpoints -
  
  #checkpoint_timeout = 5min		# range 30s-1h
! #max_wal_size = 1GB
  #min_wal_size = 80MB
  #checkpoint_completion_target = 0.5	# checkpoint target duration, 0.0 - 1.0
  #checkpoint_warning = 30s		# 0 disables
#16Josh Berkus
josh@agliodbs.com
In reply to: Josh Berkus (#1)
Re: Patch: raise default for max_wal_segments to 1GB

On 03/03/2015 10:58 AM, Corey Huinker wrote:

Naive question: would it be /possible/ to change configuration to accept
percentages, and have a percent mean "of existing RAM at startup time"?

I ask because most of the tuning guidelines I see suggest setting memory
parameters as a % of RAM available.

Waaaaaaay off topic for this thread. Please don't hijack; start a new
thread if you want to discuss this.

--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

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

#17Tom Lane
tgl@sss.pgh.pa.us
In reply to: Josh Berkus (#9)
Re: Patch: raise default for max_wal_segments to 1GB

Josh Berkus <josh@agliodbs.com> writes:

Do we want to remove unit comments from all settings which accept
"MB,GB" or "ms,s,min"? There's more than a few. I'd be in favor of
this, but seems like (a) it should be universal, and (b) its own patch.

Meh. Doing this strikes me as a serious documentation failure, since it
is no longer apparent what happens if you put in a unitless number.

Now, if we were to change the server so that it *refused* settings that
didn't have a unit, that argument would become moot. But I'm not going
to defend the castle against the villagers who will show up if you do
that.

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

#18Josh Berkus
josh@agliodbs.com
In reply to: Josh Berkus (#1)
Re: Patch: raise default for max_wal_segments to 1GB

On 03/03/2015 11:57 AM, Tom Lane wrote:

Josh Berkus <josh@agliodbs.com> writes:

Do we want to remove unit comments from all settings which accept
"MB,GB" or "ms,s,min"? There's more than a few. I'd be in favor of
this, but seems like (a) it should be universal, and (b) its own patch.

Meh. Doing this strikes me as a serious documentation failure, since it
is no longer apparent what happens if you put in a unitless number.

Well, you know my opinion about documentation in the pg.conf file.
However, I'm not confident I'm in the majority there.

Now, if we were to change the server so that it *refused* settings that
didn't have a unit, that argument would become moot. But I'm not going
to defend the castle against the villagers who will show up if you do
that.

No, thanks!

--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

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

#19Gavin Flower
GavinFlower@archidevsys.co.nz
In reply to: Tom Lane (#17)
Re: Patch: raise default for max_wal_segments to 1GB

On 04/03/15 08:57, Tom Lane wrote:

Josh Berkus <josh@agliodbs.com> writes:

Do we want to remove unit comments from all settings which accept
"MB,GB" or "ms,s,min"? There's more than a few. I'd be in favor of
this, but seems like (a) it should be universal, and (b) its own patch.

Meh. Doing this strikes me as a serious documentation failure, since it
is no longer apparent what happens if you put in a unitless number.

Now, if we were to change the server so that it *refused* settings that
didn't have a unit, that argument would become moot. But I'm not going
to defend the castle against the villagers who will show up if you do
that.

regards, tom lane

How about introducing a 'strict' mode that does extra checking like that?

JavaScript and Perl, both have a 'strict' mode.

In strict mode you could insist on all quantities having units, and
possibly some additional sanity checking - without enraging the peasants!

Cheers,
Gavin

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

#20Corey Huinker
corey.huinker@gmail.com
In reply to: Josh Berkus (#16)
Re: Patch: raise default for max_wal_segments to 1GB

No intention to hijack. Dropping issue for now.

On Tue, Mar 3, 2015 at 2:05 PM, Josh Berkus <josh@agliodbs.com> wrote:

Show quoted text

On 03/03/2015 10:58 AM, Corey Huinker wrote:

Naive question: would it be /possible/ to change configuration to accept
percentages, and have a percent mean "of existing RAM at startup time"?

I ask because most of the tuning guidelines I see suggest setting memory
parameters as a % of RAM available.

Waaaaaaay off topic for this thread. Please don't hijack; start a new
thread if you want to discuss this.

--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

#21Andrew Dunstan
andrew@dunslane.net
In reply to: Josh Berkus (#18)
Re: Patch: raise default for max_wal_segments to 1GB

On 03/03/2015 02:59 PM, Josh Berkus wrote:

On 03/03/2015 11:57 AM, Tom Lane wrote:

Josh Berkus <josh@agliodbs.com> writes:

Do we want to remove unit comments from all settings which accept
"MB,GB" or "ms,s,min"? There's more than a few. I'd be in favor of
this, but seems like (a) it should be universal, and (b) its own patch.

Meh. Doing this strikes me as a serious documentation failure, since it
is no longer apparent what happens if you put in a unitless number.

Well, you know my opinion about documentation in the pg.conf file.
However, I'm not confident I'm in the majority there.

I think there's a difference between comments about the function of a
GUC and stating the units it's specified in. It's more than annoying to
have to go and look that up where it's not stated.

cheers

andrew

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

#22Jim Nasby
Jim.Nasby@BlueTreble.com
In reply to: Andrew Dunstan (#21)
Re: Patch: raise default for max_wal_segments to 1GB

On 3/3/15 2:36 PM, Andrew Dunstan wrote:

On 03/03/2015 02:59 PM, Josh Berkus wrote:

On 03/03/2015 11:57 AM, Tom Lane wrote:

Josh Berkus <josh@agliodbs.com> writes:

Do we want to remove unit comments from all settings which accept
"MB,GB" or "ms,s,min"? There's more than a few. I'd be in favor of
this, but seems like (a) it should be universal, and (b) its own patch.

Meh. Doing this strikes me as a serious documentation failure, since it
is no longer apparent what happens if you put in a unitless number.

Well, you know my opinion about documentation in the pg.conf file.
However, I'm not confident I'm in the majority there.

If we were consistent across all variables for each vartype we could
maybe get away with this. But that's not the case. How are users
supposed to know that statement_timeout is in ms while
authentication_timeout is in seconds?

I think there's a difference between comments about the function of a
GUC and stating the units it's specified in. It's more than annoying to
have to go and look that up where it's not stated.

Look up the units?
--
Jim Nasby, Data Architect, Blue Treble Consulting
Data in Trouble? Get it in Treble! http://BlueTreble.com

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

#23Greg Stark
stark@mit.edu
In reply to: Tom Lane (#17)
Re: Patch: raise default for max_wal_segments to 1GB

On Tue, Mar 3, 2015 at 7:57 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Now, if we were to change the server so that it *refused* settings that
didn't have a unit, that argument would become moot. But I'm not going
to defend the castle against the villagers who will show up if you do
that.

That might be something we could get away with eventually if we put in
a warning like this for a few years:

WARNING: Using default units of seconds for GUC tcp_keepalives

Though we'll have to deal with the "0 means system default" and "-1
means really really disabled" stuff.

--
greg

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

#24Robert Haas
robertmhaas@gmail.com
In reply to: Josh Berkus (#13)
Re: Normalizing units for postgresql.conf WAS: Patch: raise default for max_wal_segments to 1GB

On Tue, Mar 3, 2015 at 1:42 PM, Josh Berkus <josh@agliodbs.com> wrote:

Sure. Although, do we take (s) for tcp_keepalives_idle? Or only an INT?

It's a "time unit", so you can say "10s" or "10000ms". If you don't
specify a unit, it implies seconds.

So if we're going to make this consistent, let's make it consistent.

1. All GUCs which accept time/size units will have them on the default
setting.

+1.

2. Time/size comments will be removed, *except* from GUCs which do not
accept (ms/s/min) or (kB/MB/GB).

+1.

Argument Against: will create unnecessary diff changes between 9.4's
pg.conf and 9.5's pg.conf.

I don't care about that. I don't like to change postgresql.conf in
minor releases unless we have important reasons for doing so, but
changing it in major releases seems fine.

I do like Greg Stark's suggestion of also warning about unitless settings.

--
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

#25Andres Freund
andres@2ndquadrant.com
In reply to: Josh Berkus (#15)
Re: Patch: raise default for max_wal_segments to 1GB

Hi,

On 2015-03-03 11:04:30 -0800, Josh Berkus wrote:

Attached is version D, which incorporates the above two changes, but NOT
a general unit comment cleanup of postgresql.conf, which needs to be an
entirely different patch.

Pushed!

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

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