allow segment size to be set to < 1GiB
Hi, I was recently reminded of my previous desire to allow setting the segment size to less than 1GB. It's pretty painful to test large amount of segments with a segment size of 1GB, certainly our regression test don't cover anything with multiple segments. This likely wouldn't have detected the issue fixed in 0e758ae89a2, but it make it easier to validate that the fix doesn't break anything badly. In the attached patch I renamed --with-segsize= to --with-segsize-mb= / -Dsegsize= to -Dsegsize_mb=, to avoid somebody building with --with-segsize=2 or such suddenly ending up with an incompatible build. Greetings, Andres Freund >From e8f67e3c27ff553f6a3a26947cac8df91136e8f5 Mon Sep 17 00:00:00 2001 From: Andres Freund Date: Mon, 7 Nov 2022 09:05:35 -0800 Subject: [PATCH] allow-smaller-segsize --- doc/src/sgml/installation.sgml | 6 +++--- doc/src/sgml/storage.sgml | 2 +- .cirrus.yml| 1 + configure | 27 ++- configure.ac | 12 ++-- meson.build| 6 -- meson_options.txt | 4 ++-- 7 files changed, 31 insertions(+), 27 deletions(-) diff --git a/doc/src/sgml/installation.sgml b/doc/src/sgml/installation.sgml index 319c7e69660..20a98c3887c 100644 --- a/doc/src/sgml/installation.sgml +++ b/doc/src/sgml/installation.sgml @@ -1458,13 +1458,13 @@ build-postgresql: - --with-segsize=SEGSIZE + --with-segsize-mb=SEGSIZE - Set the segment size, in gigabytes. Large tables are + Set the segment size, in megabytes. Large tables are divided into multiple operating-system files, each of size equal to the segment size. This avoids problems with file size limits - that exist on many platforms. The default segment size, 1 gigabyte, + that exist on many platforms. The default segment size, 1024 megabytes, is safe on all supported platforms. If your operating system has largefile support (which most do, nowadays), you can use a larger segment size. This can be helpful to reduce the number of diff --git a/doc/src/sgml/storage.sgml b/doc/src/sgml/storage.sgml index e5b9f3f1ffa..03e0bb0ecbf 100644 --- a/doc/src/sgml/storage.sgml +++ b/doc/src/sgml/storage.sgml @@ -236,7 +236,7 @@ When a table or index exceeds 1 GB, it is divided into gigabyte-sized filenode; subsequent segments are named filenode.1, filenode.2, etc. This arrangement avoids problems on platforms that have file size limitations. (Actually, 1 GB is just the default segment size. The segment size can be -adjusted using the configuration option --with-segsize +adjusted using the configuration option --with-segsize-mb when building PostgreSQL.) In principle, free space map and visibility map forks could require multiple segments as well, though this is unlikely to happen in practice. diff --git a/.cirrus.yml b/.cirrus.yml index 9f2282471a9..b0543f20175 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -377,6 +377,7 @@ task: -Dextra_lib_dirs=${brewpath}/lib \ -Dcassert=true \ -Dssl=openssl -Duuid=e2fs -Ddtrace=auto \ + -Dsegsize_mb=1 \ -DPG_TEST_EXTRA="$PG_TEST_EXTRA" \ build diff --git a/configure b/configure index 3966368b8d9..fb55792f51b 100755 --- a/configure +++ b/configure @@ -843,7 +843,7 @@ enable_coverage enable_dtrace enable_tap_tests with_blocksize -with_segsize +with_segsize_mb with_wal_blocksize with_CC with_llvm @@ -1553,7 +1553,8 @@ Optional Packages: --with-pgport=PORTNUM set default port number [5432] --with-blocksize=BLOCKSIZE set table block size in kB [8] - --with-segsize=SEGSIZE set table segment size in GB [1] + --with-segsize-mb=SEGSIZE + set table segment size in MB [1024] --with-wal-blocksize=BLOCKSIZE set WAL block size in kB [8] --with-CC=CMD set compiler (deprecated) @@ -3740,32 +3741,32 @@ $as_echo_n "checking for segment size... " >&6; } -# Check whether --with-segsize was given. -if test "${with_segsize+set}" = set; then : - withval=$with_segsize; +# Check whether --with-segsize-mb was given. +if test "${with_segsize_mb+set}" = set; then : + withval=$with_segsize_mb; case $withval in yes) - as_fn_error $? "argument required for --with-segsize option" "$LINENO" 5 + as_fn_error $? "argument required for --with-segsize-mb option" "$LINENO" 5 ;; no) - as_fn_error $? "argument required for --with-segsize option" "$LINENO" 5 + as_fn_error $? "argument required for --with-segsize-mb option" "$LINENO" 5 ;; *) - segsize=$withval + segsize_mb=$withval ;; esac else - segsize=1 + segsize_mb=1024 fi # this expression is set up to avoid unnecessary integer overflow # blocksize is already guaranteed to be a factor of 1024 -RELSEG_SIZE=`e
Re: allow segment size to be set to < 1GiB
Andres Freund writes: > In the attached patch I renamed --with-segsize= to --with-segsize-mb= / > -Dsegsize= to -Dsegsize_mb=, to avoid somebody building with --with-segsize=2 > or such suddenly ending up with an incompatible build. For the purpose of exercising these code paths with the standard regression tests, even a megabyte seems large -- we don't create very many test tables that are that big. How about instead allowing the segment size to be set in pages? regards, tom lane
Re: allow segment size to be set to < 1GiB
Hi, On 2022-11-07 12:52:25 -0500, Tom Lane wrote: > Andres Freund writes: > > In the attached patch I renamed --with-segsize= to --with-segsize-mb= / > > -Dsegsize= to -Dsegsize_mb=, to avoid somebody building with > > --with-segsize=2 > > or such suddenly ending up with an incompatible build. > > For the purpose of exercising these code paths with the standard > regression tests, even a megabyte seems large -- we don't create > very many test tables that are that big. Good point. > How about instead allowing the segment size to be set in pages? In addition or instead of --with-segsize/-Dsegsize? Just offering the number of pages seems like a not great UI. I guess we could add support for units or such? But that seems messy as well. Greetings, Andres Freund
Re: allow segment size to be set to < 1GiB
Andres Freund writes: > On 2022-11-07 12:52:25 -0500, Tom Lane wrote: >> How about instead allowing the segment size to be set in pages? > In addition or instead of --with-segsize/-Dsegsize? In addition to. What I meant by "instead" was to replace your proposal of --with-segsize-mb. > Just offering the number of pages seems like a not great UI. Well, it's a developer/debug focused API. I think regular users would only care for the existing --with-segsize = so-many-GB API. But for testing, I think --with-segsize-pages = so-many-pages is actually a pretty good UI. regards, tom lane
Re: allow segment size to be set to < 1GiB
On Tue, Nov 8, 2022 at 8:06 AM Tom Lane wrote: > > Andres Freund writes: > > On 2022-11-07 12:52:25 -0500, Tom Lane wrote: > >> How about instead allowing the segment size to be set in pages? > > > In addition or instead of --with-segsize/-Dsegsize? > > In addition to. What I meant by "instead" was to replace > your proposal of --with-segsize-mb. > > > Just offering the number of pages seems like a not great UI. > > Well, it's a developer/debug focused API. I think regular users > would only care for the existing --with-segsize = so-many-GB API. > But for testing, I think --with-segsize-pages = so-many-pages > is actually a pretty good UI. Perhaps --with-segsize-blocks is a better name here as we use block instead of page for --with-blocksize and --with-wal-blocksize. If this option is for dev/debug purposes only, do we want to put a mechanism to disallow it in release builds or something like that, just in case? Or at least, add a note in the documentation? -- Bharath Rupireddy PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Re: allow segment size to be set to < 1GiB
Hi, On 2022-11-07 21:36:33 -0500, Tom Lane wrote: > Andres Freund writes: > > On 2022-11-07 12:52:25 -0500, Tom Lane wrote: > >> How about instead allowing the segment size to be set in pages? > > > In addition or instead of --with-segsize/-Dsegsize? > > In addition to. What I meant by "instead" was to replace > your proposal of --with-segsize-mb. Working on updating the patch. One semi-interesting bit is that <= 5 blocks per segment fails, because corrupt_page_checksum() doesn't know about segments and src/bin/pg_basebackup/t/010_pg_basebackup.pl does # induce further corruption in 5 more blocks $node->stop; for my $i (1 .. 5) { $node->corrupt_page_checksum($file_corrupt1, $i * $block_size); } $node->start; I'd be content with not dealing with that given the use case of the functionality? A buildfarm animal setting it to 10 seem to suffice. Alternatively we could add segment support to corrupt_page_checksum(). Opinions? FWIW, with HEAD, all tests pass with -Dsegsize_blocks=6 on HEAD. Greetings, Andres Freund
Re: allow segment size to be set to < 1GiB
On Tue, Nov 08, 2022 at 06:28:08PM -0800, Andres Freund wrote: > FWIW, with HEAD, all tests pass with -Dsegsize_blocks=6 on HEAD. Wow. The relation page size influences some of the plans in the main regression test suite, but this is nice to hear. +1 from me for more flexibility with this option at compile-time. -- Michael signature.asc Description: PGP signature
Re: allow segment size to be set to < 1GiB
On 2022-11-08 18:28:08 -0800, Andres Freund wrote: > On 2022-11-07 21:36:33 -0500, Tom Lane wrote: > > Andres Freund writes: > > > On 2022-11-07 12:52:25 -0500, Tom Lane wrote: > > >> How about instead allowing the segment size to be set in pages? > > > > > In addition or instead of --with-segsize/-Dsegsize? > > > > In addition to. What I meant by "instead" was to replace > > your proposal of --with-segsize-mb. > > Working on updating the patch. > > One semi-interesting bit is that <= 5 blocks per segment fails, because > corrupt_page_checksum() doesn't know about segments and > src/bin/pg_basebackup/t/010_pg_basebackup.pl does A second question: Both autoconf and meson print the segment size as GB right now. Obviously that'll print out a size of 0 for a segsize < 1GB. The easiest way to would be to just display the number of blocks, but that's not particularly nice. We could show kB, but that ends up being large. Or we can have some code to adjust the unit, but that seems a bit overkill. Opinions?
Re: allow segment size to be set to < 1GiB
Andres Freund writes: > A second question: Both autoconf and meson print the segment size as GB right > now. Obviously that'll print out a size of 0 for a segsize < 1GB. > The easiest way to would be to just display the number of blocks, but that's > not particularly nice. Well, it would be fine if you'd written --with-segsize-blocks, wouldn't it? Can we make the printout format depend on which switch was used? regards, tom lane
Re: allow segment size to be set to < 1GiB
Hi, On 2022-11-09 14:44:42 -0500, Tom Lane wrote: > Andres Freund writes: > > A second question: Both autoconf and meson print the segment size as GB > > right > > now. Obviously that'll print out a size of 0 for a segsize < 1GB. > > > The easiest way to would be to just display the number of blocks, but that's > > not particularly nice. > > Well, it would be fine if you'd written --with-segsize-blocks, wouldn't > it? Can we make the printout format depend on which switch was used? Not sure why I didn't think of that... Updated patch attached. I made one autoconf and one meson CI task use a small block size, but just to ensure it work on both. I'd probably leave it set on one, so we keep the coverage for cfbot? Greetings, Andres Freund >From 7a76be232dd0587e9c84af4e4481d5a98f94bd22 Mon Sep 17 00:00:00 2001 From: Andres Freund Date: Wed, 9 Nov 2022 12:01:54 -0800 Subject: [PATCH v2] Add option to specify segment size in blocks The tests don't have much coverage of segment related code, as we don't create large enough tables. To make it easier to test these paths, add a new option specifying the segment size in blocks. Set the new option to 6 blocks in one of the CI tasks. Smaller numbers currently fail one of the tests, for understandable reasons. While at it, fix some segment size related issues in the meson build. Author: Andres Freund Discussion: https://postgr.es/m/20221107171355.c23fzwanfzq2p...@awork3.anarazel.de --- meson.build| 24 ++--- meson_options.txt | 3 ++ configure.ac | 36 ++- doc/src/sgml/installation.sgml | 14 .cirrus.yml| 2 ++ configure | 63 -- 6 files changed, 118 insertions(+), 24 deletions(-) diff --git a/meson.build b/meson.build index ce2f223a409..b0fcf82a9c2 100644 --- a/meson.build +++ b/meson.build @@ -418,7 +418,19 @@ meson_bin = find_program(meson_binpath, native: true) cdata.set('USE_ASSERT_CHECKING', get_option('cassert') ? 1 : false) -cdata.set('BLCKSZ', get_option('blocksize').to_int() * 1024, description: +blocksize = get_option('blocksize').to_int() * 1024 + +if get_option('segsize_blocks') != 0 + if get_option('segsize') != 1 +warning('both segsize and segsize_blocks specified, segsize_blocks wins') + endif + + segsize = get_option('segsize_blocks') +else + segsize = (get_option('segsize') * 1024 * 1024 * 1024) / blocksize +endif + +cdata.set('BLCKSZ', blocksize, description: '''Size of a disk block --- this also limits the size of a tuple. You can set it bigger if you need bigger tuples (although TOAST should reduce the need to have large tuples, since fields can be spread across multiple tuples). @@ -428,7 +440,7 @@ cdata.set('BLCKSZ', get_option('blocksize').to_int() * 1024, description: Changing BLCKSZ requires an initdb.''') cdata.set('XLOG_BLCKSZ', get_option('wal_blocksize').to_int() * 1024) -cdata.set('RELSEG_SIZE', get_option('segsize') * 131072) +cdata.set('RELSEG_SIZE', segsize) cdata.set('DEF_PGPORT', get_option('pgport')) cdata.set_quoted('DEF_PGPORT_STR', get_option('pgport').to_string()) cdata.set_quoted('PG_KRB_SRVNAM', get_option('krb_srvnam')) @@ -3053,9 +3065,11 @@ if meson.version().version_compare('>=0.57') summary( { - 'data block size': cdata.get('BLCKSZ'), - 'WAL block size': cdata.get('XLOG_BLCKSZ') / 1024, - 'segment size': cdata.get('RELSEG_SIZE') / 131072, + 'data block size': '@0@ kB'.format(cdata.get('BLCKSZ') / 1024), + 'WAL block size': '@0@ kB'.format(cdata.get('XLOG_BLCKSZ') / 1024), + 'segment size': get_option('segsize_blocks') != 0 ? +'@0@ blocks'.format(cdata.get('RELSEG_SIZE')) : +'@0@ GB'.format(get_option('segsize')), }, section: 'Data layout', ) diff --git a/meson_options.txt b/meson_options.txt index c7ea57994dc..0d2a34fd154 100644 --- a/meson_options.txt +++ b/meson_options.txt @@ -13,6 +13,9 @@ option('wal_blocksize', type : 'combo', option('segsize', type : 'integer', value : 1, description : '''Segment size, in gigabytes''') +option('segsize_blocks', type : 'integer', value: 0, + description : '''Segment size, in blocks''') + # Miscellaneous options diff --git a/configure.ac b/configure.ac index f76b7ee31fc..94542e862cf 100644 --- a/configure.ac +++ b/configure.ac @@ -285,15 +285,31 @@ AC_DEFINE_UNQUOTED([BLCKSZ], ${BLCKSZ}, [ # # Relation segment size # -AC_MSG_CHECKING([for segment size]) PGAC_ARG_REQ(with, segsize, [SEGSIZE], [set table segment size in GB [1]], [segsize=$withval], [segsize=1]) -# this expression is set up to avoid unnecessary integer overflow -# blocksize is already guaranteed to be a factor of 1024 -RELSEG_SIZE=`expr '(' 1024 / ${blocksize} ')' '*' ${segsize} '*' 1024` -test $? -eq 0 || exit 1 -AC_MSG_RESULT([${segsize}GB]) +PGAC_ARG_REQ(with, segsize-blocks, [SEGSIZE_BLOCKS], [set ta
Re: allow segment size to be set to < 1GiB
On 2022-11-09 We 15:25, Andres Freund wrote: > Hi, > > On 2022-11-09 14:44:42 -0500, Tom Lane wrote: >> Andres Freund writes: >>> A second question: Both autoconf and meson print the segment size as GB >>> right >>> now. Obviously that'll print out a size of 0 for a segsize < 1GB. >>> The easiest way to would be to just display the number of blocks, but that's >>> not particularly nice. >> Well, it would be fine if you'd written --with-segsize-blocks, wouldn't >> it? Can we make the printout format depend on which switch was used? > Not sure why I didn't think of that... > > Updated patch attached. > > I made one autoconf and one meson CI task use a small block size, but just to > ensure it work on both. I'd probably leave it set on one, so we keep the > coverage for cfbot? > Are we going to impose some sane minimum, or leave it up to developers to discover that for themselves? cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com
Re: allow segment size to be set to < 1GiB
Hi, On 2022-11-17 09:58:48 -0500, Andrew Dunstan wrote: > On 2022-11-09 We 15:25, Andres Freund wrote: > > Hi, > > > > On 2022-11-09 14:44:42 -0500, Tom Lane wrote: > >> Andres Freund writes: > >>> A second question: Both autoconf and meson print the segment size as GB > >>> right > >>> now. Obviously that'll print out a size of 0 for a segsize < 1GB. > >>> The easiest way to would be to just display the number of blocks, but > >>> that's > >>> not particularly nice. > >> Well, it would be fine if you'd written --with-segsize-blocks, wouldn't > >> it? Can we make the printout format depend on which switch was used? > > Not sure why I didn't think of that... > > > > Updated patch attached. > > > > I made one autoconf and one meson CI task use a small block size, but just > > to > > ensure it work on both. I'd probably leave it set on one, so we keep the > > coverage for cfbot? > > > > Are we going to impose some sane minimum, or leave it up to developers > to discover that for themselves? I don't think we should. It's actually useful to e.g. use 1 page sized segments for testing, and with one exceptions the tests pass with it too. Do you see a reason to impose one? Greetings, Andres Freund
Re: allow segment size to be set to < 1GiB
Andres Freund writes: > On 2022-11-17 09:58:48 -0500, Andrew Dunstan wrote: >> Are we going to impose some sane minimum, or leave it up to developers >> to discover that for themselves? > I don't think we should. It's actually useful to e.g. use 1 page sized > segments for testing, and with one exceptions the tests pass with it too. Do > you see a reason to impose one? Yeah, I think we should allow setting it to 1 block. This switch is only for testing purposes (I hope the docs make that clear). regards, tom lane
Re: allow segment size to be set to < 1GiB
On 2022-11-17 10:48:52 -0500, Tom Lane wrote: > Yeah, I think we should allow setting it to 1 block. This switch is > only for testing purposes (I hope the docs make that clear). "This option is only for developers, to test segment related code."
Re: allow segment size to be set to < 1GiB
On 2022-11-17 Th 10:48, Tom Lane wrote: > Andres Freund writes: >> On 2022-11-17 09:58:48 -0500, Andrew Dunstan wrote: >>> Are we going to impose some sane minimum, or leave it up to developers >>> to discover that for themselves? >> I don't think we should. It's actually useful to e.g. use 1 page sized >> segments for testing, and with one exceptions the tests pass with it too. Do >> you see a reason to impose one? > Yeah, I think we should allow setting it to 1 block. This switch is > only for testing purposes (I hope the docs make that clear). > > Yeah clearly if 1 is useful there's no point in limiting it. cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com
Re: allow segment size to be set to < 1GiB
Hi, On 2022-11-09 12:25:09 -0800, Andres Freund wrote: > Updated patch attached. I pushed it now. > I made one autoconf and one meson CI task use a small block size, but just to > ensure it work on both. I'd probably leave it set on one, so we keep the > coverage for cfbot? It doesn't seem to cost that much, so I left it set in those two tasks for now. Greetings, Andres Freund