($INBOX_DIR/description missing)  
help / color / mirror / Atom feed
[PATCH 03/23] cirrus/ccache: disable compression and show stats
16+ messages / 6 participants
[nested] [flat]

* [PATCH 03/23] cirrus/ccache: disable compression and show stats
@ 2022-04-03 05:10  Justin Pryzby <[email protected]>
  0 siblings, 0 replies; 16+ messages in thread

From: Justin Pryzby @ 2022-04-03 05:10 UTC (permalink / raw)

Since v4.0, ccache enables zstd compression by default, saving roughly
2x-3x.  But, cirrus caches are compressed as tar.gz, so we could disable
ccache compression, allowing cirrus to gzip the uncompressed data
(better than ccache's default of zstd-1).

With default compression enabled (https://cirrus-ci.com/task/6692342840164352):
linux/debian/bullseye has 4.2; 99MB cirrus cache; cache_size_kibibyte	109616
macos has 4.5.1: 47MB cirrus cache; cache_size_kibibyte	52500
freebsd has 3.7.12: 42MB cirrus cache; cache_size_kibibyte	134064
windows has 4.6.1; 180MB cirrus cache; cache_size_kibibyte	51179
todo: compiler warnings

With compression disabled (https://cirrus-ci.com/task/4614182514458624):
linux: 91MB cirrus cache; cache_size_kibibyte	316136
macos: 41MB cirrus cache; cache_size_kibibyte	118068
windows: 42MB cirrus cache; cache_size_kibibyte	134064
freebsd is the same

The stats should either be shown or logged (or maybe run with CCACHE_NOSTATS,
to avoid re-uploading cache tarball in a 100% cached build, due only to
updating ./**/stats).

Note that ccache 4.4 supports CCACHE_STATSLOG, which seems ideal.

freebsd, linux
ci-os-only: macos
---
 .cirrus.yml | 33 ++++++++++++++++++++++++++++-----
 1 file changed, 28 insertions(+), 5 deletions(-)

diff --git a/.cirrus.yml b/.cirrus.yml
index 591e802bee2..54256570030 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -16,7 +16,9 @@ env:
   # Useful to be able to analyse what in a script takes long
   CIRRUS_LOG_TIMESTAMP: true
 
-  CCACHE_MAXSIZE: "250M"
+  CCACHE_MAXSIZE: "500M"
+  CCACHE_NOCOMPRESS: 1
+  #CCACHE_STATSLOG: ccache-stats.log
 
   # target to test, for all but windows
   CHECK: check-world PROVE_FLAGS=$PROVE_FLAGS
@@ -110,7 +112,10 @@ task:
         CXX="ccache c++" \
         CFLAGS="-Og -ggdb"
     EOF
-  build_script: su postgres -c "gmake -s -j${BUILD_JOBS} world-bin"
+  build_script: |
+    su postgres -c "ccache --zero-stats"
+    su postgres -c "gmake -s -j${BUILD_JOBS} world-bin"
+    su postgres -c "ccache --show-stats"
   upload_caches: ccache
 
   # The use of script avoids make -Otarget complaints about fcntl() on
@@ -206,7 +211,11 @@ task:
         CFLAGS="-Og -ggdb" \
         CXXFLAGS="-Og -ggdb"
     EOF
-  build_script: su postgres -c "make -s -j${BUILD_JOBS} world-bin"
+
+  build_script: |
+    su postgres -c "ccache --zero-stats"
+    su postgres -c "make -s -j${BUILD_JOBS} world-bin"
+    su postgres -c "ccache --show-stats"
   upload_caches: ccache
 
   test_world_script: |
@@ -324,7 +333,11 @@ task:
       \
       LLVM_CONFIG=${brewpath}/llvm/bin/llvm-config \
       PYTHON=python3
-  build_script: gmake -s -j${BUILD_JOBS} world-bin
+
+  build_script: |
+    ccache --zero-stats
+    gmake -s -j${BUILD_JOBS} world-bin
+    ccache --show-stats -v
   upload_caches: ccache
 
   test_world_script: |
@@ -482,7 +495,7 @@ task:
 
     # Use larger ccache cache, as this task compiles with multiple compilers /
     # flag combinations
-    CCACHE_MAXSIZE: "1GB"
+    CCACHE_MAXSIZE: "1500MB"
     CCACHE_DIR: "/tmp/ccache_dir"
 
     LINUX_CONFIGURE_FEATURES: *LINUX_CONFIGURE_FEATURES
@@ -530,7 +543,9 @@ task:
         ${LINUX_CONFIGURE_FEATURES} \
         CC="ccache gcc" CXX="ccache g++" CLANG="ccache clang"
       make -s -j${BUILD_JOBS} clean
+      ccache --zero-stats
       time make -s -j${BUILD_JOBS} world-bin
+      ccache --show-stats
 
   # gcc, cassert on, dtrace off
   always:
@@ -541,7 +556,9 @@ task:
         ${LINUX_CONFIGURE_FEATURES} \
         CC="ccache gcc" CXX="ccache g++" CLANG="ccache clang"
       make -s -j${BUILD_JOBS} clean
+      ccache --zero-stats
       time make -s -j${BUILD_JOBS} world-bin
+      ccache --show-stats
 
   # clang, cassert off, dtrace off
   always:
@@ -551,7 +568,9 @@ task:
         ${LINUX_CONFIGURE_FEATURES} \
         CC="ccache clang" CXX="ccache clang++" CLANG="ccache clang"
       make -s -j${BUILD_JOBS} clean
+      ccache --zero-stats
       time make -s -j${BUILD_JOBS} world-bin
+      ccache --show-stats
 
   # clang, cassert on, dtrace on
   always:
@@ -563,7 +582,9 @@ task:
         ${LINUX_CONFIGURE_FEATURES} \
         CC="ccache clang" CXX="ccache clang++" CLANG="ccache clang"
       make -s -j${BUILD_JOBS} clean
+      ccache --zero-stats
       time make -s -j${BUILD_JOBS} world-bin
+      ccache --show-stats
 
   # cross-compile to windows
   always:
@@ -574,7 +595,9 @@ task:
         CC="ccache x86_64-w64-mingw32-gcc" \
         CXX="ccache x86_64-w64-mingw32-g++"
       make -s -j${BUILD_JOBS} clean
+      ccache --zero-stats
       time make -s -j${BUILD_JOBS} world-bin
+      ccache --show-stats
 
   ###
   # Verify docs can be built
-- 
2.17.1


--tKkaNMvYmhQvRCRK
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="0004-cirrus-ccache-add-explicit-cache-keys.patch"



^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* [PATCH 03/23] cirrus/ccache: disable compression and show stats
@ 2022-04-03 05:10  Justin Pryzby <[email protected]>
  0 siblings, 0 replies; 16+ messages in thread

From: Justin Pryzby @ 2022-04-03 05:10 UTC (permalink / raw)

Since v4.0, ccache enables zstd compression by default, saving roughly
2x-3x.  But, cirrus caches are compressed as tar.gz, so we could disable
ccache compression, allowing cirrus to gzip the uncompressed data
(better than ccache's default of zstd-1).

With default compression enabled (https://cirrus-ci.com/task/6692342840164352):
linux/debian/bullseye has 4.2; 99MB cirrus cache; cache_size_kibibyte	109616
macos has 4.5.1: 47MB cirrus cache; cache_size_kibibyte	52500
freebsd has 3.7.12: 42MB cirrus cache; cache_size_kibibyte	134064
windows has 4.6.1; 180MB cirrus cache; cache_size_kibibyte	51179
todo: compiler warnings

With compression disabled (https://cirrus-ci.com/task/4614182514458624):
linux: 91MB cirrus cache; cache_size_kibibyte	316136
macos: 41MB cirrus cache; cache_size_kibibyte	118068
windows: 42MB cirrus cache; cache_size_kibibyte	134064
freebsd is the same

The stats should either be shown or logged (or maybe run with CCACHE_NOSTATS,
to avoid re-uploading cache tarball in a 100% cached build, due only to
updating ./**/stats).

Note that ccache 4.4 supports CCACHE_STATSLOG, which seems ideal.

freebsd, linux
ci-os-only: macos
---
 .cirrus.yml | 33 ++++++++++++++++++++++++++++-----
 1 file changed, 28 insertions(+), 5 deletions(-)

diff --git a/.cirrus.yml b/.cirrus.yml
index 591e802bee2..54256570030 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -16,7 +16,9 @@ env:
   # Useful to be able to analyse what in a script takes long
   CIRRUS_LOG_TIMESTAMP: true
 
-  CCACHE_MAXSIZE: "250M"
+  CCACHE_MAXSIZE: "500M"
+  CCACHE_NOCOMPRESS: 1
+  #CCACHE_STATSLOG: ccache-stats.log
 
   # target to test, for all but windows
   CHECK: check-world PROVE_FLAGS=$PROVE_FLAGS
@@ -110,7 +112,10 @@ task:
         CXX="ccache c++" \
         CFLAGS="-Og -ggdb"
     EOF
-  build_script: su postgres -c "gmake -s -j${BUILD_JOBS} world-bin"
+  build_script: |
+    su postgres -c "ccache --zero-stats"
+    su postgres -c "gmake -s -j${BUILD_JOBS} world-bin"
+    su postgres -c "ccache --show-stats"
   upload_caches: ccache
 
   # The use of script avoids make -Otarget complaints about fcntl() on
@@ -206,7 +211,11 @@ task:
         CFLAGS="-Og -ggdb" \
         CXXFLAGS="-Og -ggdb"
     EOF
-  build_script: su postgres -c "make -s -j${BUILD_JOBS} world-bin"
+
+  build_script: |
+    su postgres -c "ccache --zero-stats"
+    su postgres -c "make -s -j${BUILD_JOBS} world-bin"
+    su postgres -c "ccache --show-stats"
   upload_caches: ccache
 
   test_world_script: |
@@ -324,7 +333,11 @@ task:
       \
       LLVM_CONFIG=${brewpath}/llvm/bin/llvm-config \
       PYTHON=python3
-  build_script: gmake -s -j${BUILD_JOBS} world-bin
+
+  build_script: |
+    ccache --zero-stats
+    gmake -s -j${BUILD_JOBS} world-bin
+    ccache --show-stats -v
   upload_caches: ccache
 
   test_world_script: |
@@ -482,7 +495,7 @@ task:
 
     # Use larger ccache cache, as this task compiles with multiple compilers /
     # flag combinations
-    CCACHE_MAXSIZE: "1GB"
+    CCACHE_MAXSIZE: "1500MB"
     CCACHE_DIR: "/tmp/ccache_dir"
 
     LINUX_CONFIGURE_FEATURES: *LINUX_CONFIGURE_FEATURES
@@ -530,7 +543,9 @@ task:
         ${LINUX_CONFIGURE_FEATURES} \
         CC="ccache gcc" CXX="ccache g++" CLANG="ccache clang"
       make -s -j${BUILD_JOBS} clean
+      ccache --zero-stats
       time make -s -j${BUILD_JOBS} world-bin
+      ccache --show-stats
 
   # gcc, cassert on, dtrace off
   always:
@@ -541,7 +556,9 @@ task:
         ${LINUX_CONFIGURE_FEATURES} \
         CC="ccache gcc" CXX="ccache g++" CLANG="ccache clang"
       make -s -j${BUILD_JOBS} clean
+      ccache --zero-stats
       time make -s -j${BUILD_JOBS} world-bin
+      ccache --show-stats
 
   # clang, cassert off, dtrace off
   always:
@@ -551,7 +568,9 @@ task:
         ${LINUX_CONFIGURE_FEATURES} \
         CC="ccache clang" CXX="ccache clang++" CLANG="ccache clang"
       make -s -j${BUILD_JOBS} clean
+      ccache --zero-stats
       time make -s -j${BUILD_JOBS} world-bin
+      ccache --show-stats
 
   # clang, cassert on, dtrace on
   always:
@@ -563,7 +582,9 @@ task:
         ${LINUX_CONFIGURE_FEATURES} \
         CC="ccache clang" CXX="ccache clang++" CLANG="ccache clang"
       make -s -j${BUILD_JOBS} clean
+      ccache --zero-stats
       time make -s -j${BUILD_JOBS} world-bin
+      ccache --show-stats
 
   # cross-compile to windows
   always:
@@ -574,7 +595,9 @@ task:
         CC="ccache x86_64-w64-mingw32-gcc" \
         CXX="ccache x86_64-w64-mingw32-g++"
       make -s -j${BUILD_JOBS} clean
+      ccache --zero-stats
       time make -s -j${BUILD_JOBS} world-bin
+      ccache --show-stats
 
   ###
   # Verify docs can be built
-- 
2.17.1


--tKkaNMvYmhQvRCRK
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="0004-cirrus-ccache-add-explicit-cache-keys.patch"



^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* [PATCH 13/21] cirrus/ccache: disable compression and show stats
@ 2022-04-03 05:10  Justin Pryzby <[email protected]>
  0 siblings, 0 replies; 16+ messages in thread

From: Justin Pryzby @ 2022-04-03 05:10 UTC (permalink / raw)

ci-os-only: linux, windows

ccache since 4.0 enables zstd compression by default.

With default compression enabled (https://cirrus-ci.com/task/6692342840164352):
linux/debian/bullseye has 4.2; 99MB cirrus cache; cache_size_kibibyte	109616
macos has 4.5.1: 47MB cirrus cache; cache_size_kibibyte	52500
freebsd has 3.7.12: 42MB cirrus cache; cache_size_kibibyte	134064
windows has 4.6.1; 180MB cirrus cache; cache_size_kibibyte	51179
todo: compiler warnings

With compression disabled (https://cirrus-ci.com/task/4614182514458624):
linux: 91MB cirrus cache; cache_size_kibibyte	316136
macos: 41MB cirrus cache; cache_size_kibibyte	118068
windows: 42MB cirrus cache; cache_size_kibibyte	134064
freebsd is the same

The stats should either be shown or logged (or maybe run with CCACHE_NOSTATS,
to avoid re-uploading cache tarball in a 100% cached build, due only to
updating ./**/stats).

Note that ccache 4.4 supports CCACHE_STATSLOG, which seems ideal.
---
 .cirrus.yml | 34 ++++++++++++++++++++++++++++++----
 1 file changed, 30 insertions(+), 4 deletions(-)

diff --git a/.cirrus.yml b/.cirrus.yml
index e5654bd5589..fd8f27ff593 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -16,7 +16,9 @@ env:
   # Useful to be able to analyse what in a script takes long
   CIRRUS_LOG_TIMESTAMP: true
 
-  CCACHE_MAXSIZE: "250M"
+  CCACHE_MAXSIZE: "500M"
+  CCACHE_NOCOMPRESS: 1
+  #CCACHE_STATSLOG: ccache.log
 
   # target to test, for all but windows
   CHECK: check-world PROVE_FLAGS=$PROVE_FLAGS
@@ -110,7 +112,11 @@ task:
         CXX="ccache c++" \
         CFLAGS="-Og -ggdb"
     EOF
-  build_script: su postgres -c "gmake -s -j${BUILD_JOBS} world-bin"
+
+  build_script: |
+    ccache --zero-stats
+    su postgres -c "gmake -s -j${BUILD_JOBS} world-bin"
+    ccache --show-stats
   upload_caches: ccache
 
   # The use of script avoids make -Otarget complaints about fcntl() on
@@ -206,7 +212,11 @@ task:
         CFLAGS="-Og -ggdb -fsanitize=undefined,alignment -fno-sanitize-recover=all" \
         CXXFLAGS="-Og -ggdb -fsanitize=undefined,alignment -fno-sanitize-recover=all"
     EOF
-  build_script: su postgres -c "make -s -j${BUILD_JOBS} world-bin"
+
+  build_script: |
+    ccache --zero-stats
+    su postgres -c "make -s -j${BUILD_JOBS} world-bin"
+    ccache --show-stats
   upload_caches: ccache
 
   test_world_script: |
@@ -325,7 +335,11 @@ task:
       \
       LLVM_CONFIG=${brewpath}/llvm/bin/llvm-config \
       PYTHON=python3
-  build_script: gmake -s -j${BUILD_JOBS} world-bin
+
+  build_script: |
+    ccache --zero-stats
+    gmake -s -j${BUILD_JOBS} world-bin
+    ccache --show-stats
   upload_caches: ccache
 
   test_world_script: |
@@ -421,7 +435,9 @@ task:
     - perl src/tools/msvc/mkvcbuild.pl
   build_script:
     - vcvarsall x64
+    - ccache --zero-stats
     - msbuild %MSBFLAGS% pgsql.sln
+    - ccache --show-stats
 
   always:
     upload_caches: ccache
@@ -536,7 +552,9 @@ task:
         ${LINUX_CONFIGURE_FEATURES} \
         CC="ccache gcc" CXX="ccache g++" CLANG="ccache clang"
       make -s -j${BUILD_JOBS} clean
+      ccache --zero-stats
       time make -s -j${BUILD_JOBS} world-bin
+      ccache --show-stats
 
   # gcc, cassert on, dtrace off
   always:
@@ -547,7 +565,9 @@ task:
         ${LINUX_CONFIGURE_FEATURES} \
         CC="ccache gcc" CXX="ccache g++" CLANG="ccache clang"
       make -s -j${BUILD_JOBS} clean
+      ccache --zero-stats
       time make -s -j${BUILD_JOBS} world-bin
+      ccache --show-stats
 
   # clang, cassert off, dtrace off
   always:
@@ -557,7 +577,9 @@ task:
         ${LINUX_CONFIGURE_FEATURES} \
         CC="ccache clang" CXX="ccache clang++" CLANG="ccache clang"
       make -s -j${BUILD_JOBS} clean
+      ccache --zero-stats
       time make -s -j${BUILD_JOBS} world-bin
+      ccache --show-stats
 
   # clang, cassert on, dtrace on
   always:
@@ -569,7 +591,9 @@ task:
         ${LINUX_CONFIGURE_FEATURES} \
         CC="ccache clang" CXX="ccache clang++" CLANG="ccache clang"
       make -s -j${BUILD_JOBS} clean
+      ccache --zero-stats
       time make -s -j${BUILD_JOBS} world-bin
+      ccache --show-stats
 
   # cross-compile to windows
   always:
@@ -580,7 +604,9 @@ task:
         CC="ccache x86_64-w64-mingw32-gcc" \
         CXX="ccache x86_64-w64-mingw32-g++"
       make -s -j${BUILD_JOBS} clean
+      ccache --zero-stats
       time make -s -j${BUILD_JOBS} world-bin
+      ccache --show-stats
 
   ###
   # Verify docs can be built
-- 
2.17.1


--STPqjqpCrtky8aYs
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="0014-another-way-to-install-uri-regress-uri_regress.patch"



^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* [PATCH 05/11] cirrus/ccache: disable compression and show stats
@ 2022-04-03 05:10  Justin Pryzby <[email protected]>
  0 siblings, 0 replies; 16+ messages in thread

From: Justin Pryzby @ 2022-04-03 05:10 UTC (permalink / raw)

Since v4.0, ccache enables zstd compression by default, saving roughly
2x-3x.  But, cirrus caches are compressed as tar.gz, so we could disable
ccache compression, allowing cirrus to gzip the uncompressed data
(better than ccache's default of zstd-1).

With default compression enabled (https://cirrus-ci.com/task/6692342840164352):
linux/debian/bullseye has 4.2; 99MB cirrus cache; cache_size_kibibyte	109616
macos has 4.5.1: 47MB cirrus cache; cache_size_kibibyte	52500
freebsd has 3.7.12: 42MB cirrus cache; cache_size_kibibyte	134064
XXX windows has 4.7.2; 180MB cirrus cache; cache_size_kibibyte	51179
todo: compiler warnings

With compression disabled (https://cirrus-ci.com/task/4614182514458624):
linux: 91MB cirrus cache; cache_size_kibibyte	316136
macos: 41MB cirrus cache; cache_size_kibibyte	118068
windows: 42MB cirrus cache; cache_size_kibibyte	134064
freebsd is the same

The stats should be shown and/or logged.
ccache --show-stats shows the *cumulative* stats (including prior
compilations)
ccache --zero-stats clears out not only the global stats, but the
per-file cache stats (from which the global stats are derived) - which
obviously makes the cache work poorly.

Note that ccache 4.4 supports CCACHE_STATSLOG, which seems ideal.
The log should be written *outside* the ccache folder - it shouldn't be
preserved across cirrusci task invocations.

freebsd, linux, macos
ci-os-only: linux
---
 .cirrus.yml | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/.cirrus.yml b/.cirrus.yml
index 47c4caf51ac..d9660149a73 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -16,7 +16,11 @@ env:
   # Useful to be able to analyse what in a script takes long
   CIRRUS_LOG_TIMESTAMP: true
 
-  CCACHE_MAXSIZE: "250M"
+  CCACHE_MAXSIZE: "750M"
+  CCACHE_HARDLINK: 1
+  #CCACHE_NOCOMPRESS: 1
+  # The statslog should be stored somewhere *outside* the persistent ccache dir
+  CCACHE_STATSLOG: ${CIRRUS_WORKING_DIR}/ccache-stats.log
 
   # target to test, for all but windows
   CHECK: check-world PROVE_FLAGS=$PROVE_FLAGS
@@ -236,7 +240,7 @@ task:
     - name: Linux - Debian Bullseye - Meson
 
       env:
-        CCACHE_MAXSIZE: "800M" # tests two different builds
+        CCACHE_MAXSIZE: "2400M" # tests two different builds
 
       configure_script: |
         su postgres <<-EOF
@@ -289,7 +293,7 @@ task:
 
       ccache_stats_end_script:
         ccache -s
-        CCACHE_MAXSIZE=400M ccache --cleanup
+        CCACHE_MAXSIZE=1200M ccache --cleanup
 
       on_failure:
         <<: *on_failure_meson
@@ -392,7 +396,7 @@ task:
       -DPG_TEST_EXTRA="$PG_TEST_EXTRA" \
       build
 
-  build_script: ninja -C build -j${BUILD_JOBS}
+  build_script: ninja -C build -j${BUILD_JOBS} && ccache --show-log-stats
   upload_caches: ccache
 
   test_world_script: |
@@ -499,7 +503,7 @@ task:
     # flag combinations.  The cache size is set high now, and manually pruned later,
     # to avoid cache cleaning cycles during the early compilations pruning
     # compilation results which could've been used during later compilations.
-    CCACHE_MAXSIZE: "2G"
+    CCACHE_MAXSIZE: "6G"
     CCACHE_DIR: "/tmp/ccache_dir"
 
     LINUX_CONFIGURE_FEATURES: *LINUX_CONFIGURE_FEATURES
@@ -620,7 +624,7 @@ task:
   always:
     ccache_stats_end_script:
       ccache -s
-      CCACHE_MAXSIZE=1G ccache --cleanup
+      CCACHE_MAXSIZE=3G ccache --cleanup
 
     upload_caches: ccache
 
-- 
2.25.1


--I4VOKWutKNZEOIPu
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="0006-cirrus-warnings-use-a-single-common-always-block.patch"



^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* [PATCH 7/8] cirrus/ccache: disable compression and show stats
@ 2022-04-03 05:10  Justin Pryzby <[email protected]>
  0 siblings, 0 replies; 16+ messages in thread

From: Justin Pryzby @ 2022-04-03 05:10 UTC (permalink / raw)

Since v4.0, ccache enables zstd compression by default, saving roughly
2x-3x.  But, cirrus caches are compressed as tar.gz, so we could disable
ccache compression, allowing cirrus to gzip the uncompressed data
(better than ccache's default of zstd-1).

With default compression enabled (https://cirrus-ci.com/task/6692342840164352):
linux/debian/bullseye has 4.2; 99MB cirrus cache; cache_size_kibibyte	109616
macos has 4.5.1: 47MB cirrus cache; cache_size_kibibyte	52500
freebsd has 3.7.12: 42MB cirrus cache; cache_size_kibibyte	134064
XXX windows has 4.7.2; 180MB cirrus cache; cache_size_kibibyte	51179
todo: compiler warnings

With compression disabled (https://cirrus-ci.com/task/4614182514458624):
linux: 91MB cirrus cache; cache_size_kibibyte	316136
macos: 41MB cirrus cache; cache_size_kibibyte	118068
windows: 42MB cirrus cache; cache_size_kibibyte	134064
freebsd is the same

The stats should be shown and/or logged.
ccache --show-stats shows the *cumulative* stats (including prior
compilations)
ccache --zero-stats clears out not only the global stats, but the
per-file cache stats (from which the global stats are derived) - which
obviously makes the cache work poorly.

Note that ccache 4.4 supports CCACHE_STATSLOG, which seems ideal.
The log should be written *outside* the ccache folder - it shouldn't be
preserved across cirrusci task invocations.

freebsd, linux, macos
ci-os-only: linux
---
 .cirrus.yml | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/.cirrus.yml b/.cirrus.yml
index 221431e70c4..d1c3119e4ff 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -16,7 +16,9 @@ env:
   # Useful to be able to analyse what in a script takes long
   CIRRUS_LOG_TIMESTAMP: true
 
-  CCACHE_MAXSIZE: "250M"
+  CCACHE_MAXSIZE: "750M"
+  CCACHE_NOCOMPRESS: 1
+  #CCACHE_STATSLOG: ccache-stats.log
 
   # target to test, for all but windows
   CHECK: check-world PROVE_FLAGS=$PROVE_FLAGS
@@ -236,7 +238,7 @@ task:
     - name: Linux - Debian Bullseye - Meson
 
       env:
-        CCACHE_MAXSIZE: "400M" # tests two different builds
+        CCACHE_MAXSIZE: "1200M" # tests two different builds
 
       configure_script: |
         su postgres <<-EOF
@@ -307,6 +309,7 @@ task:
 
     CIRRUS_WORKING_DIR: ${HOME}/pgsql/
     CCACHE_DIR: ${HOME}/ccache
+    CCACHE_STATSLOG: ${CCACHE_DIR}.stats.log
     HOMEBREW_CACHE: ${HOME}/homebrew-cache
     PERL5LIB: ${HOME}/perl5/lib/perl5
 
@@ -388,7 +391,7 @@ task:
       -DPG_TEST_EXTRA="$PG_TEST_EXTRA" \
       build
 
-  build_script: ninja -C build -j${BUILD_JOBS}
+  build_script: ninja -C build -j${BUILD_JOBS} && ccache --show-log-stats
   upload_caches: ccache
 
   test_world_script: |
@@ -496,14 +499,14 @@ task:
 
     # Use larger ccache cache, as this task compiles with multiple compilers /
     # flag combinations
-    CCACHE_MAXSIZE: "1GB"
+    CCACHE_MAXSIZE: "3G"
     CCACHE_DIR: "/tmp/ccache_dir"
 
     LINUX_CONFIGURE_FEATURES: *LINUX_CONFIGURE_FEATURES
     LINUX_MESON_FEATURES: *LINUX_MESON_FEATURES
 
   # task that did not run, count as a success, so we need to recheck Linux'
-  # condition here ...
+  # condition here; cirus warns if the "only_if" condition doesn't match the task being depended on
   only_if: $CIRRUS_CHANGE_MESSAGE !=~ '.*\nci-os-only:.*' || $CIRRUS_CHANGE_MESSAGE =~ '.*\nci-os-only:[^\n]*linux.*'
 
   container:
-- 
2.25.1


--rPFbv4B2w2tcwGo5
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="0008-cirrus-warnings-use-a-single-common-always-block.patch"



^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* [PATCH 03/25] cirrus/ccache: disable compression and show stats
@ 2022-04-03 05:10  Justin Pryzby <[email protected]>
  0 siblings, 0 replies; 16+ messages in thread

From: Justin Pryzby @ 2022-04-03 05:10 UTC (permalink / raw)

ccache since 4.0 enables zstd compression by default.

With default compression enabled (https://cirrus-ci.com/task/6692342840164352):
linux/debian/bullseye has 4.2; 99MB cirrus cache; cache_size_kibibyte	109616
macos has 4.5.1: 47MB cirrus cache; cache_size_kibibyte	52500
freebsd has 3.7.12: 42MB cirrus cache; cache_size_kibibyte	134064
windows has 4.6.1; 180MB cirrus cache; cache_size_kibibyte	51179
todo: compiler warnings

With compression disabled (https://cirrus-ci.com/task/4614182514458624):
linux: 91MB cirrus cache; cache_size_kibibyte	316136
macos: 41MB cirrus cache; cache_size_kibibyte	118068
windows: 42MB cirrus cache; cache_size_kibibyte	134064
freebsd is the same

The stats should either be shown or logged (or maybe run with CCACHE_NOSTATS,
to avoid re-uploading cache tarball in a 100% cached build, due only to
updating ./**/stats).

Note that ccache 4.4 supports CCACHE_STATSLOG, which seems ideal.

freebsd, linux
ci-os-only: macos
---
 .cirrus.yml | 33 ++++++++++++++++++++++++++++-----
 1 file changed, 28 insertions(+), 5 deletions(-)

diff --git a/.cirrus.yml b/.cirrus.yml
index f2861176be2..c0616e8560d 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -16,7 +16,9 @@ env:
   # Useful to be able to analyse what in a script takes long
   CIRRUS_LOG_TIMESTAMP: true
 
-  CCACHE_MAXSIZE: "250M"
+  CCACHE_MAXSIZE: "500M"
+  CCACHE_NOCOMPRESS: 1
+  #CCACHE_STATSLOG: ccache-stats.log
 
   # target to test, for all but windows
   CHECK: check-world PROVE_FLAGS=$PROVE_FLAGS
@@ -110,7 +112,10 @@ task:
         CXX="ccache c++" \
         CFLAGS="-Og -ggdb"
     EOF
-  build_script: su postgres -c "gmake -s -j${BUILD_JOBS} world-bin"
+  build_script: |
+    su postgres -c "ccache --zero-stats"
+    su postgres -c "gmake -s -j${BUILD_JOBS} world-bin"
+    su postgres -c "ccache --show-stats"
   upload_caches: ccache
 
   # The use of script avoids make -Otarget complaints about fcntl() on
@@ -206,7 +211,11 @@ task:
         CFLAGS="-Og -ggdb" \
         CXXFLAGS="-Og -ggdb"
     EOF
-  build_script: su postgres -c "make -s -j${BUILD_JOBS} world-bin"
+
+  build_script: |
+    su postgres -c "ccache --zero-stats"
+    su postgres -c "make -s -j${BUILD_JOBS} world-bin"
+    su postgres -c "ccache --show-stats"
   upload_caches: ccache
 
   test_world_script: |
@@ -324,7 +333,11 @@ task:
       \
       LLVM_CONFIG=${brewpath}/llvm/bin/llvm-config \
       PYTHON=python3
-  build_script: gmake -s -j${BUILD_JOBS} world-bin
+
+  build_script: |
+    ccache --zero-stats
+    gmake -s -j${BUILD_JOBS} world-bin
+    ccache --show-stats -v
   upload_caches: ccache
 
   test_world_script: |
@@ -476,7 +489,7 @@ task:
 
     # Use larger ccache cache, as this task compiles with multiple compilers /
     # flag combinations
-    CCACHE_MAXSIZE: "1GB"
+    CCACHE_MAXSIZE: "1500MB"
     CCACHE_DIR: "/tmp/ccache_dir"
 
     LINUX_CONFIGURE_FEATURES: *LINUX_CONFIGURE_FEATURES
@@ -524,7 +537,9 @@ task:
         ${LINUX_CONFIGURE_FEATURES} \
         CC="ccache gcc" CXX="ccache g++" CLANG="ccache clang"
       make -s -j${BUILD_JOBS} clean
+      ccache --zero-stats
       time make -s -j${BUILD_JOBS} world-bin
+      ccache --show-stats
 
   # gcc, cassert on, dtrace off
   always:
@@ -535,7 +550,9 @@ task:
         ${LINUX_CONFIGURE_FEATURES} \
         CC="ccache gcc" CXX="ccache g++" CLANG="ccache clang"
       make -s -j${BUILD_JOBS} clean
+      ccache --zero-stats
       time make -s -j${BUILD_JOBS} world-bin
+      ccache --show-stats
 
   # clang, cassert off, dtrace off
   always:
@@ -545,7 +562,9 @@ task:
         ${LINUX_CONFIGURE_FEATURES} \
         CC="ccache clang" CXX="ccache clang++" CLANG="ccache clang"
       make -s -j${BUILD_JOBS} clean
+      ccache --zero-stats
       time make -s -j${BUILD_JOBS} world-bin
+      ccache --show-stats
 
   # clang, cassert on, dtrace on
   always:
@@ -557,7 +576,9 @@ task:
         ${LINUX_CONFIGURE_FEATURES} \
         CC="ccache clang" CXX="ccache clang++" CLANG="ccache clang"
       make -s -j${BUILD_JOBS} clean
+      ccache --zero-stats
       time make -s -j${BUILD_JOBS} world-bin
+      ccache --show-stats
 
   # cross-compile to windows
   always:
@@ -568,7 +589,9 @@ task:
         CC="ccache x86_64-w64-mingw32-gcc" \
         CXX="ccache x86_64-w64-mingw32-g++"
       make -s -j${BUILD_JOBS} clean
+      ccache --zero-stats
       time make -s -j${BUILD_JOBS} world-bin
+      ccache --show-stats
 
   ###
   # Verify docs can be built
-- 
2.17.1


--IS0zKkzwUGydFO0o
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="0004-cirrus-ccache-add-explicit-cache-keys.patch"



^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* [PATCH 13/21] cirrus/ccache: disable compression and show stats
@ 2022-04-03 05:10  Justin Pryzby <[email protected]>
  0 siblings, 0 replies; 16+ messages in thread

From: Justin Pryzby @ 2022-04-03 05:10 UTC (permalink / raw)

ci-os-only: linux, windows

ccache since 4.0 enables zstd compression by default.

With default compression enabled (https://cirrus-ci.com/task/6692342840164352):
linux/debian/bullseye has 4.2; 99MB cirrus cache; cache_size_kibibyte	109616
macos has 4.5.1: 47MB cirrus cache; cache_size_kibibyte	52500
freebsd has 3.7.12: 42MB cirrus cache; cache_size_kibibyte	134064
windows has 4.6.1; 180MB cirrus cache; cache_size_kibibyte	51179
todo: compiler warnings

With compression disabled (https://cirrus-ci.com/task/4614182514458624):
linux: 91MB cirrus cache; cache_size_kibibyte	316136
macos: 41MB cirrus cache; cache_size_kibibyte	118068
windows: 42MB cirrus cache; cache_size_kibibyte	134064
freebsd is the same

The stats should either be shown or logged (or maybe run with CCACHE_NOSTATS,
to avoid re-uploading cache tarball in a 100% cached build, due only to
updating ./**/stats).

Note that ccache 4.4 supports CCACHE_STATSLOG, which seems ideal.
---
 .cirrus.yml | 34 ++++++++++++++++++++++++++++++----
 1 file changed, 30 insertions(+), 4 deletions(-)

diff --git a/.cirrus.yml b/.cirrus.yml
index 50a34373647..1d16594803f 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -16,7 +16,9 @@ env:
   # Useful to be able to analyse what in a script takes long
   CIRRUS_LOG_TIMESTAMP: true
 
-  CCACHE_MAXSIZE: "250M"
+  CCACHE_MAXSIZE: "500M"
+  CCACHE_NOCOMPRESS: 1
+  #CCACHE_STATSLOG: ccache.log
 
   # target to test, for all but windows
   CHECK: check-world PROVE_FLAGS=$PROVE_FLAGS
@@ -110,7 +112,11 @@ task:
         CXX="ccache c++" \
         CFLAGS="-Og -ggdb"
     EOF
-  build_script: su postgres -c "gmake -s -j${BUILD_JOBS} world-bin"
+
+  build_script: |
+    ccache --zero-stats
+    su postgres -c "gmake -s -j${BUILD_JOBS} world-bin"
+    ccache --show-stats
   upload_caches: ccache
 
   # The use of script avoids make -Otarget complaints about fcntl() on
@@ -206,7 +212,11 @@ task:
         CFLAGS="-Og -ggdb -fsanitize=undefined,alignment -fno-sanitize-recover=all" \
         CXXFLAGS="-Og -ggdb -fsanitize=undefined,alignment -fno-sanitize-recover=all"
     EOF
-  build_script: su postgres -c "make -s -j${BUILD_JOBS} world-bin"
+
+  build_script: |
+    ccache --zero-stats
+    su postgres -c "make -s -j${BUILD_JOBS} world-bin"
+    ccache --show-stats
   upload_caches: ccache
 
   test_world_script: |
@@ -325,7 +335,11 @@ task:
       \
       LLVM_CONFIG=${brewpath}/llvm/bin/llvm-config \
       PYTHON=python3
-  build_script: gmake -s -j${BUILD_JOBS} world-bin
+
+  build_script: |
+    ccache --zero-stats
+    gmake -s -j${BUILD_JOBS} world-bin
+    ccache --show-stats
   upload_caches: ccache
 
   test_world_script: |
@@ -422,7 +436,9 @@ task:
   build_script:
     #- ccache --clear
     - vcvarsall x64
+    - ccache --zero-stats
     - msbuild %MSBFLAGS% pgsql.sln
+    - ccache --show-stats
 
   always:
     upload_caches: ccache
@@ -537,7 +553,9 @@ task:
         ${LINUX_CONFIGURE_FEATURES} \
         CC="ccache gcc" CXX="ccache g++" CLANG="ccache clang"
       make -s -j${BUILD_JOBS} clean
+      ccache --zero-stats
       time make -s -j${BUILD_JOBS} world-bin
+      ccache --show-stats
 
   # gcc, cassert on, dtrace off
   always:
@@ -548,7 +566,9 @@ task:
         ${LINUX_CONFIGURE_FEATURES} \
         CC="ccache gcc" CXX="ccache g++" CLANG="ccache clang"
       make -s -j${BUILD_JOBS} clean
+      ccache --zero-stats
       time make -s -j${BUILD_JOBS} world-bin
+      ccache --show-stats
 
   # clang, cassert off, dtrace off
   always:
@@ -558,7 +578,9 @@ task:
         ${LINUX_CONFIGURE_FEATURES} \
         CC="ccache clang" CXX="ccache clang++" CLANG="ccache clang"
       make -s -j${BUILD_JOBS} clean
+      ccache --zero-stats
       time make -s -j${BUILD_JOBS} world-bin
+      ccache --show-stats
 
   # clang, cassert on, dtrace on
   always:
@@ -570,7 +592,9 @@ task:
         ${LINUX_CONFIGURE_FEATURES} \
         CC="ccache clang" CXX="ccache clang++" CLANG="ccache clang"
       make -s -j${BUILD_JOBS} clean
+      ccache --zero-stats
       time make -s -j${BUILD_JOBS} world-bin
+      ccache --show-stats
 
   # cross-compile to windows
   always:
@@ -581,7 +605,9 @@ task:
         CC="ccache x86_64-w64-mingw32-gcc" \
         CXX="ccache x86_64-w64-mingw32-g++"
       make -s -j${BUILD_JOBS} clean
+      ccache --zero-stats
       time make -s -j${BUILD_JOBS} world-bin
+      ccache --show-stats
 
   ###
   # Verify docs can be built
-- 
2.17.1


--mln0rGgUGuXEqmuI
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="0014-another-way-to-install-uri-regress-uri_regress.patch"



^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* Re: Let's make PostgreSQL multi-threaded
@ 2023-06-05 17:29  Heikki Linnakangas <[email protected]>
  0 siblings, 1 reply; 16+ messages in thread

From: Heikki Linnakangas @ 2023-06-05 17:29 UTC (permalink / raw)
  To: Bruce Momjian <[email protected]>; +Cc: pgsql-hackers

On 05/06/2023 13:10, Bruce Momjian wrote:
> nOn Mon, Jun  5, 2023 at 05:51:57PM +0300, Heikki Linnakangas wrote:
>> # Restart on crash
>>
>> If a backend process crashes, postmaster terminates all other backends and
>> restarts the system. That's hard (impossible?) to do safely if everything
>> runs in one process. We can continue have a separate postmaster process that
>> just monitors the main process and restarts it on crash.
> 
> It would be good to know what new class of errors would cause server
> restarts, e.g., memory allocation failures?

You mean "out of memory"? No, that would be horrible.

I don't think there would be any new class of errors that would cause 
server restarts. In theory, having a separate address space for each 
backend gives you some protection. In practice, there are a lot of 
shared memory structures anyway that you can stomp over, and a segfault 
or unexpected exit of any backend process causes postmaster to restart 
the whole system anyway.

-- 
Heikki Linnakangas
Neon (https://neon.tech)







^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* Re: Let's make PostgreSQL multi-threaded
@ 2023-06-05 18:04  Bruce Momjian <[email protected]>
  parent: Heikki Linnakangas <[email protected]>
  0 siblings, 1 reply; 16+ messages in thread

From: Bruce Momjian @ 2023-06-05 18:04 UTC (permalink / raw)
  To: Heikki Linnakangas <[email protected]>; +Cc: pgsql-hackers

On Mon, Jun  5, 2023 at 08:29:16PM +0300, Heikki Linnakangas wrote:
> On 05/06/2023 13:10, Bruce Momjian wrote:
> > nOn Mon, Jun  5, 2023 at 05:51:57PM +0300, Heikki Linnakangas wrote:
> > > # Restart on crash
> > > 
> > > If a backend process crashes, postmaster terminates all other backends and
> > > restarts the system. That's hard (impossible?) to do safely if everything
> > > runs in one process. We can continue have a separate postmaster process that
> > > just monitors the main process and restarts it on crash.
> > 
> > It would be good to know what new class of errors would cause server
> > restarts, e.g., memory allocation failures?
> 
> You mean "out of memory"? No, that would be horrible.
> 
> I don't think there would be any new class of errors that would cause server
> restarts. In theory, having a separate address space for each backend gives
> you some protection. In practice, there are a lot of shared memory
> structures anyway that you can stomp over, and a segfault or unexpected exit
> of any backend process causes postmaster to restart the whole system anyway.

Uh, yes, but don't we detect failures while modifying shared memory and
force a restart?  Wouldn't the scope of failures be much larger?

-- 
  Bruce Momjian  <[email protected]>        https://momjian.us
  EDB                                      https://enterprisedb.com

  Only you can decide what is important to you.






^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* Re: Let's make PostgreSQL multi-threaded
@ 2023-06-05 18:30  Heikki Linnakangas <[email protected]>
  parent: Bruce Momjian <[email protected]>
  0 siblings, 1 reply; 16+ messages in thread

From: Heikki Linnakangas @ 2023-06-05 18:30 UTC (permalink / raw)
  To: Bruce Momjian <[email protected]>; +Cc: pgsql-hackers

On 05/06/2023 14:04, Bruce Momjian wrote:
> On Mon, Jun  5, 2023 at 08:29:16PM +0300, Heikki Linnakangas wrote:
>> I don't think there would be any new class of errors that would cause server
>> restarts. In theory, having a separate address space for each backend gives
>> you some protection. In practice, there are a lot of shared memory
>> structures anyway that you can stomp over, and a segfault or unexpected exit
>> of any backend process causes postmaster to restart the whole system anyway.
> 
> Uh, yes, but don't we detect failures while modifying shared memory and
> force a restart?  Wouldn't the scope of failures be much larger?

If one process writes over shared memory that it shouldn't, it can cause 
a crash in that process or some other process that reads it. Same with 
multiple threads, no difference there.

With a single process, one thread can modify another thread's "backend 
private" memory, and cause the other thread to crash. Perhaps that's 
what you meant?

In practice, I don't think it's so bad. Even in a multi-threaded 
environment, common bugs like buffer overflows and use-after-free are 
still much more likely to access memory owned by the same thread, thanks 
to how memory allocators work. And a completely random memory access is 
still more likely to cause a segfault than corrupting another thread's 
memory. And tools like CLOBBER_FREED_MEMORY/MEMORY_CONTEXT_CHECKING and 
valgrind are pretty good at catching memory access bugs at development 
time, whether it's multiple processes or threads.

-- 
Heikki Linnakangas
Neon (https://neon.tech)







^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* Re: Let's make PostgreSQL multi-threaded
@ 2023-06-05 23:26  Bruce Momjian <[email protected]>
  parent: Heikki Linnakangas <[email protected]>
  0 siblings, 1 reply; 16+ messages in thread

From: Bruce Momjian @ 2023-06-05 23:26 UTC (permalink / raw)
  To: Heikki Linnakangas <[email protected]>; +Cc: pgsql-hackers

On Mon, Jun  5, 2023 at 09:30:28PM +0300, Heikki Linnakangas wrote:
> If one process writes over shared memory that it shouldn't, it can cause a
> crash in that process or some other process that reads it. Same with
> multiple threads, no difference there.
> 
> With a single process, one thread can modify another thread's "backend
> private" memory, and cause the other thread to crash. Perhaps that's what
> you meant?
> 
> In practice, I don't think it's so bad. Even in a multi-threaded
> environment, common bugs like buffer overflows and use-after-free are still
> much more likely to access memory owned by the same thread, thanks to how
> memory allocators work. And a completely random memory access is still more
> likely to cause a segfault than corrupting another thread's memory. And
> tools like CLOBBER_FREED_MEMORY/MEMORY_CONTEXT_CHECKING and valgrind are
> pretty good at catching memory access bugs at development time, whether it's
> multiple processes or threads.

I remember we used to have macros we called before we modified critical
parts of shared memory, and if a process exited while in those blocks,
the server would restart.  Unfortunately, I can't find that in the code
now.

-- 
  Bruce Momjian  <[email protected]>        https://momjian.us
  EDB                                      https://enterprisedb.com

  Only you can decide what is important to you.






^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* Re: Let's make PostgreSQL multi-threaded
@ 2023-06-05 23:50  Peter Geoghegan <[email protected]>
  parent: Bruce Momjian <[email protected]>
  0 siblings, 1 reply; 16+ messages in thread

From: Peter Geoghegan @ 2023-06-05 23:50 UTC (permalink / raw)
  To: Bruce Momjian <[email protected]>; +Cc: Heikki Linnakangas <[email protected]>; pgsql-hackers

On Mon, Jun 5, 2023 at 4:26 PM Bruce Momjian <[email protected]> wrote:
> I remember we used to have macros we called before we modified critical
> parts of shared memory, and if a process exited while in those blocks,
> the server would restart.  Unfortunately, I can't find that in the code
> now.

Isn't that what we call a critical section? They effectively "promote"
any ERROR (e.g., from an OOM) into a PANIC.

I thought that we only used critical sections for things that are
WAL-logged, but I double checked just now. Turns out that I was wrong:
PGSTAT_BEGIN_WRITE_ACTIVITY() contains its own START_CRIT_SECTION(),
despite not being involved in WAL logging. And so critical sections
could indeed be described as something that we use whenever shared
memory cannot be left in an inconsistent state (which often coincides
with WAL logging, but need not).

-- 
Peter Geoghegan






^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* Re: Let's make PostgreSQL multi-threaded
@ 2023-06-06 00:15  Bruce Momjian <[email protected]>
  parent: Peter Geoghegan <[email protected]>
  0 siblings, 2 replies; 16+ messages in thread

From: Bruce Momjian @ 2023-06-06 00:15 UTC (permalink / raw)
  To: Peter Geoghegan <[email protected]>; +Cc: Heikki Linnakangas <[email protected]>; pgsql-hackers

On Mon, Jun  5, 2023 at 04:50:11PM -0700, Peter Geoghegan wrote:
> On Mon, Jun 5, 2023 at 4:26 PM Bruce Momjian <[email protected]> wrote:
> > I remember we used to have macros we called before we modified critical
> > parts of shared memory, and if a process exited while in those blocks,
> > the server would restart.  Unfortunately, I can't find that in the code
> > now.
> 
> Isn't that what we call a critical section? They effectively "promote"
> any ERROR (e.g., from an OOM) into a PANIC.
> 
> I thought that we only used critical sections for things that are
> WAL-logged, but I double checked just now. Turns out that I was wrong:
> PGSTAT_BEGIN_WRITE_ACTIVITY() contains its own START_CRIT_SECTION(),
> despite not being involved in WAL logging. And so critical sections
> could indeed be described as something that we use whenever shared
> memory cannot be left in an inconsistent state (which often coincides
> with WAL logging, but need not).

Yes, sorry, critical sections is what I was remembering.  My question is
whether all unexpected backend exits should be treated as critical
sections?

-- 
  Bruce Momjian  <[email protected]>        https://momjian.us
  EDB                                      https://enterprisedb.com

  Only you can decide what is important to you.






^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* Re: Let's make PostgreSQL multi-threaded
@ 2023-06-06 00:50  Peter Geoghegan <[email protected]>
  parent: Bruce Momjian <[email protected]>
  1 sibling, 0 replies; 16+ messages in thread

From: Peter Geoghegan @ 2023-06-06 00:50 UTC (permalink / raw)
  To: Bruce Momjian <[email protected]>; +Cc: Heikki Linnakangas <[email protected]>; pgsql-hackers

On Mon, Jun 5, 2023 at 5:15 PM Bruce Momjian <[email protected]> wrote:
> > Isn't that what we call a critical section? They effectively "promote"
> > any ERROR (e.g., from an OOM) into a PANIC.

> Yes, sorry, critical sections is what I was remembering.  My question is
> whether all unexpected backend exits should be treated as critical
> sections?

I think that it boils down to this: critical sections help us to avoid
various inconsistencies that might otherwise be introduced to critical
state, usually in shared memory. And so critical sections are mostly
about protecting truly crucial state, even in the presence of
irrecoverable problems (e.g., those caused by corruption that was
missed before the critical section was reached, fsync() reporting
failure on recent Postgres versions). This is mostly about the state
itself -- it's not about cleaning up from routine errors at all. The
server isn't supposed to PANIC, and won't unless some fundamental
assumption that the system makes isn't met.

I said that an OOM could cause a PANIC. But that really shouldn't be
possible in practice, since it can only happen when code in a critical
section actually attempts to allocate memory in the first place. There
is an assertion in palloc() that will catch code that violates that
rule. It has been known to happen from time to time, but theoretically
it should never happen.

Discussion about the robustness of threads versus processes seems to
only be concerned with what can happen after something "impossible"
takes place. Not before. Backend code is not supposed to corrupt
memory, whether shared or local, with or without threads. Code in
critical sections isn't supposed to even attempt memory allocation.
Jeremy and others have suggested that processes have significant
robustness advantages. Maybe they do, but it's hard to say either way
because these benefits only apply "when the impossible happens". In
any given case it's reasonable to wonder if the user was protected by
our multi-process architecture, or protected by dumb luck. Could even
be both.

-- 
Peter Geoghegan






^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* Re: Let's make PostgreSQL multi-threaded
@ 2023-06-07 21:45  Andres Freund <[email protected]>
  parent: Bruce Momjian <[email protected]>
  1 sibling, 1 reply; 16+ messages in thread

From: Andres Freund @ 2023-06-07 21:45 UTC (permalink / raw)
  To: Bruce Momjian <[email protected]>; +Cc: Peter Geoghegan <[email protected]>; Heikki Linnakangas <[email protected]>; pgsql-hackers

Hi,

On 2023-06-05 20:15:56 -0400, Bruce Momjian wrote:
> Yes, sorry, critical sections is what I was remembering.  My question is
> whether all unexpected backend exits should be treated as critical
> sections?

Yes.

People have argued that the process model is more robust. But it turns out
that we have to crash-restart for just about any "bad failure" anyway. It used
to be (a long time ago) that we didn't, but that was just broken.

There are some advantages in debuggability, because it's a *tad* harder for a
bug in one process to cause another to crash, if less state is shared. But
that's by far outweighed by most debugging / validation tools not
understanding the multi-processes-with-shared-shmem model.

Greetings,

Andres Freund






^ permalink  raw  reply  [nested|flat] 16+ messages in thread

* Re: Let's make PostgreSQL multi-threaded
@ 2023-06-08 14:17  Robert Haas <[email protected]>
  parent: Andres Freund <[email protected]>
  0 siblings, 0 replies; 16+ messages in thread

From: Robert Haas @ 2023-06-08 14:17 UTC (permalink / raw)
  To: Andres Freund <[email protected]>; +Cc: Bruce Momjian <[email protected]>; Peter Geoghegan <[email protected]>; Heikki Linnakangas <[email protected]>; pgsql-hackers

On Wed, Jun 7, 2023 at 5:45 PM Andres Freund <[email protected]> wrote:
> People have argued that the process model is more robust. But it turns out
> that we have to crash-restart for just about any "bad failure" anyway. It used
> to be (a long time ago) that we didn't, but that was just broken.

How hard have you thought about memory leaks as a failure mode? Or
file descriptor leaks?

Right now, a process needs to release all of its shared resources
before exiting, or trigger a crash-and-restart cycle. But it doesn't
need to release any process-local resources, because the OS will take
care of that. But that wouldn't be true any more, and that seems like
it might require fixing quite a few things.

-- 
Robert Haas
EDB: http://www.enterprisedb.com






^ permalink  raw  reply  [nested|flat] 16+ messages in thread


end of thread, other threads:[~2023-06-08 14:17 UTC | newest]

Thread overview: 16+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2022-04-03 05:10 [PATCH 03/23] cirrus/ccache: disable compression and show stats Justin Pryzby <[email protected]>
2022-04-03 05:10 [PATCH 13/21] cirrus/ccache: disable compression and show stats Justin Pryzby <[email protected]>
2022-04-03 05:10 [PATCH 05/11] cirrus/ccache: disable compression and show stats Justin Pryzby <[email protected]>
2022-04-03 05:10 [PATCH 7/8] cirrus/ccache: disable compression and show stats Justin Pryzby <[email protected]>
2022-04-03 05:10 [PATCH 03/25] cirrus/ccache: disable compression and show stats Justin Pryzby <[email protected]>
2022-04-03 05:10 [PATCH 13/21] cirrus/ccache: disable compression and show stats Justin Pryzby <[email protected]>
2022-04-03 05:10 [PATCH 03/23] cirrus/ccache: disable compression and show stats Justin Pryzby <[email protected]>
2023-06-05 17:29 Re: Let's make PostgreSQL multi-threaded Heikki Linnakangas <[email protected]>
2023-06-05 18:04 ` Re: Let's make PostgreSQL multi-threaded Bruce Momjian <[email protected]>
2023-06-05 18:30   ` Re: Let's make PostgreSQL multi-threaded Heikki Linnakangas <[email protected]>
2023-06-05 23:26     ` Re: Let's make PostgreSQL multi-threaded Bruce Momjian <[email protected]>
2023-06-05 23:50       ` Re: Let's make PostgreSQL multi-threaded Peter Geoghegan <[email protected]>
2023-06-06 00:15         ` Re: Let's make PostgreSQL multi-threaded Bruce Momjian <[email protected]>
2023-06-06 00:50           ` Re: Let's make PostgreSQL multi-threaded Peter Geoghegan <[email protected]>
2023-06-07 21:45           ` Re: Let's make PostgreSQL multi-threaded Andres Freund <[email protected]>
2023-06-08 14:17             ` Re: Let's make PostgreSQL multi-threaded Robert Haas <[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