public inbox for [email protected]
help / color / mirror / Atom feed[PATCH 5/8] f!3nd attempt to use pkgconfig to allow compiling on OSX
10+ messages / 5 participants
[nested] [flat]
* [PATCH 5/8] f!3nd attempt to use pkgconfig to allow compiling on OSX
@ 2021-03-11 15:18 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 10+ messages in thread
From: Justin Pryzby @ 2021-03-11 15:18 UTC (permalink / raw)
---
configure | 249 +++++++-----------
configure.ac | 13 +-
src/backend/access/common/toast_compression.c | 10 +-
src/include/pg_config.h.in | 6 +-
src/tools/msvc/Solution.pm | 2 +-
5 files changed, 118 insertions(+), 162 deletions(-)
diff --git a/configure b/configure
index 87ed16060e..440d1e8ce5 100755
--- a/configure
+++ b/configure
@@ -654,8 +654,6 @@ UUID_LIBS
LDAP_LIBS_BE
LDAP_LIBS_FE
with_ssl
-LZ4_LIBS
-LZ4_CFLAGS
PTHREAD_CFLAGS
PTHREAD_LIBS
PTHREAD_CC
@@ -701,6 +699,8 @@ with_gnu_ld
LD
LDFLAGS_SL
LDFLAGS_EX
+LZ4_LIBS
+LZ4_CFLAGS
with_lz4
with_zlib
with_system_tzdata
@@ -895,12 +895,12 @@ ICU_LIBS
XML2_CONFIG
XML2_CFLAGS
XML2_LIBS
+LZ4_CFLAGS
+LZ4_LIBS
LDFLAGS_EX
LDFLAGS_SL
PERL
PYTHON
-LZ4_CFLAGS
-LZ4_LIBS
MSGFMT
TCLSH'
@@ -1603,12 +1603,12 @@ Some influential environment variables:
XML2_CONFIG path to xml2-config utility
XML2_CFLAGS C compiler flags for XML2, overriding pkg-config
XML2_LIBS linker flags for XML2, overriding pkg-config
+ LZ4_CFLAGS C compiler flags for LZ4, overriding pkg-config
+ LZ4_LIBS linker flags for LZ4, overriding pkg-config
LDFLAGS_EX extra linker flags for linking executables only
LDFLAGS_SL extra linker flags for linking shared libraries only
PERL Perl program
PYTHON Python program
- LZ4_CFLAGS C compiler flags for LZ4, overriding pkg-config
- LZ4_LIBS linker flags for LZ4, overriding pkg-config
MSGFMT msgfmt program for NLS
TCLSH Tcl interpreter program (tclsh)
@@ -8607,6 +8607,102 @@ fi
$as_echo "$with_lz4" >&6; }
+if test "$with_lz4" = yes; then
+
+pkg_failed=no
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for liblz4" >&5
+$as_echo_n "checking for liblz4... " >&6; }
+
+if test -n "$LZ4_CFLAGS"; then
+ pkg_cv_LZ4_CFLAGS="$LZ4_CFLAGS"
+ elif test -n "$PKG_CONFIG"; then
+ if test -n "$PKG_CONFIG" && \
+ { { $as_echo "$as_me:${as_lineno-$LINENO}: \$PKG_CONFIG --exists --print-errors \"liblz4\""; } >&5
+ ($PKG_CONFIG --exists --print-errors "liblz4") 2>&5
+ ac_status=$?
+ $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
+ test $ac_status = 0; }; then
+ pkg_cv_LZ4_CFLAGS=`$PKG_CONFIG --cflags "liblz4" 2>/dev/null`
+ test "x$?" != "x0" && pkg_failed=yes
+else
+ pkg_failed=yes
+fi
+ else
+ pkg_failed=untried
+fi
+if test -n "$LZ4_LIBS"; then
+ pkg_cv_LZ4_LIBS="$LZ4_LIBS"
+ elif test -n "$PKG_CONFIG"; then
+ if test -n "$PKG_CONFIG" && \
+ { { $as_echo "$as_me:${as_lineno-$LINENO}: \$PKG_CONFIG --exists --print-errors \"liblz4\""; } >&5
+ ($PKG_CONFIG --exists --print-errors "liblz4") 2>&5
+ ac_status=$?
+ $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
+ test $ac_status = 0; }; then
+ pkg_cv_LZ4_LIBS=`$PKG_CONFIG --libs "liblz4" 2>/dev/null`
+ test "x$?" != "x0" && pkg_failed=yes
+else
+ pkg_failed=yes
+fi
+ else
+ pkg_failed=untried
+fi
+
+
+
+if test $pkg_failed = yes; then
+ { $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5
+$as_echo "no" >&6; }
+
+if $PKG_CONFIG --atleast-pkgconfig-version 0.20; then
+ _pkg_short_errors_supported=yes
+else
+ _pkg_short_errors_supported=no
+fi
+ if test $_pkg_short_errors_supported = yes; then
+ LZ4_PKG_ERRORS=`$PKG_CONFIG --short-errors --print-errors --cflags --libs "liblz4" 2>&1`
+ else
+ LZ4_PKG_ERRORS=`$PKG_CONFIG --print-errors --cflags --libs "liblz4" 2>&1`
+ fi
+ # Put the nasty error message in config.log where it belongs
+ echo "$LZ4_PKG_ERRORS" >&5
+
+ as_fn_error $? "Package requirements (liblz4) were not met:
+
+$LZ4_PKG_ERRORS
+
+Consider adjusting the PKG_CONFIG_PATH environment variable if you
+installed software in a non-standard prefix.
+
+Alternatively, you may set the environment variables LZ4_CFLAGS
+and LZ4_LIBS to avoid the need to call pkg-config.
+See the pkg-config man page for more details." "$LINENO" 5
+elif test $pkg_failed = untried; then
+ { $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5
+$as_echo "no" >&6; }
+ { { $as_echo "$as_me:${as_lineno-$LINENO}: error: in \`$ac_pwd':" >&5
+$as_echo "$as_me: error: in \`$ac_pwd':" >&2;}
+as_fn_error $? "The pkg-config script could not be found or is too old. Make sure it
+is in your PATH or set the PKG_CONFIG environment variable to the full
+path to pkg-config.
+
+Alternatively, you may set the environment variables LZ4_CFLAGS
+and LZ4_LIBS to avoid the need to call pkg-config.
+See the pkg-config man page for more details.
+
+To get pkg-config, see <http://pkg-config.freedesktop.org/;.
+See \`config.log' for more details" "$LINENO" 5; }
+else
+ LZ4_CFLAGS=$pkg_cv_LZ4_CFLAGS
+ LZ4_LIBS=$pkg_cv_LZ4_LIBS
+ { $as_echo "$as_me:${as_lineno-$LINENO}: result: yes" >&5
+$as_echo "yes" >&6; }
+
+fi
+ LIBS="$LZ4_LIBS $LIBS"
+ CFLAGS="$LZ4_CFLAGS $CFLAGS"
+fi
+
#
# Assignments
#
@@ -12154,147 +12250,6 @@ fi
fi
-if test "$with_lz4" = yes; then
-
-pkg_failed=no
-{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for liblz4" >&5
-$as_echo_n "checking for liblz4... " >&6; }
-
-if test -n "$LZ4_CFLAGS"; then
- pkg_cv_LZ4_CFLAGS="$LZ4_CFLAGS"
- elif test -n "$PKG_CONFIG"; then
- if test -n "$PKG_CONFIG" && \
- { { $as_echo "$as_me:${as_lineno-$LINENO}: \$PKG_CONFIG --exists --print-errors \"liblz4\""; } >&5
- ($PKG_CONFIG --exists --print-errors "liblz4") 2>&5
- ac_status=$?
- $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
- test $ac_status = 0; }; then
- pkg_cv_LZ4_CFLAGS=`$PKG_CONFIG --cflags "liblz4" 2>/dev/null`
- test "x$?" != "x0" && pkg_failed=yes
-else
- pkg_failed=yes
-fi
- else
- pkg_failed=untried
-fi
-if test -n "$LZ4_LIBS"; then
- pkg_cv_LZ4_LIBS="$LZ4_LIBS"
- elif test -n "$PKG_CONFIG"; then
- if test -n "$PKG_CONFIG" && \
- { { $as_echo "$as_me:${as_lineno-$LINENO}: \$PKG_CONFIG --exists --print-errors \"liblz4\""; } >&5
- ($PKG_CONFIG --exists --print-errors "liblz4") 2>&5
- ac_status=$?
- $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
- test $ac_status = 0; }; then
- pkg_cv_LZ4_LIBS=`$PKG_CONFIG --libs "liblz4" 2>/dev/null`
- test "x$?" != "x0" && pkg_failed=yes
-else
- pkg_failed=yes
-fi
- else
- pkg_failed=untried
-fi
-
-
-
-if test $pkg_failed = yes; then
- { $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5
-$as_echo "no" >&6; }
-
-if $PKG_CONFIG --atleast-pkgconfig-version 0.20; then
- _pkg_short_errors_supported=yes
-else
- _pkg_short_errors_supported=no
-fi
- if test $_pkg_short_errors_supported = yes; then
- LZ4_PKG_ERRORS=`$PKG_CONFIG --short-errors --print-errors --cflags --libs "liblz4" 2>&1`
- else
- LZ4_PKG_ERRORS=`$PKG_CONFIG --print-errors --cflags --libs "liblz4" 2>&1`
- fi
- # Put the nasty error message in config.log where it belongs
- echo "$LZ4_PKG_ERRORS" >&5
-
- as_fn_error $? "Package requirements (liblz4) were not met:
-
-$LZ4_PKG_ERRORS
-
-Consider adjusting the PKG_CONFIG_PATH environment variable if you
-installed software in a non-standard prefix.
-
-Alternatively, you may set the environment variables LZ4_CFLAGS
-and LZ4_LIBS to avoid the need to call pkg-config.
-See the pkg-config man page for more details." "$LINENO" 5
-elif test $pkg_failed = untried; then
- { $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5
-$as_echo "no" >&6; }
- { { $as_echo "$as_me:${as_lineno-$LINENO}: error: in \`$ac_pwd':" >&5
-$as_echo "$as_me: error: in \`$ac_pwd':" >&2;}
-as_fn_error $? "The pkg-config script could not be found or is too old. Make sure it
-is in your PATH or set the PKG_CONFIG environment variable to the full
-path to pkg-config.
-
-Alternatively, you may set the environment variables LZ4_CFLAGS
-and LZ4_LIBS to avoid the need to call pkg-config.
-See the pkg-config man page for more details.
-
-To get pkg-config, see <http://pkg-config.freedesktop.org/;.
-See \`config.log' for more details" "$LINENO" 5; }
-else
- LZ4_CFLAGS=$pkg_cv_LZ4_CFLAGS
- LZ4_LIBS=$pkg_cv_LZ4_LIBS
- { $as_echo "$as_me:${as_lineno-$LINENO}: result: yes" >&5
-$as_echo "yes" >&6; }
-
-fi
- { $as_echo "$as_me:${as_lineno-$LINENO}: checking for LZ4_compress in -llz4" >&5
-$as_echo_n "checking for LZ4_compress in -llz4... " >&6; }
-if ${ac_cv_lib_lz4_LZ4_compress+:} false; then :
- $as_echo_n "(cached) " >&6
-else
- ac_check_lib_save_LIBS=$LIBS
-LIBS="-llz4 $LIBS"
-cat confdefs.h - <<_ACEOF >conftest.$ac_ext
-/* end confdefs.h. */
-
-/* Override any GCC internal prototype to avoid an error.
- Use char because int might match the return type of a GCC
- builtin and then its argument prototype would still apply. */
-#ifdef __cplusplus
-extern "C"
-#endif
-char LZ4_compress ();
-int
-main ()
-{
-return LZ4_compress ();
- ;
- return 0;
-}
-_ACEOF
-if ac_fn_c_try_link "$LINENO"; then :
- ac_cv_lib_lz4_LZ4_compress=yes
-else
- ac_cv_lib_lz4_LZ4_compress=no
-fi
-rm -f core conftest.err conftest.$ac_objext \
- conftest$ac_exeext conftest.$ac_ext
-LIBS=$ac_check_lib_save_LIBS
-fi
-{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_cv_lib_lz4_LZ4_compress" >&5
-$as_echo "$ac_cv_lib_lz4_LZ4_compress" >&6; }
-if test "x$ac_cv_lib_lz4_LZ4_compress" = xyes; then :
- cat >>confdefs.h <<_ACEOF
-#define HAVE_LIBLZ4 1
-_ACEOF
-
- LIBS="-llz4 $LIBS"
-
-else
- as_fn_error $? "library 'lz4' is required for LZ4 support" "$LINENO" 5
-fi
-
-fi
-
if test "$enable_spinlocks" = yes; then
$as_echo "#define HAVE_SPINLOCKS 1" >>confdefs.h
diff --git a/configure.ac b/configure.ac
index c8f199dc5e..780791ae8a 100644
--- a/configure.ac
+++ b/configure.ac
@@ -995,6 +995,12 @@ PGAC_ARG_BOOL(with, lz4, no, [build with LZ4 support],
AC_MSG_RESULT([$with_lz4])
AC_SUBST(with_lz4)
+if test "$with_lz4" = yes; then
+ PKG_CHECK_MODULES(LZ4, liblz4)
+ LIBS="$LZ4_LIBS $LIBS"
+ CFLAGS="$LZ4_CFLAGS $CFLAGS"
+fi
+
#
# Assignments
#
@@ -1183,11 +1189,6 @@ failure. It is possible the compiler isn't looking in the proper directory.
Use --without-zlib to disable zlib support.])])
fi
-if test "$with_lz4" = yes; then
- PKG_CHECK_MODULES(LZ4, liblz4)
- AC_CHECK_LIB(lz4, LZ4_compress, [], [AC_MSG_ERROR([library 'lz4' is required for LZ4 support])])
-fi
-
if test "$enable_spinlocks" = yes; then
AC_DEFINE(HAVE_SPINLOCKS, 1, [Define to 1 if you have spinlocks.])
else
@@ -1423,7 +1424,7 @@ fi
if test "$with_lz4" = yes; then
AC_CHECK_HEADERS(lz4/lz4.h, [],
- [AC_CHECK_HEADERS(lz4.h, [], [AC_MSG_ERROR([lz4.h header file is required for LZ4])])])
+ [AC_CHECK_HEADERS(lz4.h, [], [AC_MSG_ERROR([lz4.h header file is required for LZ4])])])
fi
if test "$with_gssapi" = yes ; then
diff --git a/src/backend/access/common/toast_compression.c b/src/backend/access/common/toast_compression.c
index 3463b42438..db4911ce43 100644
--- a/src/backend/access/common/toast_compression.c
+++ b/src/backend/access/common/toast_compression.c
@@ -13,7 +13,7 @@
*/
#include "postgres.h"
-#ifdef HAVE_LIBLZ4
+#ifdef USE_LZ4
#include <lz4.h>
#endif
@@ -168,7 +168,7 @@ pglz_cmdecompress_slice(const struct varlena *value,
static struct varlena *
lz4_cmcompress(const struct varlena *value)
{
-#ifndef HAVE_LIBLZ4
+#ifndef USE_LZ4
NO_LZ4_SUPPORT();
#else
int32 valsize;
@@ -212,7 +212,7 @@ lz4_cmcompress(const struct varlena *value)
static struct varlena *
lz4_cmdecompress(const struct varlena *value)
{
-#ifndef HAVE_LIBLZ4
+#ifndef USE_LZ4
NO_LZ4_SUPPORT();
#else
int32 rawsize;
@@ -246,7 +246,7 @@ lz4_cmdecompress(const struct varlena *value)
static struct varlena *
lz4_cmdecompress_slice(const struct varlena *value, int32 slicelength)
{
-#ifndef HAVE_LIBLZ4
+#ifndef USE_LZ4
NO_LZ4_SUPPORT();
#elif LZ4_VERSION_NUMBER < 10803
return lz4_cmdecompress(value);
@@ -289,7 +289,7 @@ CompressionNameToMethod(char *compression)
else if (strcmp(toast_compression[LZ4_COMPRESSION_ID].cmname,
compression) == 0)
{
-#ifndef HAVE_LIBLZ4
+#ifndef USE_LZ4
NO_LZ4_SUPPORT();
#endif
return LZ4_COMPRESSION;
diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in
index 6007d72a73..0a6422da4f 100644
--- a/src/include/pg_config.h.in
+++ b/src/include/pg_config.h.in
@@ -346,9 +346,6 @@
/* Define to 1 if you have the `z' library (-lz). */
#undef HAVE_LIBZ
-/* Define to 1 if you have the `lz4' library (-llz4). */
-#undef HAVE_LIBLZ4
-
/* Define to 1 if you have the `link' function. */
#undef HAVE_LINK
@@ -902,6 +899,9 @@
/* Define to 1 to build with LLVM based JIT support. (--with-llvm) */
#undef USE_LLVM
+/* Define to 1 to build with LZ4 support (--with-lz4) */
+#undef USE_LZ4
+
/* Define to select named POSIX semaphores. */
#undef USE_NAMED_POSIX_SEMAPHORES
diff --git a/src/tools/msvc/Solution.pm b/src/tools/msvc/Solution.pm
index 5f39a92111..14605371bb 100644
--- a/src/tools/msvc/Solution.pm
+++ b/src/tools/msvc/Solution.pm
@@ -307,7 +307,6 @@ sub GenerateFiles
HAVE_LIBXML2 => undef,
HAVE_LIBXSLT => undef,
HAVE_LIBZ => $self->{options}->{zlib} ? 1 : undef,
- HAVE_LIBLZ4 => undef,
HAVE_LINK => undef,
HAVE_LOCALE_T => 1,
HAVE_LONG_INT_64 => undef,
@@ -486,6 +485,7 @@ sub GenerateFiles
USE_ICU => $self->{options}->{icu} ? 1 : undef,
USE_LIBXML => undef,
USE_LIBXSLT => undef,
+ USE_LZ4 => undef,
USE_LDAP => $self->{options}->{ldap} ? 1 : undef,
USE_LLVM => undef,
USE_NAMED_POSIX_SEMAPHORES => undef,
--
2.17.0
--C94crkcyjafcjHxo
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="0006-Add-default_toast_compression-GUC.patch"
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: scalability bottlenecks with (many) partitions (and more)
@ 2024-06-24 15:05 Robert Haas <[email protected]>
0 siblings, 1 reply; 10+ messages in thread
From: Robert Haas @ 2024-06-24 15:05 UTC (permalink / raw)
To: Tomas Vondra <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>
On Sun, Jan 28, 2024 at 4:57 PM Tomas Vondra
<[email protected]> wrote:
> For NUM_LOCK_PARTITIONS this is pretty simple (see 0001 patch). The
> LWLock table has 16 partitions by default - it's quite possible that on
> machine with many cores and/or many partitions, we can easily hit this.
> So I bumped this 4x to 64 partitions.
I think this probably makes sense. I'm a little worried that we're
just kicking the can down the road here where maybe we should be
solving the problem in some more fundamental way, and I'm also a
little worried that we might be reducing single-core performance. But
it's probably fine.
> What I ended up doing is having a hash table of 16-element arrays. There
> are 64 "pieces", each essentially the (16 x OID + UINT64 bitmap) that we
> have now. Each OID is mapped to exactly one of these parts as if in a
> hash table, and in each of those 16-element parts we do exactly the same
> thing we do now (linear search, removal, etc.). This works great, the
> locality is great, etc. The one disadvantage is this makes PGPROC
> larger, but I did a lot of benchmarks and I haven't seen any regression
> that I could attribute to this. (More about this later.)
I think this is a reasonable approach. Some comments:
- FastPathLocalUseInitialized seems unnecessary to me; the contents of
an uninitialized local variable are undefined, but an uninitialized
global variable always starts out zeroed.
- You need comments in various places, including here, where someone
is certain to have questions about the algorithm and choice of
constants:
+#define FAST_PATH_LOCK_REL_GROUP(rel) (((uint64) (rel) * 7883 + 4481)
% FP_LOCK_GROUPS_PER_BACKEND)
When I originally coded up the fast-path locking stuff, I supposed
that we couldn't make the number of slots too big because the
algorithm requires a linear search of the whole array. But with this
one trick (a partially-associative cache), there's no real reason that
I can think of why you can't make the number of slots almost
arbitrarily large. At some point you're going to use too much memory,
and probably before that point you're going to make the cache big
enough that it doesn't fit in the CPU cache of an individual core, at
which point possibly it will stop working as well. But honestly ... I
don't quite see why this approach couldn't be scaled quite far.
Like, if we raised FP_LOCK_GROUPS_PER_BACKEND from your proposed value
of 64 to say 65536, would that still perform well? I'm not saying we
should do that, because that's probably a silly amount of memory to
use for this, but the point is that when you start to have enough
partitions that you run out of lock slots, performance is going to
degrade, so you can imagine wanting to try to have enough lock groups
to make that unlikely. Which leads me to wonder if there's any
particular number of lock groups that is clearly "too many" or whether
it's just about how much memory we want to use.
--
Robert Haas
EDB: http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: scalability bottlenecks with (many) partitions (and more)
@ 2024-06-25 10:04 Tomas Vondra <[email protected]>
parent: Robert Haas <[email protected]>
0 siblings, 2 replies; 10+ messages in thread
From: Tomas Vondra @ 2024-06-25 10:04 UTC (permalink / raw)
To: Robert Haas <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>
On 6/24/24 17:05, Robert Haas wrote:
> On Sun, Jan 28, 2024 at 4:57 PM Tomas Vondra
> <[email protected]> wrote:
>> For NUM_LOCK_PARTITIONS this is pretty simple (see 0001 patch). The
>> LWLock table has 16 partitions by default - it's quite possible that on
>> machine with many cores and/or many partitions, we can easily hit this.
>> So I bumped this 4x to 64 partitions.
>
> I think this probably makes sense. I'm a little worried that we're
> just kicking the can down the road here where maybe we should be
> solving the problem in some more fundamental way, and I'm also a
> little worried that we might be reducing single-core performance. But
> it's probably fine.
>
Yeah, I haven't seen this causing any regressions - the sensitive paths
typically lock only one partition, so having more of them does not
affect that. Or if it does, it's likely a reasonable trade off as it
reduces the risk of lock contention.
That being said, I don't recall benchmarking this patch in isolation,
only with the other patches. Maybe I should do that ...
>> What I ended up doing is having a hash table of 16-element arrays. There
>> are 64 "pieces", each essentially the (16 x OID + UINT64 bitmap) that we
>> have now. Each OID is mapped to exactly one of these parts as if in a
>> hash table, and in each of those 16-element parts we do exactly the same
>> thing we do now (linear search, removal, etc.). This works great, the
>> locality is great, etc. The one disadvantage is this makes PGPROC
>> larger, but I did a lot of benchmarks and I haven't seen any regression
>> that I could attribute to this. (More about this later.)
>
> I think this is a reasonable approach. Some comments:
>
> - FastPathLocalUseInitialized seems unnecessary to me; the contents of
> an uninitialized local variable are undefined, but an uninitialized
> global variable always starts out zeroed.
>
OK. I didn't realize global variables start a zero.
> - You need comments in various places, including here, where someone
> is certain to have questions about the algorithm and choice of
> constants:
>
> +#define FAST_PATH_LOCK_REL_GROUP(rel) (((uint64) (rel) * 7883 + 4481)
> % FP_LOCK_GROUPS_PER_BACKEND)
>
Yeah, definitely needs comment explaining this.
I admit those numbers are pretty arbitrary primes, to implement a
trivial hash function. That was good enough for a PoC patch, but maybe
for a "proper" version this should use a better hash function. It needs
to be fast, and maybe it doesn't matter that much if it's not perfect.
> When I originally coded up the fast-path locking stuff, I supposed
> that we couldn't make the number of slots too big because the
> algorithm requires a linear search of the whole array. But with this
> one trick (a partially-associative cache), there's no real reason that
> I can think of why you can't make the number of slots almost
> arbitrarily large. At some point you're going to use too much memory,
> and probably before that point you're going to make the cache big
> enough that it doesn't fit in the CPU cache of an individual core, at
> which point possibly it will stop working as well. But honestly ... I
> don't quite see why this approach couldn't be scaled quite far.
>
I don't think I've heard the term "partially-associative cache" before,
but now that I look at the approach again, it very much reminds me how
set-associative caches work (e.g. with cachelines in CPU caches). It's a
16-way associative cache, assigning each entry into one of 16 slots.
I must have been reading some papers in this area shortly before the PoC
patch, and the idea came from there, probably. Which is good, because it
means it's a well-understood and widely-used approach.
> Like, if we raised FP_LOCK_GROUPS_PER_BACKEND from your proposed value
> of 64 to say 65536, would that still perform well? I'm not saying we
> should do that, because that's probably a silly amount of memory to
> use for this, but the point is that when you start to have enough
> partitions that you run out of lock slots, performance is going to
> degrade, so you can imagine wanting to try to have enough lock groups
> to make that unlikely. Which leads me to wonder if there's any
> particular number of lock groups that is clearly "too many" or whether
> it's just about how much memory we want to use.
>
That's an excellent question. I don't know.
I agree 64 groups is pretty arbitrary, and having 1024 may not be enough
even with a modest number of partitions. When I was thinking about using
a higher value, my main concern was that it'd made the PGPROC entry
larger. Each "fast-path" group is ~72B, so 64 groups is ~4.5kB, and that
felt like quite a bit.
But maybe it's fine and we could make it much larger - L3 caches tend to
be many MBs these days, although AFAIK it's shared by threads running on
the CPU.
I'll see if I can do some more testing of this, and see if there's a
value where it stops improving / starts degrading, etc.
regards
--
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: scalability bottlenecks with (many) partitions (and more)
@ 2024-06-25 20:13 Robert Haas <[email protected]>
parent: Tomas Vondra <[email protected]>
1 sibling, 0 replies; 10+ messages in thread
From: Robert Haas @ 2024-06-25 20:13 UTC (permalink / raw)
To: Tomas Vondra <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>
On Tue, Jun 25, 2024 at 6:04 AM Tomas Vondra
<[email protected]> wrote:
> Yeah, definitely needs comment explaining this.
>
> I admit those numbers are pretty arbitrary primes, to implement a
> trivial hash function. That was good enough for a PoC patch, but maybe
> for a "proper" version this should use a better hash function. It needs
> to be fast, and maybe it doesn't matter that much if it's not perfect.
Right. My guess is that if we try too hard to make the hash function
good, there will be a performance hit. Unlike, say, strings that come
from the user, we have no reason to believe that relfilenumbers will
have any particular structure or pattern to them, so a low-quality,
fast function seems like a good trade-off to me. But I'm *far* from a
hashing expert, so I'm prepared for someone who is to tell me that I'm
full of garbage.
> I don't think I've heard the term "partially-associative cache" before
> That's an excellent question. I don't know.
>
> I agree 64 groups is pretty arbitrary, and having 1024 may not be enough
> even with a modest number of partitions. When I was thinking about using
> a higher value, my main concern was that it'd made the PGPROC entry
> larger. Each "fast-path" group is ~72B, so 64 groups is ~4.5kB, and that
> felt like quite a bit.
>
> But maybe it's fine and we could make it much larger - L3 caches tend to
> be many MBs these days, although AFAIK it's shared by threads running on
> the CPU.
>
> I'll see if I can do some more testing of this, and see if there's a
> value where it stops improving / starts degrading, etc.
Sounds good.
--
Robert Haas
EDB: http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: scalability bottlenecks with (many) partitions (and more)
@ 2024-08-05 11:38 Tomas Vondra <[email protected]>
parent: Tomas Vondra <[email protected]>
1 sibling, 1 reply; 10+ messages in thread
From: Tomas Vondra @ 2024-08-05 11:38 UTC (permalink / raw)
To: Robert Haas <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>
Hi,
On 6/25/24 12:04, Tomas Vondra wrote:
>
>
> On 6/24/24 17:05, Robert Haas wrote:
>> On Sun, Jan 28, 2024 at 4:57 PM Tomas Vondra
>> <[email protected]> wrote:
>>> For NUM_LOCK_PARTITIONS this is pretty simple (see 0001 patch). The
>>> LWLock table has 16 partitions by default - it's quite possible that on
>>> machine with many cores and/or many partitions, we can easily hit this.
>>> So I bumped this 4x to 64 partitions.
>>
>> I think this probably makes sense. I'm a little worried that we're
>> just kicking the can down the road here where maybe we should be
>> solving the problem in some more fundamental way, and I'm also a
>> little worried that we might be reducing single-core performance. But
>> it's probably fine.
>>
>
> Yeah, I haven't seen this causing any regressions - the sensitive paths
> typically lock only one partition, so having more of them does not
> affect that. Or if it does, it's likely a reasonable trade off as it
> reduces the risk of lock contention.
>
> That being said, I don't recall benchmarking this patch in isolation,
> only with the other patches. Maybe I should do that ...
>
>>> What I ended up doing is having a hash table of 16-element arrays. There
>>> are 64 "pieces", each essentially the (16 x OID + UINT64 bitmap) that we
>>> have now. Each OID is mapped to exactly one of these parts as if in a
>>> hash table, and in each of those 16-element parts we do exactly the same
>>> thing we do now (linear search, removal, etc.). This works great, the
>>> locality is great, etc. The one disadvantage is this makes PGPROC
>>> larger, but I did a lot of benchmarks and I haven't seen any regression
>>> that I could attribute to this. (More about this later.)
>>
>> I think this is a reasonable approach. Some comments:
>>
>> - FastPathLocalUseInitialized seems unnecessary to me; the contents of
>> an uninitialized local variable are undefined, but an uninitialized
>> global variable always starts out zeroed.
>>
>
> OK. I didn't realize global variables start a zero.
>
I haven't fixed this yet, but it's pretty clear the "init" is not really
needed, because it did the memset() wrong:
memset(FastPathLocalUseCounts, 0, sizeof(FastPathLocalUseInitialized));
This only resets one byte of the array, yet it still worked correctly.
>> - You need comments in various places, including here, where someone
>> is certain to have questions about the algorithm and choice of
>> constants:
>>
>> +#define FAST_PATH_LOCK_REL_GROUP(rel) (((uint64) (rel) * 7883 + 4481)
>> % FP_LOCK_GROUPS_PER_BACKEND)
>>
>
> Yeah, definitely needs comment explaining this.
>
> I admit those numbers are pretty arbitrary primes, to implement a
> trivial hash function. That was good enough for a PoC patch, but maybe
> for a "proper" version this should use a better hash function. It needs
> to be fast, and maybe it doesn't matter that much if it's not perfect.
>
>> When I originally coded up the fast-path locking stuff, I supposed
>> that we couldn't make the number of slots too big because the
>> algorithm requires a linear search of the whole array. But with this
>> one trick (a partially-associative cache), there's no real reason that
>> I can think of why you can't make the number of slots almost
>> arbitrarily large. At some point you're going to use too much memory,
>> and probably before that point you're going to make the cache big
>> enough that it doesn't fit in the CPU cache of an individual core, at
>> which point possibly it will stop working as well. But honestly ... I
>> don't quite see why this approach couldn't be scaled quite far.
>>
>
> I don't think I've heard the term "partially-associative cache" before,
> but now that I look at the approach again, it very much reminds me how
> set-associative caches work (e.g. with cachelines in CPU caches). It's a
> 16-way associative cache, assigning each entry into one of 16 slots.
>
> I must have been reading some papers in this area shortly before the PoC
> patch, and the idea came from there, probably. Which is good, because it
> means it's a well-understood and widely-used approach.
>
>> Like, if we raised FP_LOCK_GROUPS_PER_BACKEND from your proposed value
>> of 64 to say 65536, would that still perform well? I'm not saying we
>> should do that, because that's probably a silly amount of memory to
>> use for this, but the point is that when you start to have enough
>> partitions that you run out of lock slots, performance is going to
>> degrade, so you can imagine wanting to try to have enough lock groups
>> to make that unlikely. Which leads me to wonder if there's any
>> particular number of lock groups that is clearly "too many" or whether
>> it's just about how much memory we want to use.
>>
>
> That's an excellent question. I don't know.
>
> I agree 64 groups is pretty arbitrary, and having 1024 may not be enough
> even with a modest number of partitions. When I was thinking about using
> a higher value, my main concern was that it'd made the PGPROC entry
> larger. Each "fast-path" group is ~72B, so 64 groups is ~4.5kB, and that
> felt like quite a bit.
>
> But maybe it's fine and we could make it much larger - L3 caches tend to
> be many MBs these days, although AFAIK it's shared by threads running on
> the CPU.
>
> I'll see if I can do some more testing of this, and see if there's a
> value where it stops improving / starts degrading, etc.
>
I finally got to do those experiments. The scripts and results (both raw
and summarized) are too big to attach everything here, available at
https://github.com/tvondra/scalability-tests
The initial patch used 64 (which means 1024 fast-path slots), I ran the
tests with 0, 1, 8, 32, 128, 512, 1024 (so up to 16k locks). I thought
about testing with ~64k groups, but I didn't go with the extreme value
because I don't quite see the point.
It would only matter for cases with a truly extreme number of partitions
(64k groups is ~1M fast-path slots), and just creating enough partitions
would take a lot of time. Moreover, with that many partitions we seems
to have various other bottlenecks, and improving this does not make it
really practical. And it's so slow the benchmark results are somewhat
bogus too.
Because if we achieve 50 tps with 1000 partitions, does it really matter
a patch changes that to 25 of 100 tps? I doubt that, especially if going
to 100 partitions gives you 2000 tps. Now imagine you have 10k or 100k
partitions - how fast is that going to be?
So I think stopping at 1024 groups is sensible, and if there are some
inefficiencies / costs, I'd expect those to gradually show up even at
those lower sizes.
But if you look at results, for example from the "join" test:
https://github.com/tvondra/scalability-tests/blob/main/join.pdf
there's no such negative effect. the table shows results for different
combinations of parameters, with the first group of columns being on
regular glibc, the second one has glibc tuning (see [1] for details).
And the values are for different number of fast-path groups (0 means the
patch was not applied).
And the color scale on the show the impact of increasing the number of
groups. So for example when a column for "32 groups" says 150%, it means
going from 8 to 32 groups improved throughput to 1.5x. As usual, green
is "good" and red is "bad".
But if you look at the tables, there's very little change - most of the
values are close to 100%. This might seem a bit strange, considering the
promise of these patches is to improve throughput, and "no change" is an
absence of that. But that's because the charts illustrate effect of
changing the group count with other parameters fixed. It never compares
runs with/without glibc runing, and that's an important part of the
improvement. Doing the pivot table a bit differently would still show a
substantial 2-3x improvement.
There's a fair amount of noise - especially for the rpi5 machines (not
the right hw for sensitive benchmarks), but also on some i5/xeon runs. I
attribute this to only doing one short run (10s) for each combinations
of parameters. I'll do more runs next time.
Anyway, I think these results show a couple things:
1) There's no systemic negative effect of increasing the number of
groups. We could go with 32k or 64k groups, and it doesn't seem like
there would be a problem.
2) But there's not much point in doing that, because we run into various
other bottlenecks well before having that many locks. By the results, it
doesn't seem going beyond 32 or 64 groups would give us much.
3) The memory allocation caching (be it the mempool patch, or the glibc
tuning like in this test round) is a crucial piece for this. Not doing
that means some tests get no improvement at all, or a much smaller one.
4) The increase of NUM_LOCK_PARTITIONS has very limited effect, or
perhaps even no effect at all.
Based on this, my plan is to polish the patch adding fast-path groups,
with either 32 or 64 groups, which seems to be reasonable values. Then
in the future, if/when the other bottlenecks get addressed, we can
rethink and increase this.
This however reminds me that all those machines are pretty small. Which
is good for showing it doesn't break existing/smaller systems, but the
initial goal of the patch was to improve behavior on big boxes. I don't
have access to the original box at the moment, so if someone could
provide an access to one of those big epyc/xeon machines with 100+ cores
for a couple days, that would be helpful.
That being said, I think it's pretty clear how serious the issue with
memory allocation overhead can be, especially in cases when the existing
memory context caching is ineffective (like for the btree palloc). I'm
not sure what to do about that. The mempool patch shared in this thread
does the trick, it's fairly complex/invasive. I still like it, but maybe
doing something with the glibc tuning would be enough - it's not as
effective, but 80% of the improvement is better than no improvement.
regards
[1]
https://www.postgresql.org/message-id/[email protected]
--
Tomas Vondra
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: scalability bottlenecks with (many) partitions (and more)
@ 2024-09-01 19:30 Tomas Vondra <[email protected]>
parent: Tomas Vondra <[email protected]>
0 siblings, 1 reply; 10+ messages in thread
From: Tomas Vondra @ 2024-09-01 19:30 UTC (permalink / raw)
To: Robert Haas <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>; Andres Freund <[email protected]>
Hi,
While discussing this patch with Robert off-list, one of the questions
he asked was is there's some size threshold after which it starts to
have negative impact. I didn't have a good answer to that - I did have
some intuition (that making it too large would not hurt), but I haven't
done any tests with "extreme" sizes of the fast-path structs.
So I ran some more tests, with up to 4096 "groups" (which means 64k
fast-path slots). And no matter how I slice the results, there's no
clear regression points, beyond which the performance would start to
decline (even just slowly). It's the same for all benchmarks, client
counts, query mode, and so on.
I'm attaching two PDFs with results for the "join" benchmark I described
earlier (query with a join on many partitions) from EPYC 7763 (64/128c).
The first one is with "raw" data (throughput = tps), the second one is
relative throughput to the first column (which is pretty much current
master, with no optimizations applied).
The complete results including some nice .odp spreadsheets and scripts
are available here:
https://github.com/tvondra/pg-lock-scalability-results
There's often a very clear point where the performance significantly
improves - this is usually when all the relation locks start to fit into
the fast-path array. With 1000 relations that's ~64 groups, and so on.
But there's no point where it would start declining.
My explanation is that the PGPROC (where the fast-path array is) is so
large already (close to 1kB), that making it large does not really cause
any additional cache misses, etc. And if it does, it's far out-weighted
by cost of accessing (or not having to) the shared lock table.
So I don't think there's any point at which point we'd start to regress,
at least not because of cache misses, CPU etc. It stops improving, but
that's just a sign that we've hit some other bottleneck - that's not a
fault of this patch.
But that's not the whole story, of course. Because if there were no
issues, why not to just make the fast-path array insanely large? In
another off-list discussion Andres asked me about the memory this would
need, and after looking at the numbers I think that's a strong argument
to keep the numbers reasonable.
I did a quick experiment to see the per-connection memory requirements,
and how would it be affected by this patch. I simply logged the amount
of shared memory CalculateShmemSize(), started the server with 100 and
1000 connections, and did a bit of math to calculate how much memory we
need "per connection".
For master and different numbers of fast-path groups I got this:
master 64 1024 32765
---------------------------------
47668 52201 121324 2406892
So by default we need ~48kB / connection, with 64 groups we need ~52kB
(which makes sense because that's 1024 x 4B slots), and then with 1024
slots we get to 120kB etc and with 32k ~2.5MB.
I guess those higher values seem a bit insane - we don't want to just
increase the per-connection memory requirements 50x for everyone, right?
But what about the people who actually want this many locks? Let's bump
the max_locks_per_transactions from 64 to 1024, and we get this:
master 64 1024 32765
-------------------------------------
419367 423909 493022 2778590
Suddenly, the differences are much smaller, especially for the 64
groups, which is roughly the same number of fast-path slots as the max
locks per transactions. That shrunk to ~1% difference. But wen for 1024
groups it's now just ~20%, which I think it well worth the benefits.
And likely something the system should have available - with 1000
connections that's ~80MB. And if you run with 1000 connections, 80MB
should be rounding error, IMO.
Of course, it does not seem great to force everyone to pay this price,
even if they don't need that many locks (and so there's no benefit). So
how would we improve that?
I don't think that's possible with hard-coded size of the array - that
allocates the memory for everyone. We'd need to make it variable-length,
and while doing those benchmarks I think we actually already have a GUC
for that - max_locks_per_transaction tells us exactly what we need to
know, right? I mean, if I know I'll need ~1000 locks, why not to make
the fast-path array large enough for that?
Of course, the consequence of this would be making PGPROC variable
length, or having to point to a memory allocated separately (I prefer
the latter option, I think). I haven't done any experiments, but it
seems fairly doable - of course, not sure if it might be more expensive
compared to compile-time constants.
At this point I think it's fairly clear we have significant bottlenecks
when having to lock many relations - and that won't go away, thanks to
partitioning etc. We're already fixing various other bottlenecks for
these workloads, which will just increase pressure on locking.
Fundamentally, I think we'll need to either evolve the fast-path system
to handle more relations (the limit of 16 was always rather quite low),
or invent some entirely new thing that does something radical (say,
locking a "group" of relations instead of locking them one by one).
This patch is doing the first thing, and IMHO the increased memory
consumption is a sensible / acceptable trade off. I'm not sure of any
proposal for the second approach, and I don't have any concrete idea how
it might work.
regards
--
Tomas Vondra
Attachments:
[application/pdf] join-epyc-relative.pdf (50.4K, ../../[email protected]/2-join-epyc-relative.pdf)
download
[application/pdf] join-epyc-data.pdf (40.1K, ../../[email protected]/3-join-epyc-data.pdf)
download
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: scalability bottlenecks with (many) partitions (and more)
@ 2024-09-01 23:53 Robert Haas <[email protected]>
parent: Tomas Vondra <[email protected]>
0 siblings, 1 reply; 10+ messages in thread
From: Robert Haas @ 2024-09-01 23:53 UTC (permalink / raw)
To: Tomas Vondra <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>; Andres Freund <[email protected]>
On Sun, Sep 1, 2024 at 3:30 PM Tomas Vondra <[email protected]> wrote:
> I don't think that's possible with hard-coded size of the array - that
> allocates the memory for everyone. We'd need to make it variable-length,
> and while doing those benchmarks I think we actually already have a GUC
> for that - max_locks_per_transaction tells us exactly what we need to
> know, right? I mean, if I know I'll need ~1000 locks, why not to make
> the fast-path array large enough for that?
I really like this idea. I'm not sure about exactly how many fast path
slots you should get for what value of max_locks_per_transaction, but
coupling the two things together in some way sounds smart.
> Of course, the consequence of this would be making PGPROC variable
> length, or having to point to a memory allocated separately (I prefer
> the latter option, I think). I haven't done any experiments, but it
> seems fairly doable - of course, not sure if it might be more expensive
> compared to compile-time constants.
I agree that this is a potential problem but it sounds like the idea
works well enough that we'd probably still come out quite far ahead
even with a bit more overhead.
--
Robert Haas
EDB: http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: scalability bottlenecks with (many) partitions (and more)
@ 2024-09-02 17:46 Tomas Vondra <[email protected]>
parent: Robert Haas <[email protected]>
0 siblings, 1 reply; 10+ messages in thread
From: Tomas Vondra @ 2024-09-02 17:46 UTC (permalink / raw)
To: Robert Haas <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>; Andres Freund <[email protected]>
On 9/2/24 01:53, Robert Haas wrote:
> On Sun, Sep 1, 2024 at 3:30 PM Tomas Vondra <[email protected]> wrote:
>> I don't think that's possible with hard-coded size of the array - that
>> allocates the memory for everyone. We'd need to make it variable-length,
>> and while doing those benchmarks I think we actually already have a GUC
>> for that - max_locks_per_transaction tells us exactly what we need to
>> know, right? I mean, if I know I'll need ~1000 locks, why not to make
>> the fast-path array large enough for that?
>
> I really like this idea. I'm not sure about exactly how many fast path
> slots you should get for what value of max_locks_per_transaction, but
> coupling the two things together in some way sounds smart.
>
I think we should keep that simple and make the cache large enough for
max_locks_per_transaction locks. That's the best information about
expected number of locks we have. If the GUC is left at the default
value, that probably means they backends need that many locks on
average. Yes, maybe there's an occasional spike in one of the backends,
but then that means other backends need fewer locks, and so there's less
contention for the shared lock table.
Of course, it's possible to construct counter-examples to this. Say a
single backend that needs a lot of these locks. But how's that different
from every other fixed-size cache with eviction?
The one argument to not tie this to max_locks_per_transaction is the
vastly different "per element" memory requirements. If you add one entry
to max_locks_per_transaction, that adds LOCK which is a whopping 152B.
OTOH one fast-path entry is ~5B, give or take. That's a pretty big
difference, and it if the locks fit into the shared lock table, but
you'd like to allow more fast-path locks, having to increase
max_locks_per_transaction is not great - pretty wastefull.
OTOH I'd really hate to just add another GUC and hope the users will
magically know how to set it correctly. That's pretty unlikely, IMO. I
myself wouldn't know what a good value is, I think.
But say we add a GUC and set it to -1 by default, in which case it just
inherits the max_locks_per_transaction value. And then also provide some
basic metric about this fast-path cache, so that people can tune this?
I think just knowing the "hit ratio" would be enough, i.e. counters for
how often it fits into the fast-path array, and how often we had to
promote it to the shared lock table would be enough, no?
>> Of course, the consequence of this would be making PGPROC variable
>> length, or having to point to a memory allocated separately (I prefer
>> the latter option, I think). I haven't done any experiments, but it
>> seems fairly doable - of course, not sure if it might be more expensive
>> compared to compile-time constants.
>
> I agree that this is a potential problem but it sounds like the idea
> works well enough that we'd probably still come out quite far ahead
> even with a bit more overhead.
>
OK, I did some quick tests on this, and I don't see any regressions.
Attached are 4 patches:
1) 0001 - original patch, with some minor fixes (remove init, which is
not necessary, that sort of thing)
2) 0002 - a bit of reworks, improving comments, structuring the macros a
little bit better, etc. But still compile-time constants.
3) 0003 - dynamic sizing, based on max_locks_pet_transaction. It's a bit
ugly, because the size is calculated during shmem allocation - it
should happen earlier, but good enough for PoC.
4) 0004 - introduce a separate GUC, this is mostly to allow testing of
different values without changing max_locks_per_transaction
I've only did that on my smaller 32-core machine, but for three simple
tests it looks like this (throughput using 16 clients):
mode test master 1 2 3 4
----------------------------------------------------------------
prepared count 1460 1477 1488 1490 1491
join 15556 24451 26044 25026 24237
pgbench 148187 151192 151688 150389 152681
----------------------------------------------------------------
simple count 1341 1351 1373 1374 1370
join 4643 5439 5459 5393 5345
pgbench 139763 141267 142796 141207 142600
Those are some simple benchmarks on 100 partitions, where the regular
pgbench and count(*) are expected to not be improved, and the join is
the partitioned join this thread started with. 1-4 are the attached
patches, to see the impact for each of them.
Translated to results relative to
mode test 1 2 3 4
-------------------------------------------------
prepared count 101% 102% 102% 102%
join 157% 167% 161% 156%
pgbench 102% 102% 101% 103%
-------------------------------------------------
simple count 101% 102% 102% 102%
join 117% 118% 116% 115%
pgbench 101% 102% 101% 102%
So pretty much no difference between the patches. A bit of noise, but
that's what I'd expect on this machine.
I'll do more testing on the bit EPYC machine once it gets available, but
from these results it seems pretty promising.
regards
--
Tomas Vondra
Attachments:
[text/x-patch] v20240902-0001-v1.patch (10.5K, ../../[email protected]/2-v20240902-0001-v1.patch)
download | inline diff:
From a145f2f14fc4a995953563c5adfc72f365dbad8a Mon Sep 17 00:00:00 2001
From: Tomas Vondra <[email protected]>
Date: Mon, 2 Sep 2024 00:55:13 +0200
Subject: [PATCH v20240902 1/4] v1
---
src/backend/storage/lmgr/lock.c | 97 +++++++++++++++++++++++++--------
src/include/storage/proc.h | 9 +--
2 files changed, 79 insertions(+), 27 deletions(-)
diff --git a/src/backend/storage/lmgr/lock.c b/src/backend/storage/lmgr/lock.c
index 6dbc41dae70..78e152a0b36 100644
--- a/src/backend/storage/lmgr/lock.c
+++ b/src/backend/storage/lmgr/lock.c
@@ -167,7 +167,7 @@ typedef struct TwoPhaseLockRecord
* our locks to the primary lock table, but it can never be lower than the
* real value, since only we can acquire locks on our own behalf.
*/
-static int FastPathLocalUseCount = 0;
+static int FastPathLocalUseCounts[FP_LOCK_GROUPS_PER_BACKEND];
/*
* Flag to indicate if the relation extension lock is held by this backend.
@@ -187,20 +187,23 @@ static bool IsRelationExtensionLockHeld PG_USED_FOR_ASSERTS_ONLY = false;
/* Macros for manipulating proc->fpLockBits */
#define FAST_PATH_BITS_PER_SLOT 3
#define FAST_PATH_LOCKNUMBER_OFFSET 1
+#define FAST_PATH_LOCK_REL_GROUP(rel) (((uint64) (rel) * 7883 + 4481) % FP_LOCK_GROUPS_PER_BACKEND)
+#define FAST_PATH_LOCK_INDEX(n) ((n) % FP_LOCK_SLOTS_PER_GROUP)
+#define FAST_PATH_LOCK_GROUP(n) ((n) / FP_LOCK_SLOTS_PER_GROUP)
#define FAST_PATH_MASK ((1 << FAST_PATH_BITS_PER_SLOT) - 1)
#define FAST_PATH_GET_BITS(proc, n) \
- (((proc)->fpLockBits >> (FAST_PATH_BITS_PER_SLOT * n)) & FAST_PATH_MASK)
+ (((proc)->fpLockBits[(n)/16] >> (FAST_PATH_BITS_PER_SLOT * FAST_PATH_LOCK_INDEX(n))) & FAST_PATH_MASK)
#define FAST_PATH_BIT_POSITION(n, l) \
(AssertMacro((l) >= FAST_PATH_LOCKNUMBER_OFFSET), \
AssertMacro((l) < FAST_PATH_BITS_PER_SLOT+FAST_PATH_LOCKNUMBER_OFFSET), \
- AssertMacro((n) < FP_LOCK_SLOTS_PER_BACKEND), \
- ((l) - FAST_PATH_LOCKNUMBER_OFFSET + FAST_PATH_BITS_PER_SLOT * (n)))
+ AssertMacro((n) < FP_LOCKS_PER_BACKEND), \
+ ((l) - FAST_PATH_LOCKNUMBER_OFFSET + FAST_PATH_BITS_PER_SLOT * (FAST_PATH_LOCK_INDEX(n))))
#define FAST_PATH_SET_LOCKMODE(proc, n, l) \
- (proc)->fpLockBits |= UINT64CONST(1) << FAST_PATH_BIT_POSITION(n, l)
+ (proc)->fpLockBits[FAST_PATH_LOCK_GROUP(n)] |= UINT64CONST(1) << FAST_PATH_BIT_POSITION(n, l)
#define FAST_PATH_CLEAR_LOCKMODE(proc, n, l) \
- (proc)->fpLockBits &= ~(UINT64CONST(1) << FAST_PATH_BIT_POSITION(n, l))
+ (proc)->fpLockBits[FAST_PATH_LOCK_GROUP(n)] &= ~(UINT64CONST(1) << FAST_PATH_BIT_POSITION(n, l))
#define FAST_PATH_CHECK_LOCKMODE(proc, n, l) \
- ((proc)->fpLockBits & (UINT64CONST(1) << FAST_PATH_BIT_POSITION(n, l)))
+ ((proc)->fpLockBits[FAST_PATH_LOCK_GROUP(n)] & (UINT64CONST(1) << FAST_PATH_BIT_POSITION(n, l)))
/*
* The fast-path lock mechanism is concerned only with relation locks on
@@ -926,7 +929,7 @@ LockAcquireExtended(const LOCKTAG *locktag,
* for now we don't worry about that case either.
*/
if (EligibleForRelationFastPath(locktag, lockmode) &&
- FastPathLocalUseCount < FP_LOCK_SLOTS_PER_BACKEND)
+ FastPathLocalUseCounts[FAST_PATH_LOCK_REL_GROUP(locktag->locktag_field2)] < FP_LOCK_SLOTS_PER_GROUP)
{
uint32 fasthashcode = FastPathStrongLockHashPartition(hashcode);
bool acquired;
@@ -1970,6 +1973,7 @@ LockRelease(const LOCKTAG *locktag, LOCKMODE lockmode, bool sessionLock)
PROCLOCK *proclock;
LWLock *partitionLock;
bool wakeupNeeded;
+ int group;
if (lockmethodid <= 0 || lockmethodid >= lengthof(LockMethods))
elog(ERROR, "unrecognized lock method: %d", lockmethodid);
@@ -2063,9 +2067,14 @@ LockRelease(const LOCKTAG *locktag, LOCKMODE lockmode, bool sessionLock)
*/
locallock->lockCleared = false;
+ /* Which FP group does the lock belong to? */
+ group = FAST_PATH_LOCK_REL_GROUP(locktag->locktag_field2);
+
+ Assert(group >= 0 && group < FP_LOCK_GROUPS_PER_BACKEND);
+
/* Attempt fast release of any lock eligible for the fast path. */
if (EligibleForRelationFastPath(locktag, lockmode) &&
- FastPathLocalUseCount > 0)
+ FastPathLocalUseCounts[group] > 0)
{
bool released;
@@ -2633,12 +2642,21 @@ LockReassignOwner(LOCALLOCK *locallock, ResourceOwner parent)
static bool
FastPathGrantRelationLock(Oid relid, LOCKMODE lockmode)
{
+ uint32 i;
uint32 f;
- uint32 unused_slot = FP_LOCK_SLOTS_PER_BACKEND;
+ uint32 unused_slot = FP_LOCKS_PER_BACKEND;
+
+ int group = FAST_PATH_LOCK_REL_GROUP(relid);
+
+ Assert(group >= 0 && group < FP_LOCK_GROUPS_PER_BACKEND);
/* Scan for existing entry for this relid, remembering empty slot. */
- for (f = 0; f < FP_LOCK_SLOTS_PER_BACKEND; f++)
+ for (i = 0; i < FP_LOCK_SLOTS_PER_GROUP; i++)
{
+ f = group * FP_LOCK_SLOTS_PER_GROUP + i;
+
+ Assert(f >= 0 && f < FP_LOCKS_PER_BACKEND);
+
if (FAST_PATH_GET_BITS(MyProc, f) == 0)
unused_slot = f;
else if (MyProc->fpRelId[f] == relid)
@@ -2650,11 +2668,11 @@ FastPathGrantRelationLock(Oid relid, LOCKMODE lockmode)
}
/* If no existing entry, use any empty slot. */
- if (unused_slot < FP_LOCK_SLOTS_PER_BACKEND)
+ if (unused_slot < FP_LOCKS_PER_BACKEND)
{
MyProc->fpRelId[unused_slot] = relid;
FAST_PATH_SET_LOCKMODE(MyProc, unused_slot, lockmode);
- ++FastPathLocalUseCount;
+ ++FastPathLocalUseCounts[group];
return true;
}
@@ -2670,12 +2688,21 @@ FastPathGrantRelationLock(Oid relid, LOCKMODE lockmode)
static bool
FastPathUnGrantRelationLock(Oid relid, LOCKMODE lockmode)
{
+ uint32 i;
uint32 f;
bool result = false;
- FastPathLocalUseCount = 0;
- for (f = 0; f < FP_LOCK_SLOTS_PER_BACKEND; f++)
+ int group = FAST_PATH_LOCK_REL_GROUP(relid);
+
+ Assert(group >= 0 && group < FP_LOCK_GROUPS_PER_BACKEND);
+
+ FastPathLocalUseCounts[group] = 0;
+ for (i = 0; i < FP_LOCK_SLOTS_PER_GROUP; i++)
{
+ f = group * FP_LOCK_SLOTS_PER_GROUP + i;
+
+ Assert(f >= 0 && f < FP_LOCKS_PER_BACKEND);
+
if (MyProc->fpRelId[f] == relid
&& FAST_PATH_CHECK_LOCKMODE(MyProc, f, lockmode))
{
@@ -2685,7 +2712,7 @@ FastPathUnGrantRelationLock(Oid relid, LOCKMODE lockmode)
/* we continue iterating so as to update FastPathLocalUseCount */
}
if (FAST_PATH_GET_BITS(MyProc, f) != 0)
- ++FastPathLocalUseCount;
+ ++FastPathLocalUseCounts[group];
}
return result;
}
@@ -2703,7 +2730,7 @@ FastPathTransferRelationLocks(LockMethod lockMethodTable, const LOCKTAG *locktag
{
LWLock *partitionLock = LockHashPartitionLock(hashcode);
Oid relid = locktag->locktag_field2;
- uint32 i;
+ uint32 i, j, group;
/*
* Every PGPROC that can potentially hold a fast-path lock is present in
@@ -2739,10 +2766,18 @@ FastPathTransferRelationLocks(LockMethod lockMethodTable, const LOCKTAG *locktag
continue;
}
- for (f = 0; f < FP_LOCK_SLOTS_PER_BACKEND; f++)
+ group = FAST_PATH_LOCK_REL_GROUP(relid);
+
+ Assert(group >= 0 && group < FP_LOCK_GROUPS_PER_BACKEND);
+
+ for (j = 0; j < FP_LOCK_SLOTS_PER_GROUP; j++)
{
uint32 lockmode;
+ f = group * FP_LOCK_SLOTS_PER_GROUP + j;
+
+ Assert(f >= 0 && f < FP_LOCKS_PER_BACKEND);
+
/* Look for an allocated slot matching the given relid. */
if (relid != proc->fpRelId[f] || FAST_PATH_GET_BITS(proc, f) == 0)
continue;
@@ -2793,14 +2828,22 @@ FastPathGetRelationLockEntry(LOCALLOCK *locallock)
PROCLOCK *proclock = NULL;
LWLock *partitionLock = LockHashPartitionLock(locallock->hashcode);
Oid relid = locktag->locktag_field2;
- uint32 f;
+ uint32 f, i;
+
+ int group = FAST_PATH_LOCK_REL_GROUP(relid);
+
+ Assert(group >= 0 && group < FP_LOCK_GROUPS_PER_BACKEND);
LWLockAcquire(&MyProc->fpInfoLock, LW_EXCLUSIVE);
- for (f = 0; f < FP_LOCK_SLOTS_PER_BACKEND; f++)
+ for (i = 0; i < FP_LOCK_SLOTS_PER_GROUP; i++)
{
uint32 lockmode;
+ f = group * FP_LOCK_SLOTS_PER_GROUP + i;
+
+ Assert(f >= 0 && f < FP_LOCKS_PER_BACKEND);
+
/* Look for an allocated slot matching the given relid. */
if (relid != MyProc->fpRelId[f] || FAST_PATH_GET_BITS(MyProc, f) == 0)
continue;
@@ -2904,6 +2947,10 @@ GetLockConflicts(const LOCKTAG *locktag, LOCKMODE lockmode, int *countp)
int count = 0;
int fast_count = 0;
+ int group = FAST_PATH_LOCK_REL_GROUP(locktag->locktag_field2);
+
+ Assert(group >= 0 && group < FP_LOCK_GROUPS_PER_BACKEND);
+
if (lockmethodid <= 0 || lockmethodid >= lengthof(LockMethods))
elog(ERROR, "unrecognized lock method: %d", lockmethodid);
lockMethodTable = LockMethods[lockmethodid];
@@ -2940,7 +2987,7 @@ GetLockConflicts(const LOCKTAG *locktag, LOCKMODE lockmode, int *countp)
*/
if (ConflictsWithRelationFastPath(locktag, lockmode))
{
- int i;
+ int i, j;
Oid relid = locktag->locktag_field2;
VirtualTransactionId vxid;
@@ -2979,10 +3026,14 @@ GetLockConflicts(const LOCKTAG *locktag, LOCKMODE lockmode, int *countp)
continue;
}
- for (f = 0; f < FP_LOCK_SLOTS_PER_BACKEND; f++)
+ for (j = 0; j < FP_LOCK_SLOTS_PER_GROUP; j++)
{
uint32 lockmask;
+ f = group * FP_LOCK_SLOTS_PER_GROUP + j;
+
+ Assert(f >= 0 && f < FP_LOCKS_PER_BACKEND);
+
/* Look for an allocated slot matching the given relid. */
if (relid != proc->fpRelId[f])
continue;
@@ -3642,7 +3693,7 @@ GetLockStatusData(void)
LWLockAcquire(&proc->fpInfoLock, LW_SHARED);
- for (f = 0; f < FP_LOCK_SLOTS_PER_BACKEND; ++f)
+ for (f = 0; f < FP_LOCKS_PER_BACKEND; ++f)
{
LockInstanceData *instance;
uint32 lockbits = FAST_PATH_GET_BITS(proc, f);
diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h
index deeb06c9e01..f074266a48c 100644
--- a/src/include/storage/proc.h
+++ b/src/include/storage/proc.h
@@ -83,8 +83,9 @@ struct XidCache
* rather than the main lock table. This eases contention on the lock
* manager LWLocks. See storage/lmgr/README for additional details.
*/
-#define FP_LOCK_SLOTS_PER_BACKEND 16
-
+#define FP_LOCK_GROUPS_PER_BACKEND 64
+#define FP_LOCK_SLOTS_PER_GROUP 16 /* don't change */
+#define FP_LOCKS_PER_BACKEND (FP_LOCK_SLOTS_PER_GROUP * FP_LOCK_GROUPS_PER_BACKEND)
/*
* Flags for PGPROC.delayChkptFlags
*
@@ -292,8 +293,8 @@ struct PGPROC
/* Lock manager data, recording fast-path locks taken by this backend. */
LWLock fpInfoLock; /* protects per-backend fast-path state */
- uint64 fpLockBits; /* lock modes held for each fast-path slot */
- Oid fpRelId[FP_LOCK_SLOTS_PER_BACKEND]; /* slots for rel oids */
+ uint64 fpLockBits[FP_LOCK_GROUPS_PER_BACKEND]; /* lock modes held for each fast-path slot */
+ Oid fpRelId[FP_LOCKS_PER_BACKEND]; /* slots for rel oids */
bool fpVXIDLock; /* are we holding a fast-path VXID lock? */
LocalTransactionId fpLocalTransactionId; /* lxid for fast-path VXID
* lock */
--
2.46.0
[text/x-patch] v20240902-0002-rework.patch (11.1K, ../../[email protected]/3-v20240902-0002-rework.patch)
download | inline diff:
From 16abf609b3b988e27abc1cf4c48e5949e29cd344 Mon Sep 17 00:00:00 2001
From: Tomas Vondra <[email protected]>
Date: Mon, 2 Sep 2024 15:37:41 +0200
Subject: [PATCH v20240902 2/4] rework
---
src/backend/storage/lmgr/lock.c | 125 +++++++++++++++++++++++---------
src/include/storage/proc.h | 4 +-
2 files changed, 92 insertions(+), 37 deletions(-)
diff --git a/src/backend/storage/lmgr/lock.c b/src/backend/storage/lmgr/lock.c
index 78e152a0b36..524aee863fd 100644
--- a/src/backend/storage/lmgr/lock.c
+++ b/src/backend/storage/lmgr/lock.c
@@ -184,19 +184,49 @@ static int FastPathLocalUseCounts[FP_LOCK_GROUPS_PER_BACKEND];
*/
static bool IsRelationExtensionLockHeld PG_USED_FOR_ASSERTS_ONLY = false;
+/*
+ * Macros to calculate the group and index for a relation.
+ *
+ * The formula is a simple hash function, designed to spread the OIDs a bit,
+ * so that even contiguous values end up in different groups. In most cases
+ * there will be gaps anyway, but the multiplication should help a bit.
+ *
+ * The selected value (49157) is a prime not too close to 2^k, and it's
+ * small enough to not cause overflows (in 64-bit).
+ *
+ * XXX Maybe it'd be easier / cheaper to just do this in 32-bits? If we
+ * did (rel % 100000) or something like that first, that'd be enough to
+ * not wrap around. But even if it wrapped, would that be a problem?
+ */
+#define FAST_PATH_LOCK_REL_GROUP(rel) (((uint64) (rel) * 49157) % FP_LOCK_GROUPS_PER_BACKEND)
+
+/*
+ * Given a lock index (into the per-backend array), calculated using the
+ * FP_LOCK_SLOT_INDEX macro, calculate group and index (within the group).
+ */
+#define FAST_PATH_LOCK_GROUP(index) \
+ (AssertMacro(((index) >= 0) && ((index) < FP_LOCK_SLOTS_PER_BACKEND)), \
+ ((index) / FP_LOCK_SLOTS_PER_GROUP))
+#define FAST_PATH_LOCK_INDEX(index) \
+ (AssertMacro(((index) >= 0) && ((index) < FP_LOCK_SLOTS_PER_BACKEND)), \
+ ((index) % FP_LOCK_SLOTS_PER_GROUP))
+
+/* Calculate index in the whole per-backend array of lock slots. */
+#define FP_LOCK_SLOT_INDEX(group, index) \
+ (AssertMacro(((group) >= 0) && ((group) < FP_LOCK_GROUPS_PER_BACKEND)), \
+ AssertMacro(((index) >= 0) && ((index) < FP_LOCK_SLOTS_PER_GROUP)), \
+ ((group) * FP_LOCK_SLOTS_PER_GROUP + (index)))
+
/* Macros for manipulating proc->fpLockBits */
#define FAST_PATH_BITS_PER_SLOT 3
#define FAST_PATH_LOCKNUMBER_OFFSET 1
-#define FAST_PATH_LOCK_REL_GROUP(rel) (((uint64) (rel) * 7883 + 4481) % FP_LOCK_GROUPS_PER_BACKEND)
-#define FAST_PATH_LOCK_INDEX(n) ((n) % FP_LOCK_SLOTS_PER_GROUP)
-#define FAST_PATH_LOCK_GROUP(n) ((n) / FP_LOCK_SLOTS_PER_GROUP)
#define FAST_PATH_MASK ((1 << FAST_PATH_BITS_PER_SLOT) - 1)
#define FAST_PATH_GET_BITS(proc, n) \
(((proc)->fpLockBits[(n)/16] >> (FAST_PATH_BITS_PER_SLOT * FAST_PATH_LOCK_INDEX(n))) & FAST_PATH_MASK)
#define FAST_PATH_BIT_POSITION(n, l) \
(AssertMacro((l) >= FAST_PATH_LOCKNUMBER_OFFSET), \
AssertMacro((l) < FAST_PATH_BITS_PER_SLOT+FAST_PATH_LOCKNUMBER_OFFSET), \
- AssertMacro((n) < FP_LOCKS_PER_BACKEND), \
+ AssertMacro((n) < FP_LOCK_SLOTS_PER_BACKEND), \
((l) - FAST_PATH_LOCKNUMBER_OFFSET + FAST_PATH_BITS_PER_SLOT * (FAST_PATH_LOCK_INDEX(n))))
#define FAST_PATH_SET_LOCKMODE(proc, n, l) \
(proc)->fpLockBits[FAST_PATH_LOCK_GROUP(n)] |= UINT64CONST(1) << FAST_PATH_BIT_POSITION(n, l)
@@ -2642,20 +2672,25 @@ LockReassignOwner(LOCALLOCK *locallock, ResourceOwner parent)
static bool
FastPathGrantRelationLock(Oid relid, LOCKMODE lockmode)
{
- uint32 i;
- uint32 f;
- uint32 unused_slot = FP_LOCKS_PER_BACKEND;
+ uint32 unused_slot = FP_LOCK_SLOTS_PER_BACKEND;
+ uint32 i,
+ group;
- int group = FAST_PATH_LOCK_REL_GROUP(relid);
+ /* Which FP group does the lock belong to? */
+ group = FAST_PATH_LOCK_REL_GROUP(relid);
- Assert(group >= 0 && group < FP_LOCK_GROUPS_PER_BACKEND);
+ Assert(group < FP_LOCK_GROUPS_PER_BACKEND);
/* Scan for existing entry for this relid, remembering empty slot. */
for (i = 0; i < FP_LOCK_SLOTS_PER_GROUP; i++)
{
- f = group * FP_LOCK_SLOTS_PER_GROUP + i;
+ uint32 f;
+
+ /* index into the whole per-backend array */
+ f = FP_LOCK_SLOT_INDEX(group, i);
- Assert(f >= 0 && f < FP_LOCKS_PER_BACKEND);
+ /* must not overflow the array of all locks for a backend */
+ Assert(f < FP_LOCK_SLOTS_PER_BACKEND);
if (FAST_PATH_GET_BITS(MyProc, f) == 0)
unused_slot = f;
@@ -2668,7 +2703,7 @@ FastPathGrantRelationLock(Oid relid, LOCKMODE lockmode)
}
/* If no existing entry, use any empty slot. */
- if (unused_slot < FP_LOCKS_PER_BACKEND)
+ if (unused_slot < FP_LOCK_SLOTS_PER_BACKEND)
{
MyProc->fpRelId[unused_slot] = relid;
FAST_PATH_SET_LOCKMODE(MyProc, unused_slot, lockmode);
@@ -2688,20 +2723,25 @@ FastPathGrantRelationLock(Oid relid, LOCKMODE lockmode)
static bool
FastPathUnGrantRelationLock(Oid relid, LOCKMODE lockmode)
{
- uint32 i;
- uint32 f;
bool result = false;
+ uint32 i,
+ group;
- int group = FAST_PATH_LOCK_REL_GROUP(relid);
+ /* Which FP group does the lock belong to? */
+ group = FAST_PATH_LOCK_REL_GROUP(relid);
- Assert(group >= 0 && group < FP_LOCK_GROUPS_PER_BACKEND);
+ Assert(group < FP_LOCK_GROUPS_PER_BACKEND);
FastPathLocalUseCounts[group] = 0;
for (i = 0; i < FP_LOCK_SLOTS_PER_GROUP; i++)
{
- f = group * FP_LOCK_SLOTS_PER_GROUP + i;
+ uint32 f;
+
+ /* index into the whole per-backend array */
+ f = FP_LOCK_SLOT_INDEX(group, i);
- Assert(f >= 0 && f < FP_LOCKS_PER_BACKEND);
+ /* must not overflow the array of all locks for a backend */
+ Assert(f < FP_LOCK_SLOTS_PER_BACKEND);
if (MyProc->fpRelId[f] == relid
&& FAST_PATH_CHECK_LOCKMODE(MyProc, f, lockmode))
@@ -2730,7 +2770,7 @@ FastPathTransferRelationLocks(LockMethod lockMethodTable, const LOCKTAG *locktag
{
LWLock *partitionLock = LockHashPartitionLock(hashcode);
Oid relid = locktag->locktag_field2;
- uint32 i, j, group;
+ uint32 i;
/*
* Every PGPROC that can potentially hold a fast-path lock is present in
@@ -2741,7 +2781,8 @@ FastPathTransferRelationLocks(LockMethod lockMethodTable, const LOCKTAG *locktag
for (i = 0; i < ProcGlobal->allProcCount; i++)
{
PGPROC *proc = &ProcGlobal->allProcs[i];
- uint32 f;
+ uint32 j,
+ group;
LWLockAcquire(&proc->fpInfoLock, LW_EXCLUSIVE);
@@ -2766,17 +2807,21 @@ FastPathTransferRelationLocks(LockMethod lockMethodTable, const LOCKTAG *locktag
continue;
}
+ /* Which FP group does the lock belong to? */
group = FAST_PATH_LOCK_REL_GROUP(relid);
- Assert(group >= 0 && group < FP_LOCK_GROUPS_PER_BACKEND);
+ Assert(group < FP_LOCK_GROUPS_PER_BACKEND);
for (j = 0; j < FP_LOCK_SLOTS_PER_GROUP; j++)
{
uint32 lockmode;
+ uint32 f;
- f = group * FP_LOCK_SLOTS_PER_GROUP + j;
+ /* index into the whole per-backend array */
+ f = FP_LOCK_SLOT_INDEX(group, j);
- Assert(f >= 0 && f < FP_LOCKS_PER_BACKEND);
+ /* must not overflow the array of all locks for a backend */
+ Assert(f < FP_LOCK_SLOTS_PER_BACKEND);
/* Look for an allocated slot matching the given relid. */
if (relid != proc->fpRelId[f] || FAST_PATH_GET_BITS(proc, f) == 0)
@@ -2828,21 +2873,26 @@ FastPathGetRelationLockEntry(LOCALLOCK *locallock)
PROCLOCK *proclock = NULL;
LWLock *partitionLock = LockHashPartitionLock(locallock->hashcode);
Oid relid = locktag->locktag_field2;
- uint32 f, i;
+ uint32 i,
+ group;
- int group = FAST_PATH_LOCK_REL_GROUP(relid);
+ /* Which FP group does the lock belong to? */
+ group = FAST_PATH_LOCK_REL_GROUP(relid);
- Assert(group >= 0 && group < FP_LOCK_GROUPS_PER_BACKEND);
+ Assert(group < FP_LOCK_GROUPS_PER_BACKEND);
LWLockAcquire(&MyProc->fpInfoLock, LW_EXCLUSIVE);
for (i = 0; i < FP_LOCK_SLOTS_PER_GROUP; i++)
{
uint32 lockmode;
+ uint32 f;
- f = group * FP_LOCK_SLOTS_PER_GROUP + i;
+ /* index into the whole per-backend array */
+ f = FP_LOCK_SLOT_INDEX(group, i);
- Assert(f >= 0 && f < FP_LOCKS_PER_BACKEND);
+ /* must not overflow the array of all locks for a backend */
+ Assert(f < FP_LOCK_SLOTS_PER_BACKEND);
/* Look for an allocated slot matching the given relid. */
if (relid != MyProc->fpRelId[f] || FAST_PATH_GET_BITS(MyProc, f) == 0)
@@ -2946,10 +2996,12 @@ GetLockConflicts(const LOCKTAG *locktag, LOCKMODE lockmode, int *countp)
LWLock *partitionLock;
int count = 0;
int fast_count = 0;
+ uint32 group;
- int group = FAST_PATH_LOCK_REL_GROUP(locktag->locktag_field2);
+ /* Which FP group does the lock belong to? */
+ group = FAST_PATH_LOCK_REL_GROUP(locktag->locktag_field2);
- Assert(group >= 0 && group < FP_LOCK_GROUPS_PER_BACKEND);
+ Assert(group < FP_LOCK_GROUPS_PER_BACKEND);
if (lockmethodid <= 0 || lockmethodid >= lengthof(LockMethods))
elog(ERROR, "unrecognized lock method: %d", lockmethodid);
@@ -2987,7 +3039,7 @@ GetLockConflicts(const LOCKTAG *locktag, LOCKMODE lockmode, int *countp)
*/
if (ConflictsWithRelationFastPath(locktag, lockmode))
{
- int i, j;
+ int i;
Oid relid = locktag->locktag_field2;
VirtualTransactionId vxid;
@@ -3004,7 +3056,7 @@ GetLockConflicts(const LOCKTAG *locktag, LOCKMODE lockmode, int *countp)
for (i = 0; i < ProcGlobal->allProcCount; i++)
{
PGPROC *proc = &ProcGlobal->allProcs[i];
- uint32 f;
+ uint32 j;
/* A backend never blocks itself */
if (proc == MyProc)
@@ -3029,10 +3081,13 @@ GetLockConflicts(const LOCKTAG *locktag, LOCKMODE lockmode, int *countp)
for (j = 0; j < FP_LOCK_SLOTS_PER_GROUP; j++)
{
uint32 lockmask;
+ uint32 f;
- f = group * FP_LOCK_SLOTS_PER_GROUP + j;
+ /* index into the whole per-backend array */
+ f = FP_LOCK_SLOT_INDEX(group, j);
- Assert(f >= 0 && f < FP_LOCKS_PER_BACKEND);
+ /* must not overflow the array of all locks for a backend */
+ Assert(f < FP_LOCK_SLOTS_PER_BACKEND);
/* Look for an allocated slot matching the given relid. */
if (relid != proc->fpRelId[f])
@@ -3693,7 +3748,7 @@ GetLockStatusData(void)
LWLockAcquire(&proc->fpInfoLock, LW_SHARED);
- for (f = 0; f < FP_LOCKS_PER_BACKEND; ++f)
+ for (f = 0; f < FP_LOCK_SLOTS_PER_BACKEND; ++f)
{
LockInstanceData *instance;
uint32 lockbits = FAST_PATH_GET_BITS(proc, f);
diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h
index f074266a48c..d988cfce99e 100644
--- a/src/include/storage/proc.h
+++ b/src/include/storage/proc.h
@@ -85,7 +85,7 @@ struct XidCache
*/
#define FP_LOCK_GROUPS_PER_BACKEND 64
#define FP_LOCK_SLOTS_PER_GROUP 16 /* don't change */
-#define FP_LOCKS_PER_BACKEND (FP_LOCK_SLOTS_PER_GROUP * FP_LOCK_GROUPS_PER_BACKEND)
+#define FP_LOCK_SLOTS_PER_BACKEND (FP_LOCK_SLOTS_PER_GROUP * FP_LOCK_GROUPS_PER_BACKEND)
/*
* Flags for PGPROC.delayChkptFlags
*
@@ -294,7 +294,7 @@ struct PGPROC
/* Lock manager data, recording fast-path locks taken by this backend. */
LWLock fpInfoLock; /* protects per-backend fast-path state */
uint64 fpLockBits[FP_LOCK_GROUPS_PER_BACKEND]; /* lock modes held for each fast-path slot */
- Oid fpRelId[FP_LOCKS_PER_BACKEND]; /* slots for rel oids */
+ Oid fpRelId[FP_LOCK_SLOTS_PER_BACKEND]; /* slots for rel oids */
bool fpVXIDLock; /* are we holding a fast-path VXID lock? */
LocalTransactionId fpLocalTransactionId; /* lxid for fast-path VXID
* lock */
--
2.46.0
[text/x-patch] v20240902-0003-drive-this-by-max_locks_per_transaction.patch (9.7K, ../../[email protected]/4-v20240902-0003-drive-this-by-max_locks_per_transaction.patch)
download | inline diff:
From 4394b23800e1f0d96d571fac116c29ef0cf6d94a Mon Sep 17 00:00:00 2001
From: Tomas Vondra <[email protected]>
Date: Mon, 2 Sep 2024 01:56:17 +0200
Subject: [PATCH v20240902 3/4] drive this by max_locks_per_transaction
---
src/backend/storage/lmgr/lock.c | 34 ++++++++++++++++++-------
src/backend/storage/lmgr/proc.c | 45 +++++++++++++++++++++++++++++++++
src/include/storage/proc.h | 10 +++++---
3 files changed, 76 insertions(+), 13 deletions(-)
diff --git a/src/backend/storage/lmgr/lock.c b/src/backend/storage/lmgr/lock.c
index 524aee863fd..14124875bf9 100644
--- a/src/backend/storage/lmgr/lock.c
+++ b/src/backend/storage/lmgr/lock.c
@@ -166,8 +166,13 @@ typedef struct TwoPhaseLockRecord
* might be higher than the real number if another backend has transferred
* our locks to the primary lock table, but it can never be lower than the
* real value, since only we can acquire locks on our own behalf.
+ *
+ * XXX Allocate a static array of the maximum size. We could have a pointer
+ * and then allocate just the right size to save a couple kB, but that does
+ * not seem worth the extra complexity of having to initialize it etc. This
+ * way it gets initialized automaticaly.
*/
-static int FastPathLocalUseCounts[FP_LOCK_GROUPS_PER_BACKEND];
+static int FastPathLocalUseCounts[FP_LOCK_GROUPS_PER_BACKEND_MAX];
/*
* Flag to indicate if the relation extension lock is held by this backend.
@@ -184,6 +189,17 @@ static int FastPathLocalUseCounts[FP_LOCK_GROUPS_PER_BACKEND];
*/
static bool IsRelationExtensionLockHeld PG_USED_FOR_ASSERTS_ONLY = false;
+/*
+ * Number of fast-path locks per backend - size of the arrays in PGPROC.
+ * This is set only once during start, before initializing shared memory.
+ * After that it remains constant.
+ *
+ * XXX Right now this is sized based on max_locks_per_transaction GUC.
+ * We try to fit the expected number of locks into the cache, with some
+ * upper limit as a safety.
+ */
+int FastPathLockGroupsPerBackend = 0;
+
/*
* Macros to calculate the group and index for a relation.
*
@@ -198,7 +214,7 @@ static bool IsRelationExtensionLockHeld PG_USED_FOR_ASSERTS_ONLY = false;
* did (rel % 100000) or something like that first, that'd be enough to
* not wrap around. But even if it wrapped, would that be a problem?
*/
-#define FAST_PATH_LOCK_REL_GROUP(rel) (((uint64) (rel) * 49157) % FP_LOCK_GROUPS_PER_BACKEND)
+#define FAST_PATH_LOCK_REL_GROUP(rel) (((uint64) (rel) * 49157) % FastPathLockGroupsPerBackend)
/*
* Given a lock index (into the per-backend array), calculated using the
@@ -213,7 +229,7 @@ static bool IsRelationExtensionLockHeld PG_USED_FOR_ASSERTS_ONLY = false;
/* Calculate index in the whole per-backend array of lock slots. */
#define FP_LOCK_SLOT_INDEX(group, index) \
- (AssertMacro(((group) >= 0) && ((group) < FP_LOCK_GROUPS_PER_BACKEND)), \
+ (AssertMacro(((group) >= 0) && ((group) < FastPathLockGroupsPerBackend)), \
AssertMacro(((index) >= 0) && ((index) < FP_LOCK_SLOTS_PER_GROUP)), \
((group) * FP_LOCK_SLOTS_PER_GROUP + (index)))
@@ -2100,7 +2116,7 @@ LockRelease(const LOCKTAG *locktag, LOCKMODE lockmode, bool sessionLock)
/* Which FP group does the lock belong to? */
group = FAST_PATH_LOCK_REL_GROUP(locktag->locktag_field2);
- Assert(group >= 0 && group < FP_LOCK_GROUPS_PER_BACKEND);
+ Assert(group >= 0 && group < FastPathLockGroupsPerBackend);
/* Attempt fast release of any lock eligible for the fast path. */
if (EligibleForRelationFastPath(locktag, lockmode) &&
@@ -2679,7 +2695,7 @@ FastPathGrantRelationLock(Oid relid, LOCKMODE lockmode)
/* Which FP group does the lock belong to? */
group = FAST_PATH_LOCK_REL_GROUP(relid);
- Assert(group < FP_LOCK_GROUPS_PER_BACKEND);
+ Assert(group < FastPathLockGroupsPerBackend);
/* Scan for existing entry for this relid, remembering empty slot. */
for (i = 0; i < FP_LOCK_SLOTS_PER_GROUP; i++)
@@ -2730,7 +2746,7 @@ FastPathUnGrantRelationLock(Oid relid, LOCKMODE lockmode)
/* Which FP group does the lock belong to? */
group = FAST_PATH_LOCK_REL_GROUP(relid);
- Assert(group < FP_LOCK_GROUPS_PER_BACKEND);
+ Assert(group < FastPathLockGroupsPerBackend);
FastPathLocalUseCounts[group] = 0;
for (i = 0; i < FP_LOCK_SLOTS_PER_GROUP; i++)
@@ -2810,7 +2826,7 @@ FastPathTransferRelationLocks(LockMethod lockMethodTable, const LOCKTAG *locktag
/* Which FP group does the lock belong to? */
group = FAST_PATH_LOCK_REL_GROUP(relid);
- Assert(group < FP_LOCK_GROUPS_PER_BACKEND);
+ Assert(group < FastPathLockGroupsPerBackend);
for (j = 0; j < FP_LOCK_SLOTS_PER_GROUP; j++)
{
@@ -2879,7 +2895,7 @@ FastPathGetRelationLockEntry(LOCALLOCK *locallock)
/* Which FP group does the lock belong to? */
group = FAST_PATH_LOCK_REL_GROUP(relid);
- Assert(group < FP_LOCK_GROUPS_PER_BACKEND);
+ Assert(group < FastPathLockGroupsPerBackend);
LWLockAcquire(&MyProc->fpInfoLock, LW_EXCLUSIVE);
@@ -3001,7 +3017,7 @@ GetLockConflicts(const LOCKTAG *locktag, LOCKMODE lockmode, int *countp)
/* Which FP group does the lock belong to? */
group = FAST_PATH_LOCK_REL_GROUP(locktag->locktag_field2);
- Assert(group < FP_LOCK_GROUPS_PER_BACKEND);
+ Assert(group < FastPathLockGroupsPerBackend);
if (lockmethodid <= 0 || lockmethodid >= lengthof(LockMethods))
elog(ERROR, "unrecognized lock method: %d", lockmethodid);
diff --git a/src/backend/storage/lmgr/proc.c b/src/backend/storage/lmgr/proc.c
index ac66da8638f..c3d2856b151 100644
--- a/src/backend/storage/lmgr/proc.c
+++ b/src/backend/storage/lmgr/proc.c
@@ -113,6 +113,28 @@ ProcGlobalShmemSize(void)
size = add_size(size, mul_size(TotalProcs, sizeof(*ProcGlobal->subxidStates)));
size = add_size(size, mul_size(TotalProcs, sizeof(*ProcGlobal->statusFlags)));
+ /*
+ * Calculate the number of fast-path lock groups. We allow anything
+ * between 1 and 1024 groups, with the usual power-of-2 logic.
+ *
+ * XXX The 1 is the current value, 1024 is an arbitrary limit matching
+ * max_locks_per_xact = 16k. The default is max_locks_per_xact = 64,
+ * which means 4 groups by default.
+ */
+ FastPathLockGroupsPerBackend = 1;
+ while (FastPathLockGroupsPerBackend < FP_LOCK_GROUPS_PER_BACKEND_MAX)
+ {
+ /* stop once we hit max_locks_per_xact */
+ if (FastPathLockGroupsPerBackend * FP_LOCK_SLOTS_PER_GROUP >= max_locks_per_xact)
+ break;
+
+ FastPathLockGroupsPerBackend *= 2;
+ }
+
+ elog(LOG, "FastPathLockGroupsPerBackend = %d", FastPathLockGroupsPerBackend);
+
+ size = add_size(size, mul_size(TotalProcs, FastPathLockGroupsPerBackend * (sizeof(uint64) + sizeof(Oid) * FP_LOCK_SLOTS_PER_GROUP)));
+
return size;
}
@@ -162,6 +184,8 @@ InitProcGlobal(void)
j;
bool found;
uint32 TotalProcs = MaxBackends + NUM_AUXILIARY_PROCS + max_prepared_xacts;
+ char *ptr,
+ *endptr;
/* Create the ProcGlobal shared structure */
ProcGlobal = (PROC_HDR *)
@@ -211,12 +235,31 @@ InitProcGlobal(void)
ProcGlobal->statusFlags = (uint8 *) ShmemAlloc(TotalProcs * sizeof(*ProcGlobal->statusFlags));
MemSet(ProcGlobal->statusFlags, 0, TotalProcs * sizeof(*ProcGlobal->statusFlags));
+ /*
+ * Allocate arrays for fast-path locks. Those are variable-length, based
+ * on max_locks_per_transaction, so can't be included in PGPROC.
+ */
+ ptr = ShmemAlloc(TotalProcs * (FastPathLockGroupsPerBackend * (sizeof(uint64) + sizeof(Oid) * FP_LOCK_SLOTS_PER_GROUP)));
+ endptr = ptr + (TotalProcs * (FastPathLockGroupsPerBackend * (sizeof(uint64) + sizeof(Oid) * FP_LOCK_SLOTS_PER_GROUP)));
+ MemSet(ptr, 0, TotalProcs * (FastPathLockGroupsPerBackend * (sizeof(uint64) + sizeof(Oid) * FP_LOCK_SLOTS_PER_GROUP)));
+
+ elog(LOG, "ptrlen %lu", (endptr - ptr));
+
for (i = 0; i < TotalProcs; i++)
{
PGPROC *proc = &procs[i];
/* Common initialization for all PGPROCs, regardless of type. */
+ /*
+ * Set the fast-path lock arrays.
+ */
+ proc->fpLockBits = (uint64 *) ptr;
+ ptr += sizeof(uint64) * FastPathLockGroupsPerBackend;
+
+ proc->fpRelId = (Oid *) ptr;
+ ptr += sizeof(Oid) * FastPathLockGroupsPerBackend * FP_LOCK_SLOTS_PER_GROUP;
+
/*
* Set up per-PGPROC semaphore, latch, and fpInfoLock. Prepared xact
* dummy PGPROCs don't need these though - they're never associated
@@ -278,6 +321,8 @@ InitProcGlobal(void)
pg_atomic_init_u64(&(proc->waitStart), 0);
}
+ Assert(endptr == ptr);
+
/*
* Save pointers to the blocks of PGPROC structures reserved for auxiliary
* processes and prepared transactions.
diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h
index d988cfce99e..c9184cefccf 100644
--- a/src/include/storage/proc.h
+++ b/src/include/storage/proc.h
@@ -83,9 +83,11 @@ struct XidCache
* rather than the main lock table. This eases contention on the lock
* manager LWLocks. See storage/lmgr/README for additional details.
*/
-#define FP_LOCK_GROUPS_PER_BACKEND 64
+extern PGDLLIMPORT int FastPathLockGroupsPerBackend;
+#define FP_LOCK_GROUPS_PER_BACKEND_MAX 1024
#define FP_LOCK_SLOTS_PER_GROUP 16 /* don't change */
-#define FP_LOCK_SLOTS_PER_BACKEND (FP_LOCK_SLOTS_PER_GROUP * FP_LOCK_GROUPS_PER_BACKEND)
+#define FP_LOCK_SLOTS_PER_BACKEND (FP_LOCK_SLOTS_PER_GROUP * FastPathLockGroupsPerBackend)
+
/*
* Flags for PGPROC.delayChkptFlags
*
@@ -293,8 +295,8 @@ struct PGPROC
/* Lock manager data, recording fast-path locks taken by this backend. */
LWLock fpInfoLock; /* protects per-backend fast-path state */
- uint64 fpLockBits[FP_LOCK_GROUPS_PER_BACKEND]; /* lock modes held for each fast-path slot */
- Oid fpRelId[FP_LOCK_SLOTS_PER_BACKEND]; /* slots for rel oids */
+ uint64 *fpLockBits; /* lock modes held for each fast-path slot */
+ Oid *fpRelId; /* slots for rel oids */
bool fpVXIDLock; /* are we holding a fast-path VXID lock? */
LocalTransactionId fpLocalTransactionId; /* lxid for fast-path VXID
* lock */
--
2.46.0
[text/x-patch] v20240902-0004-separate-guc-to-allow-benchmarking.patch (2.1K, ../../[email protected]/5-v20240902-0004-separate-guc-to-allow-benchmarking.patch)
download | inline diff:
From 49b7bf39120535f2f258847aeb911922b7b0a192 Mon Sep 17 00:00:00 2001
From: Tomas Vondra <[email protected]>
Date: Mon, 2 Sep 2024 02:19:16 +0200
Subject: [PATCH v20240902 4/4] separate guc to allow benchmarking
---
src/backend/storage/lmgr/proc.c | 18 +++++++++---------
src/backend/utils/misc/guc_tables.c | 10 ++++++++++
2 files changed, 19 insertions(+), 9 deletions(-)
diff --git a/src/backend/storage/lmgr/proc.c b/src/backend/storage/lmgr/proc.c
index c3d2856b151..b25699a94c6 100644
--- a/src/backend/storage/lmgr/proc.c
+++ b/src/backend/storage/lmgr/proc.c
@@ -121,15 +121,15 @@ ProcGlobalShmemSize(void)
* max_locks_per_xact = 16k. The default is max_locks_per_xact = 64,
* which means 4 groups by default.
*/
- FastPathLockGroupsPerBackend = 1;
- while (FastPathLockGroupsPerBackend < FP_LOCK_GROUPS_PER_BACKEND_MAX)
- {
- /* stop once we hit max_locks_per_xact */
- if (FastPathLockGroupsPerBackend * FP_LOCK_SLOTS_PER_GROUP >= max_locks_per_xact)
- break;
-
- FastPathLockGroupsPerBackend *= 2;
- }
+// FastPathLockGroupsPerBackend = 1;
+// while (FastPathLockGroupsPerBackend < FP_LOCK_GROUPS_PER_BACKEND_MAX)
+// {
+// /* stop once we hit max_locks_per_xact */
+// if (FastPathLockGroupsPerBackend * FP_LOCK_SLOTS_PER_GROUP >= max_locks_per_xact)
+// break;
+//
+// FastPathLockGroupsPerBackend *= 2;
+// }
elog(LOG, "FastPathLockGroupsPerBackend = %d", FastPathLockGroupsPerBackend);
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index 521ec5591c8..a6d4e0a8905 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -2788,6 +2788,16 @@ struct config_int ConfigureNamesInt[] =
NULL, NULL, NULL
},
+ {
+ {"fastpath_lock_groups", PGC_POSTMASTER, LOCK_MANAGEMENT,
+ gettext_noop("Sets the maximum number of locks per transaction."),
+ gettext_noop("number of groups in the fast-path lock array.")
+ },
+ &FastPathLockGroupsPerBackend,
+ 1, 1, INT_MAX,
+ NULL, NULL, NULL
+ },
+
{
{"max_pred_locks_per_transaction", PGC_POSTMASTER, LOCK_MANAGEMENT,
gettext_noop("Sets the maximum number of predicate locks per transaction."),
--
2.46.0
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: scalability bottlenecks with (many) partitions (and more)
@ 2024-09-03 15:06 Robert Haas <[email protected]>
parent: Tomas Vondra <[email protected]>
0 siblings, 1 reply; 10+ messages in thread
From: Robert Haas @ 2024-09-03 15:06 UTC (permalink / raw)
To: Tomas Vondra <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>; Andres Freund <[email protected]>
On Mon, Sep 2, 2024 at 1:46 PM Tomas Vondra <[email protected]> wrote:
> The one argument to not tie this to max_locks_per_transaction is the
> vastly different "per element" memory requirements. If you add one entry
> to max_locks_per_transaction, that adds LOCK which is a whopping 152B.
> OTOH one fast-path entry is ~5B, give or take. That's a pretty big
> difference, and it if the locks fit into the shared lock table, but
> you'd like to allow more fast-path locks, having to increase
> max_locks_per_transaction is not great - pretty wastefull.
>
> OTOH I'd really hate to just add another GUC and hope the users will
> magically know how to set it correctly. That's pretty unlikely, IMO. I
> myself wouldn't know what a good value is, I think.
>
> But say we add a GUC and set it to -1 by default, in which case it just
> inherits the max_locks_per_transaction value. And then also provide some
> basic metric about this fast-path cache, so that people can tune this?
All things being equal, I would prefer not to add another GUC for
this, but we might need it.
Doing some worst case math, suppose somebody has max_connections=1000
(which is near the upper limit of what I'd consider a sane setting)
and max_locks_per_transaction=10000 (ditto). The product is 10
million, so every 10 bytes of storage each a gigabyte of RAM. Chewing
up 15GB of RAM when you could have chewed up only 0.5GB certainly
isn't too great. On the other hand, those values are kind of pushing
the limits of what is actually sane. If you imagine
max_locks_per_transaction=2000 rather than
max_locks_per_connection=10000, then it's only 3GB and that's
hopefully not a lot on the hopefully-giant machine where you're
running this.
> I think just knowing the "hit ratio" would be enough, i.e. counters for
> how often it fits into the fast-path array, and how often we had to
> promote it to the shared lock table would be enough, no?
Yeah, probably. I mean, that won't tell you how big it needs to be,
but it will tell you whether it's big enough.
I wonder if we should be looking at further improvements in the lock
manager of some kind. For instance, imagine if we allocated storage
via DSM or DSA for cases where we need a really large number of Lock
entries. The downside of that is that we might run out of memory for
locks at runtime, which would perhaps suck, but you'd probably use
significantly less memory on average. Or, maybe we need an even bigger
rethink where we reconsider the idea that we take a separate lock for
every single partition instead of having some kind of hierarchy-aware
lock manager. I don't know. But this feels like very old, crufty tech.
There's probably something more state of the art that we could or
should be doing.
--
Robert Haas
EDB: http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: scalability bottlenecks with (many) partitions (and more)
@ 2024-09-04 15:12 David Rowley <[email protected]>
parent: Robert Haas <[email protected]>
0 siblings, 0 replies; 10+ messages in thread
From: David Rowley @ 2024-09-04 15:12 UTC (permalink / raw)
To: Robert Haas <[email protected]>; +Cc: Tomas Vondra <[email protected]>; PostgreSQL Hackers <[email protected]>; Andres Freund <[email protected]>
On Wed, 4 Sept 2024 at 03:06, Robert Haas <[email protected]> wrote:
>
> On Mon, Sep 2, 2024 at 1:46 PM Tomas Vondra <[email protected]> wrote:
> > But say we add a GUC and set it to -1 by default, in which case it just
> > inherits the max_locks_per_transaction value. And then also provide some
> > basic metric about this fast-path cache, so that people can tune this?
>
> All things being equal, I would prefer not to add another GUC for
> this, but we might need it.
I think driving the array size from max_locks_per_transaction is a
good idea (rounded up to the next multiple of 16?). If someone comes
along one day and shows us a compelling case where some backend needs
more than its fair share of locks and performance is bad because of
that, then maybe we can consider adding a GUC then. Certainly, it's
much easier to add a GUC later if someone convinces us that it's a
good idea than it is to add it now and try to take it away in the
future if we realise it's not useful enough to keep.
David
^ permalink raw reply [nested|flat] 10+ messages in thread
end of thread, other threads:[~2024-09-04 15:12 UTC | newest]
Thread overview: 10+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2021-03-11 15:18 [PATCH 5/8] f!3nd attempt to use pkgconfig to allow compiling on OSX Justin Pryzby <[email protected]>
2024-06-24 15:05 Re: scalability bottlenecks with (many) partitions (and more) Robert Haas <[email protected]>
2024-06-25 10:04 ` Re: scalability bottlenecks with (many) partitions (and more) Tomas Vondra <[email protected]>
2024-06-25 20:13 ` Re: scalability bottlenecks with (many) partitions (and more) Robert Haas <[email protected]>
2024-08-05 11:38 ` Re: scalability bottlenecks with (many) partitions (and more) Tomas Vondra <[email protected]>
2024-09-01 19:30 ` Re: scalability bottlenecks with (many) partitions (and more) Tomas Vondra <[email protected]>
2024-09-01 23:53 ` Re: scalability bottlenecks with (many) partitions (and more) Robert Haas <[email protected]>
2024-09-02 17:46 ` Re: scalability bottlenecks with (many) partitions (and more) Tomas Vondra <[email protected]>
2024-09-03 15:06 ` Re: scalability bottlenecks with (many) partitions (and more) Robert Haas <[email protected]>
2024-09-04 15:12 ` Re: scalability bottlenecks with (many) partitions (and more) David Rowley <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox