public inbox for [email protected]help / color / mirror / Atom feed
[PATCH 07/10] add wal_compression_method: zstd 4+ messages / 4 participants [nested] [flat]
* [PATCH 07/10] add wal_compression_method: zstd @ 2021-03-12 20:43 Justin Pryzby <[email protected]> 0 siblings, 0 replies; 4+ messages in thread From: Justin Pryzby @ 2021-03-12 20:43 UTC (permalink / raw) --- configure | 163 ++++++++++++++++++ configure.ac | 26 +++ doc/src/sgml/config.sgml | 2 +- src/backend/access/transam/xlog.c | 3 + src/backend/access/transam/xloginsert.c | 19 ++ src/backend/access/transam/xlogreader.c | 16 ++ src/backend/utils/misc/postgresql.conf.sample | 2 +- src/include/access/xlog_internal.h | 1 + src/include/pg_config.h.in | 3 + src/tools/msvc/Solution.pm | 1 + 10 files changed, 234 insertions(+), 2 deletions(-) diff --git a/configure b/configure index 8d76be00c1..81e23418b2 100755 --- a/configure +++ b/configure @@ -699,6 +699,9 @@ with_gnu_ld LD LDFLAGS_SL LDFLAGS_EX +ZSTD_LIBS +ZSTD_CFLAGS +with_zstd LZ4_LIBS LZ4_CFLAGS with_lz4 @@ -868,6 +871,7 @@ with_libxslt with_system_tzdata with_zlib with_lz4 +with_zstd with_gnu_ld with_ssl with_openssl @@ -897,6 +901,8 @@ XML2_CFLAGS XML2_LIBS LZ4_CFLAGS LZ4_LIBS +ZSTD_CFLAGS +ZSTD_LIBS LDFLAGS_EX LDFLAGS_SL PERL @@ -1576,6 +1582,7 @@ Optional Packages: use system time zone data in DIR --without-zlib do not use Zlib --without-lz4 build without LZ4 support + --with-zstd build with Zstd compression library --with-gnu-ld assume the C compiler uses GNU ld [default=no] --with-ssl=LIB use LIB for SSL/TLS support (openssl) --with-openssl obsolete spelling of --with-ssl=openssl @@ -1605,6 +1612,8 @@ Some influential environment variables: 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 + ZSTD_CFLAGS C compiler flags for ZSTD, overriding pkg-config + ZSTD_LIBS linker flags for ZSTD, 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 @@ -8705,6 +8714,137 @@ fi CFLAGS="$LZ4_CFLAGS $CFLAGS" fi +# +# ZSTD +# +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking whether to build with zstd support" >&5 +$as_echo_n "checking whether to build with zstd support... " >&6; } + + + +# Check whether --with-zstd was given. +if test "${with_zstd+set}" = set; then : + withval=$with_zstd; + case $withval in + yes) + +$as_echo "#define USE_ZSTD 1" >>confdefs.h + + ;; + no) + : + ;; + *) + as_fn_error $? "no argument expected for --with-zstd option" "$LINENO" 5 + ;; + esac + +else + with_zstd=no + +fi + + +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $with_zstd" >&5 +$as_echo "$with_zstd" >&6; } + + +if test "$with_zstd" = yes; then + +pkg_failed=no +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for libzstd" >&5 +$as_echo_n "checking for libzstd... " >&6; } + +if test -n "$ZSTD_CFLAGS"; then + pkg_cv_ZSTD_CFLAGS="$ZSTD_CFLAGS" + elif test -n "$PKG_CONFIG"; then + if test -n "$PKG_CONFIG" && \ + { { $as_echo "$as_me:${as_lineno-$LINENO}: \$PKG_CONFIG --exists --print-errors \"libzstd\""; } >&5 + ($PKG_CONFIG --exists --print-errors "libzstd") 2>&5 + ac_status=$? + $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5 + test $ac_status = 0; }; then + pkg_cv_ZSTD_CFLAGS=`$PKG_CONFIG --cflags "libzstd" 2>/dev/null` + test "x$?" != "x0" && pkg_failed=yes +else + pkg_failed=yes +fi + else + pkg_failed=untried +fi +if test -n "$ZSTD_LIBS"; then + pkg_cv_ZSTD_LIBS="$ZSTD_LIBS" + elif test -n "$PKG_CONFIG"; then + if test -n "$PKG_CONFIG" && \ + { { $as_echo "$as_me:${as_lineno-$LINENO}: \$PKG_CONFIG --exists --print-errors \"libzstd\""; } >&5 + ($PKG_CONFIG --exists --print-errors "libzstd") 2>&5 + ac_status=$? + $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5 + test $ac_status = 0; }; then + pkg_cv_ZSTD_LIBS=`$PKG_CONFIG --libs "libzstd" 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 + ZSTD_PKG_ERRORS=`$PKG_CONFIG --short-errors --print-errors --cflags --libs "libzstd" 2>&1` + else + ZSTD_PKG_ERRORS=`$PKG_CONFIG --print-errors --cflags --libs "libzstd" 2>&1` + fi + # Put the nasty error message in config.log where it belongs + echo "$ZSTD_PKG_ERRORS" >&5 + + as_fn_error $? "Package requirements (libzstd) were not met: + +$ZSTD_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 ZSTD_CFLAGS +and ZSTD_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 ZSTD_CFLAGS +and ZSTD_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 + ZSTD_CFLAGS=$pkg_cv_ZSTD_CFLAGS + ZSTD_LIBS=$pkg_cv_ZSTD_LIBS + { $as_echo "$as_me:${as_lineno-$LINENO}: result: yes" >&5 +$as_echo "yes" >&6; } + +fi + LIBS="$ZSTD_LIBS $LIBS" + CFLAGS="$ZSTD_CFLAGS $CFLAGS" +fi + # # Assignments # @@ -13559,6 +13699,29 @@ done CPPFLAGS=$ac_save_CPPFLAGS fi +if test "$with_zstd" = yes; then + ac_save_CPPFLAGS=$CPPFLAGS + CPPFLAGS="$ZSTD_CFLAGS $CPPFLAGS" + + # Verify we have zstd's header files + for ac_header in zstd.h +do : + ac_fn_c_check_header_mongrel "$LINENO" "zstd.h" "ac_cv_header_zstd_h" "$ac_includes_default" +if test "x$ac_cv_header_zstd_h" = xyes; then : + cat >>confdefs.h <<_ACEOF +#define HAVE_ZSTD_H 1 +_ACEOF + +else + as_fn_error $? "zstd.h header file is required for zstd" "$LINENO" 5 +fi + +done + + + CPPFLAGS=$ac_save_CPPFLAGS +fi + if test "$with_gssapi" = yes ; then for ac_header in gssapi/gssapi.h do : diff --git a/configure.ac b/configure.ac index bfcdc88be0..d6f6349067 100644 --- a/configure.ac +++ b/configure.ac @@ -1001,6 +1001,21 @@ if test "$with_lz4" = yes; then CFLAGS="$LZ4_CFLAGS $CFLAGS" fi +# +# ZSTD +# +AC_MSG_CHECKING([whether to build with zstd support]) +PGAC_ARG_BOOL(with, zstd, no, [build with Zstd compression library], + [AC_DEFINE([USE_ZSTD], 1, [Define to 1 to build with zstd support. (--with-zstd)])]) +AC_MSG_RESULT([$with_zstd]) +AC_SUBST(with_zstd) + +if test "$with_zstd" = yes; then + PKG_CHECK_MODULES(ZSTD, libzstd) + LIBS="$ZSTD_LIBS $LIBS" + CFLAGS="$ZSTD_CFLAGS $CFLAGS" +fi + # # Assignments # @@ -1436,6 +1451,17 @@ if test "$with_lz4" = yes; then CPPFLAGS=$ac_save_CPPFLAGS fi +if test "$with_zstd" = yes; then + ac_save_CPPFLAGS=$CPPFLAGS + CPPFLAGS="$ZSTD_CFLAGS $CPPFLAGS" + + # Verify we have zstd's header files + AC_CHECK_HEADERS(zstd.h, [], + [AC_MSG_ERROR([zstd.h header file is required for zstd])]) + + CPPFLAGS=$ac_save_CPPFLAGS +fi + if test "$with_gssapi" = yes ; then AC_CHECK_HEADERS(gssapi/gssapi.h, [], [AC_CHECK_HEADERS(gssapi.h, [], [AC_MSG_ERROR([gssapi.h header file is required for GSSAPI])])]) diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index 257775c83b..94dd6ef3e9 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -3083,7 +3083,7 @@ include_dir 'conf.d' This parameter selects the compression method used to compress WAL when <varname>wal_compression</varname> is enabled. The supported methods are pglz, zlib, and (if configured when - <productname>PostgreSQL</productname> was built) lz4. + <productname>PostgreSQL</productname> was built) lz4 and zstd. The default value is <literal>pglz</literal>. Only superusers can change this setting. </para> diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 3657f74de9..307eee6626 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -189,6 +189,9 @@ const struct config_enum_entry wal_compression_options[] = { #endif #ifdef USE_LZ4 {"lz4", WAL_COMPRESSION_LZ4, false}, +#endif +#ifdef USE_ZSTD + {"zstd", WAL_COMPRESSION_ZSTD, false}, #endif {NULL, 0, false} }; diff --git a/src/backend/access/transam/xloginsert.c b/src/backend/access/transam/xloginsert.c index 9fe5c30236..5d1ae37dae 100644 --- a/src/backend/access/transam/xloginsert.c +++ b/src/backend/access/transam/xloginsert.c @@ -41,6 +41,10 @@ #include "lz4.h" #endif +#ifdef USE_ZSTD +#include "zstd.h" +#endif + /* Buffer size required to store a compressed version of backup block image */ #define PGLZ_MAX_BLCKSZ PGLZ_MAX_OUTPUT(BLCKSZ) @@ -896,6 +900,21 @@ XLogCompressBackupBlock(char *page, uint16 hole_offset, uint16 hole_length, break; #endif +#ifdef USE_ZSTD + case WAL_COMPRESSION_ZSTD: + len = ZSTD_compress(dest, PGLZ_MAX_BLCKSZ, source, orig_len, 1); + if (ZSTD_isError(len)) + { + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("failed compressing zstd: %s", + ZSTD_getErrorName(len)))); + len = -1; + } + + break; +#endif + default: /* * It should be impossible to get here for unsupported algorithms, diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c index fa1e38a810..caa1031d63 100644 --- a/src/backend/access/transam/xlogreader.c +++ b/src/backend/access/transam/xlogreader.c @@ -41,6 +41,10 @@ #include "lz4.h" #endif +#ifdef USE_ZSTD +#include "zstd.h" +#endif + static void report_invalid_record(XLogReaderState *state, const char *fmt,...) pg_attribute_printf(2, 3); static bool allocate_recordbuf(XLogReaderState *state, uint32 reclength); @@ -1564,6 +1568,8 @@ wal_compression_name(WalCompression compression) return "zlib"; case WAL_COMPRESSION_LZ4: return "lz4"; + case WAL_COMPRESSION_ZSTD: + return "zstd"; default: return "???"; } @@ -1621,6 +1627,16 @@ RestoreBlockImage(XLogReaderState *record, uint8 block_id, char *page) break; #endif +#ifdef USE_ZSTD + case WAL_COMPRESSION_ZSTD: + decomp_result = ZSTD_decompress(tmp.data, BLCKSZ-bkpb->hole_length, + ptr, bkpb->bimg_len); + // XXX: ZSTD_getErrorName + if (ZSTD_isError(decomp_result)) + decomp_result = -1; + break; +#endif + default: report_invalid_record(record, "image at %X/%X is compressed with unsupported codec, block %d (%d/%s)", (uint32) (record->ReadRecPtr >> 32), diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample index 76f494cb9b..d372e2a817 100644 --- a/src/backend/utils/misc/postgresql.conf.sample +++ b/src/backend/utils/misc/postgresql.conf.sample @@ -213,7 +213,7 @@ # open_sync #full_page_writes = on # recover from partial page writes #wal_compression = off # enable compression of full-page writes -#wal_compression_method = pglz # pglz, zlib, lz4 +#wal_compression_method = pglz # pglz, zlib, lz4, zstd #wal_log_hints = off # also do full page writes of non-critical updates # (change requires restart) #wal_init_zero = on # zero-fill new WAL files diff --git a/src/include/access/xlog_internal.h b/src/include/access/xlog_internal.h index e70886b81c..48b16b6083 100644 --- a/src/include/access/xlog_internal.h +++ b/src/include/access/xlog_internal.h @@ -329,6 +329,7 @@ typedef enum WalCompression WAL_COMPRESSION_PGLZ, WAL_COMPRESSION_ZLIB, WAL_COMPRESSION_LZ4, + WAL_COMPRESSION_ZSTD, } WalCompression; extern const char *wal_compression_name(WalCompression compression); diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in index 0a6422da4f..ad26393352 100644 --- a/src/include/pg_config.h.in +++ b/src/include/pg_config.h.in @@ -902,6 +902,9 @@ /* Define to 1 to build with LZ4 support (--with-lz4) */ #undef USE_LZ4 +/* Define to 1 if you have the `zstd' library (-lzstd). */ +#undef USE_ZSTD + /* 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 14605371bb..fff0212087 100644 --- a/src/tools/msvc/Solution.pm +++ b/src/tools/msvc/Solution.pm @@ -486,6 +486,7 @@ sub GenerateFiles USE_LIBXML => undef, USE_LIBXSLT => undef, USE_LZ4 => undef, + USE_ZSTD => $self->{options}->{zstd} ? 1 : undef, USE_LDAP => $self->{options}->{ldap} ? 1 : undef, USE_LLVM => undef, USE_NAMED_POSIX_SEMAPHORES => undef, -- 2.17.0 --XsQoSWH+UP9D9v3l Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0008-Default-to-zstd.patch" ^ permalink raw reply [nested|flat] 4+ messages in thread
* [PATCH v4 1/3] Rearrange CLUSTER rules in gram.y. @ 2023-05-15 19:02 Nathan Bossart <[email protected]> 0 siblings, 0 replies; 4+ messages in thread From: Nathan Bossart @ 2023-05-15 19:02 UTC (permalink / raw) This change moves the unparenthesized syntaxes for CLUSTER to the end of the ClusterStmt rules in preparation for a follow-up commit that will move these syntaxes to the "Compatibility" section of the CLUSTER documentation. The documentation for the unparenthesized syntaxes has also been consolidated. Suggested-by: Melanie Plageman Discussion https://postgr.es/m/CAAKRu_bc5uHieG1976kGqJKxyWtyQt9yvktjsVX%2Bi7NOigDjOA%40mail.gmail.com --- doc/src/sgml/ref/cluster.sgml | 3 +-- src/backend/parser/gram.y | 27 +++++++++++++-------------- 2 files changed, 14 insertions(+), 16 deletions(-) diff --git a/doc/src/sgml/ref/cluster.sgml b/doc/src/sgml/ref/cluster.sgml index 29f0f1fd90..35b8deaec1 100644 --- a/doc/src/sgml/ref/cluster.sgml +++ b/doc/src/sgml/ref/cluster.sgml @@ -21,9 +21,8 @@ PostgreSQL documentation <refsynopsisdiv> <synopsis> -CLUSTER [VERBOSE] <replaceable class="parameter">table_name</replaceable> [ USING <replaceable class="parameter">index_name</replaceable> ] CLUSTER ( <replaceable class="parameter">option</replaceable> [, ...] ) <replaceable class="parameter">table_name</replaceable> [ USING <replaceable class="parameter">index_name</replaceable> ] -CLUSTER [VERBOSE] +CLUSTER [VERBOSE] [ replaceable class="parameter">table_name</replaceable> [ USING <replaceable class="parameter">index_name</replaceable> ] ] <phrase>where <replaceable class="parameter">option</replaceable> can be one of:</phrase> diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index d6426f3b8e..a0b741ea72 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -11574,33 +11574,32 @@ CreateConversionStmt: /***************************************************************************** * * QUERY: - * CLUSTER [VERBOSE] <qualified_name> [ USING <index_name> ] - * CLUSTER [ (options) ] <qualified_name> [ USING <index_name> ] - * CLUSTER [VERBOSE] + * CLUSTER (options) <qualified_name> [ USING <index_name> ] + * CLUSTER [VERBOSE] [ <qualified_name> [ USING <index_name> ] ] * CLUSTER [VERBOSE] <index_name> ON <qualified_name> (for pre-8.3) * *****************************************************************************/ ClusterStmt: - CLUSTER opt_verbose qualified_name cluster_index_specification + CLUSTER '(' utility_option_list ')' qualified_name cluster_index_specification { ClusterStmt *n = makeNode(ClusterStmt); - n->relation = $3; - n->indexname = $4; - n->params = NIL; - if ($2) - n->params = lappend(n->params, makeDefElem("verbose", NULL, @2)); + n->relation = $5; + n->indexname = $6; + n->params = $3; $$ = (Node *) n; } - - | CLUSTER '(' utility_option_list ')' qualified_name cluster_index_specification + /* unparenthesized VERBOSE kept for pre-14 compatibility */ + | CLUSTER opt_verbose qualified_name cluster_index_specification { ClusterStmt *n = makeNode(ClusterStmt); - n->relation = $5; - n->indexname = $6; - n->params = $3; + n->relation = $3; + n->indexname = $4; + n->params = NIL; + if ($2) + n->params = lappend(n->params, makeDefElem("verbose", NULL, @2)); $$ = (Node *) n; } | CLUSTER opt_verbose -- 2.25.1 --/9DWx/yDrRhgMJTb Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v4-0002-Support-parenthesized-syntax-for-CLUSTER-without-.patch" ^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: [PROPOSAL] Doublewrite Buffer as an alternative torn page protection to Full Page Write @ 2026-02-26 07:13 陈宗志 <[email protected]> 2026-03-04 04:42 ` Re: [PROPOSAL] Doublewrite Buffer as an alternative torn page protection to Full Page Write Robert Treat <[email protected]> 0 siblings, 1 reply; 4+ messages in thread From: 陈宗志 @ 2026-02-26 07:13 UTC (permalink / raw) To: Robert Treat <[email protected]>; +Cc: Jakub Wartak <[email protected]>; pgsql-hackers Hi Robert, Thanks for the feedback and suggestions. > I'm somewhat less of a noob here, so I'll confirm that this proposal > has basically zero chance of getting in, at least for the v19 cycle. > This isn't so much about the proposal itself, but more in that if you > were trying to pick the worst time of year to submit a large, > complicated feature into the postgresql workflow, this would be really > close to that. > However, I have also wondered about this specific trade-off (FPW vs > DWB) for years, but until now, the level of effort required to produce > a meaningful POC that would confirm if the idea was worth pursuing was > so large that I think it stopped anyone from even trying. So, > hopefully everyone will realize that we don't live in that world > anymore, and as a side benefit, apparently the idea is worth pursuing. I completely understand, and I actually have no intention of pushing this patch for the v19 cycle. My primary goal right now is simply to share the POC results and discuss the idea with the community to see if this direction is worth pursuing. For context, I have been a MySQL InnoDB developer for over 10 years, but I admit I am a newcomer to the PostgreSQL community, so I am still familiarizing myself with the standard workflow and processes here. >> I think it would be valuable to have this as I've been hit by PostgreSQL's >> unsteady (chain-saw-like) WAL traffic, especially related to touching 1st the >> pages after checkpoint, up to the point of saturating network links. The common >> counter-argument to double buffering is probably that FPI may(?) increase WAL >> standby replication rate and this would have to be measured into account >> (but we also should take into account how much maintenance_io_concurrency/ >> posix_fadvise() prefetching that we do today helps avoid any I/O stalls on >> fetching pages - so it should be basically free), I see even that you >> got benefits >> by not using FPI. Interesting. >> >> Some notes/questions about the patches itself: >> > So, I haven't looked at the code itself; tbh honest I am a bit too > paranoid to dive into generated code that would seem to carry some > likely level of legal risk around potential reuse of GPL/proprietary > code it might be based on (either in its original training, inference, > or context used for generation. Yeah, I know innodb isn't written in > C, but still). That said, I did have some feedback and questions on > the proposal itself, and some suggestions for how to move things > forward. > >> 0. The convention here is send the patches using: >> git format-patch -v<VERSION> HEAD~<numberOfpatches> >> for easier review. The 0003 probably should be out of scope. Anyway I've >> attached all of those so maybe somebody else is going to take a >> look at them too, >> they look very mature. Is this code used in production already anywhere? (and >> BTW the numbers are quite impressive) >> > While Jakub is right that the convention is to send patches, that > convention is based on a manual development model, not an agentic > development model. While there is no official project policy on this, > IMHO the thing we really need from you is not the code output, but the > prompts that were used to generate the code. There are plenty of folks > who have access to claude that could then use those prompts to > "recreate with enough proximity" the work you had claude do, and that > process would also allow for additional verification and reduction of > any legal concerns or concerns about investing further human > time/energy. (No offense, but as you are not a regular contributor, > you could analogize this to when third parties do large code dumps and > say "here's a contribution, it's up to you to figure out how to use > it". Ideally we want other folks to be able to pick up the project and > continue with it, even if it means recreating it, and that works best > if we have the underlying prompts). > The claude code configuration file is a good start, but certainly not > enough. Probably the ideal here would be full session logs, although a > developer-diary would probably also suffice. I'm kind of guessing here > because I don't know the scope of the prompts involved or how you were > interacting with Claude in order to get where you are now, but those > seem like the more obvious tools for work of this size whose intention > is to be open. Regarding the AI-generated code, the raw output from Claude was far from perfect. I have manually reviewed and modified the code extensively to get it to this state. Our plan is to first deploy and test this thoroughly on our own product, Alibaba Cloud RDS for PostgreSQL. Once we are confident that it is stable and issue-free, we intend to submit a formalized patch to the community. I am very much looking forward to discussing and reviewing the actual code with you all when the time comes. As for sharing the prompts or session logs, I personally feel they might not be as valuable as the final code itself. The generation process involved a lot of iterative, back-and-forth communication; the AI only knew how to make the right modifications after continuous human guidance, correction, and architectural decisions. > I would be helpful if you could provide a little more information on > the system you are running these benchmarks on, specifically for me > the underlying OS/Filesystem/hardware, and I'd even be interested in > the build flags. I'd also be interested to know if you did any kind of > crash safety testing... while it is great to have improved > performance, presumably that isn't actually the primary point of these > subsystems. It'd also be worth knowing if you tested this on any > systems with replication (physical or logical) since we'd need to > understand those potential downstream effects. I'm tempted to say you > should have an AI generate some pgbench scripts. Granted its early and > fine if you have done any of this, but I imagine we'll need to look at > it eventually. I have addressed the feedback and conducted comprehensive benchmarks comparing the three io_torn_pages_protection modes. Here are the detailed performance results and the system setup information you requested. Benchmark Setup: - Hardware: x86_64, Linux 5.10, NVMe SSD - PostgreSQL: 19devel (with DWB patch applied) - Tool: pgbench (TPC-B), 64 clients, 8 threads, 60 seconds per run - Common config: shared_buffers = 1GB, wal_level = replica - Three modes tested: * io_torn_pages_protection = full_pages (traditional FPW) * io_torn_pages_protection = double_writes (DWB size = 128MB) * io_torn_pages_protection = off (no protection, baseline) Each test was run sequentially on the same machine to avoid I/O contention. Test 1: pgbench scale=100 (~1.5GB dataset), max_wal_size = 10GB With infrequent checkpoints, FPW overhead is minimal and all three modes perform similarly: Mode TPS Latency(avg) WAL Size FPI Count FPI Size ----------- ---------- ------------ --------- ---------- --------- full_pages 103,290 0.610 ms 3,903 MB 191,341 1,456 MB double_writes 104,088 0.606 ms 2,475 MB 0 0 off 104,622 0.602 ms 2,510 MB 0 0 DWB vs FPW: +0.8% TPS, WAL reduced by 36.6%. Test 2: pgbench scale=100 (~1.5GB dataset), max_wal_size = 64MB With frequent checkpoints (triggered every ~64MB of WAL), the FPW write amplification becomes severe: Mode TPS Latency(avg) WAL Size FPI Count FPI Size ----------- ---------- ------------ --------- ----------- --------- full_pages 54,324 1.171 ms 29 GB 3,806,504 28 GB double_writes 93,942 0.672 ms 2,303 MB 2 16 kB off 108,901 0.578 ms 2,746 MB 0 0 DWB vs FPW: +72.9% TPS, latency reduced by 42.6%, WAL reduced by 92.2%. Test 3: pgbench scale=10 (~150MB dataset), max_wal_size = 64MB Even with a smaller dataset that fits in shared_buffers, the advantage is clear: Mode TPS Latency(avg) WAL Size FPI Count FPI Size ----------- ---------- ------------ --------- ----------- --------- full_pages 33,707 1.895 ms 15 GB 2,010,439 14 GB double_writes 43,743 1.459 ms 1,140 MB 0 0 off 43,982 1.452 ms 1,150 MB 0 0 DWB vs FPW: +29.8% TPS, WAL reduced by 92.6%. Test 4: sysbench oltp_write_only (10 tables x 100K rows), 64 threads, 30s max_wal_size=10GB max_wal_size=64MB -------------------- -------------------- Mode TPS FPI Count TPS FPI Count ----------- -------- ---------- -------- ---------- full_pages 138,019 32,253 40,187 3,455,253 double_writes 136,642 0 133,034 0 With large max_wal_size: DWB and FPW perform equally (-1.0%). With small max_wal_size: DWB is +231% faster (3.3x). Analysis: The key factor is checkpoint frequency. FPW must write a full 8KB page image to WAL for every page's first modification after each checkpoint. When checkpoints are frequent: - FPI count explodes (32K -> 3.8M with scale=100) - WAL becomes dominated by FPI data (28GB out of 29GB = 96.6%) - This creates massive write amplification on the WAL path DWB avoids this entirely. Its WAL size stays constant regardless of checkpoint frequency (~2.3GB vs FPW's 29GB). The double-write buffer itself wrote 42GB in the heavy test, but since it uses sequential writes with batched fsync, the overhead is modest — DWB achieves 86.3% of the "no protection" baseline (93,942 vs 108,901 TPS). When does DWB matter most? - Large active datasets that exceed shared_buffers - Frequent checkpoints (small max_wal_size or short checkpoint_timeout) - Write-heavy workloads - Replication scenarios where WAL volume directly impacts network In production environments where max_wal_size is often set conservatively (e.g., 1GB) and datasets are much larger than shared_buffers, DWB should provide significant and consistent benefits over FPW. As for the crash safety testing you mentioned, it is on our roadmap as we continue to refine the patch for our internal RDS deployment. Regards, Baotiao ^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: [PROPOSAL] Doublewrite Buffer as an alternative torn page protection to Full Page Write 2026-02-26 07:13 Re: [PROPOSAL] Doublewrite Buffer as an alternative torn page protection to Full Page Write 陈宗志 <[email protected]> @ 2026-03-04 04:42 ` Robert Treat <[email protected]> 0 siblings, 0 replies; 4+ messages in thread From: Robert Treat @ 2026-03-04 04:42 UTC (permalink / raw) To: 陈宗志 <[email protected]>; +Cc: Jakub Wartak <[email protected]>; pgsql-hackers On Thu, Feb 26, 2026 at 2:14 AM 陈宗志 <[email protected]> wrote: > > Hi Robert, > > Thanks for the feedback and suggestions. > <snip> > > So, I haven't looked at the code itself; tbh honest I am a bit too > > paranoid to dive into generated code that would seem to carry some > > likely level of legal risk around potential reuse of GPL/proprietary > > code it might be based on (either in its original training, inference, > > or context used for generation. Yeah, I know innodb isn't written in > > C, but still). That said, I did have some feedback and questions on > > the proposal itself, and some suggestions for how to move things > > forward. > > > >> 0. The convention here is send the patches using: > >> git format-patch -v<VERSION> HEAD~<numberOfpatches> > >> for easier review. The 0003 probably should be out of scope. Anyway I've > >> attached all of those so maybe somebody else is going to take a > >> look at them too, > >> they look very mature. Is this code used in production already anywhere? (and > >> BTW the numbers are quite impressive) > >> > > While Jakub is right that the convention is to send patches, that > > convention is based on a manual development model, not an agentic > > development model. While there is no official project policy on this, > > IMHO the thing we really need from you is not the code output, but the > > prompts that were used to generate the code. There are plenty of folks > > who have access to claude that could then use those prompts to > > "recreate with enough proximity" the work you had claude do, and that > > process would also allow for additional verification and reduction of > > any legal concerns or concerns about investing further human > > time/energy. (No offense, but as you are not a regular contributor, > > you could analogize this to when third parties do large code dumps and > > say "here's a contribution, it's up to you to figure out how to use > > it". Ideally we want other folks to be able to pick up the project and > > continue with it, even if it means recreating it, and that works best > > if we have the underlying prompts). > > The claude code configuration file is a good start, but certainly not > > enough. Probably the ideal here would be full session logs, although a > > developer-diary would probably also suffice. I'm kind of guessing here > > because I don't know the scope of the prompts involved or how you were > > interacting with Claude in order to get where you are now, but those > > seem like the more obvious tools for work of this size whose intention > > is to be open. > > Regarding the AI-generated code, the raw output from Claude was far > from perfect. I have manually reviewed and modified the code > extensively to get it to this state. > > Our plan is to first deploy and test this thoroughly on our own > product, Alibaba Cloud RDS for PostgreSQL. Once we are confident that > it is stable and issue-free, we intend to submit a formalized patch > to the community. I am very much looking forward to discussing and > reviewing the actual code with you all when the time comes. > > As for sharing the prompts or session logs, I personally feel they > might not be as valuable as the final code itself. The generation > process involved a lot of iterative, back-and-forth communication; > the AI only knew how to make the right modifications after continuous > human guidance, correction, and architectural decisions. > Yeah, this is an issue which doesn't seem like we have very good answers to at the moment. Part of me thinks the right path is to require completely open transcripts of this back and forth, like you would see in a discussion of developers on the mailing list. OTOH, lots of patches have gone through "pre-development" work before hitting the mailing lists, not to mention that agents can operate in ways that are so ridiculously verbose that the idea of having these kinds of logs for every developer doesn't sound like it would scale, if it even remained useful. In any case, as you stated you were a former innodb developer, clearly you would understand concerns about potential ip muddiness, so to that end I decided to spin up my own agent to have it examine your patch vs the innodb implementation and provide an analysis contrasting the two implementations, and while no one should mistake that as something official, the initial read through was comforting. > > I would be helpful if you could provide a little more information on > > the system you are running these benchmarks on, specifically for me > > the underlying OS/Filesystem/hardware, and I'd even be interested in > > the build flags. I'd also be interested to know if you did any kind of > > crash safety testing... while it is great to have improved > > performance, presumably that isn't actually the primary point of these > > subsystems. It'd also be worth knowing if you tested this on any > > systems with replication (physical or logical) since we'd need to > > understand those potential downstream effects. I'm tempted to say you > > should have an AI generate some pgbench scripts. Granted its early and > > fine if you have done any of this, but I imagine we'll need to look at > > it eventually. > > I have addressed the feedback and conducted comprehensive benchmarks > comparing the three io_torn_pages_protection modes. Here are the > detailed performance results and the system setup information you > requested. > > Benchmark Setup: > - Hardware: x86_64, Linux 5.10, NVMe SSD > - PostgreSQL: 19devel (with DWB patch applied) > - Tool: pgbench (TPC-B), 64 clients, 8 threads, 60 seconds per run > - Common config: shared_buffers = 1GB, wal_level = replica > - Three modes tested: > * io_torn_pages_protection = full_pages (traditional FPW) > * io_torn_pages_protection = double_writes (DWB size = 128MB) > * io_torn_pages_protection = off (no protection, baseline) > > Each test was run sequentially on the same machine to avoid I/O > contention. > <snip> > Analysis: > > The key factor is checkpoint frequency. FPW must write a full 8KB page > image to WAL for every page's first modification after each checkpoint. > When checkpoints are frequent: > <snip> > When does DWB matter most? > - Large active datasets that exceed shared_buffers > - Frequent checkpoints (small max_wal_size or short checkpoint_timeout) > - Write-heavy workloads > - Replication scenarios where WAL volume directly impacts network > > In production environments where max_wal_size is often set > conservatively (e.g., 1GB) and datasets are much larger than > shared_buffers, DWB should provide significant and consistent benefits > over FPW. As for the crash safety testing you mentioned, it is on our > roadmap as we continue to refine the patch for our internal RDS > deployment. > I suspect that some folks would argue that the problem is as much users with poorly configured servers (primarily undersized max_wal_size and too frequent checkpointing) as it is them needing an entirely different page write implementation, but there are certainly some workloads this helps even when those things are tuned accordingly. Makes me wonder, its a bit of a crazy idea, but have you thought about the possibility of making this user settable per transaction... recreating magic similar to synchronous_commit? Robert Treat https://xzilla.net ^ permalink raw reply [nested|flat] 4+ messages in thread
end of thread, other threads:[~2026-03-04 04:42 UTC | newest] Thread overview: 4+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2021-03-12 20:43 [PATCH 07/10] add wal_compression_method: zstd Justin Pryzby <[email protected]> 2023-05-15 19:02 [PATCH v4 1/3] Rearrange CLUSTER rules in gram.y. Nathan Bossart <[email protected]> 2026-02-26 07:13 Re: [PROPOSAL] Doublewrite Buffer as an alternative torn page protection to Full Page Write 陈宗志 <[email protected]> 2026-03-04 04:42 ` Re: [PROPOSAL] Doublewrite Buffer as an alternative torn page protection to Full Page Write Robert Treat <[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