public inbox for [email protected]  
help / color / mirror / Atom feed
[PATCH 06/10] Default to LZ4..
2+ messages / 2 participants
[nested] [flat]

* [PATCH 06/10] Default to LZ4..
@ 2021-03-12 21:35 Justin Pryzby <[email protected]>
  0 siblings, 0 replies; 2+ messages in thread

From: Justin Pryzby @ 2021-03-12 21:35 UTC (permalink / raw)

this is meant to exercise in the CIs, and not meant to be merged
---
 configure                         | 6 ++++--
 configure.ac                      | 4 ++--
 src/backend/access/transam/xlog.c | 2 +-
 src/backend/utils/misc/guc.c      | 2 +-
 4 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/configure b/configure
index fed440adcf..8d76be00c1 100755
--- a/configure
+++ b/configure
@@ -1575,7 +1575,7 @@ Optional Packages:
   --with-system-tzdata=DIR
                           use system time zone data in DIR
   --without-zlib          do not use Zlib
-  --with-lz4              build with LZ4 support
+  --without-lz4           build without LZ4 support
   --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
@@ -8598,7 +8598,9 @@ $as_echo "#define USE_LZ4 1" >>confdefs.h
   esac
 
 else
-  with_lz4=no
+  with_lz4=yes
+
+$as_echo "#define USE_LZ4 1" >>confdefs.h
 
 fi
 
diff --git a/configure.ac b/configure.ac
index 8c454128bb..bfcdc88be0 100644
--- a/configure.ac
+++ b/configure.ac
@@ -990,8 +990,8 @@ AC_SUBST(with_zlib)
 # LZ4
 #
 AC_MSG_CHECKING([whether to build with LZ4 support])
-PGAC_ARG_BOOL(with, lz4, no, [build with LZ4 support],
-              [AC_DEFINE([USE_LZ4], 1, [Define to 1 to build with LZ4 support. (--with-lz4)])])
+PGAC_ARG_BOOL(with, lz4, yes, [build without LZ4 support],
+              [AC_DEFINE([USE_LZ4], 1, [Define to 1 to build without LZ4 support. (--without-lz4)])])
 AC_MSG_RESULT([$with_lz4])
 AC_SUBST(with_lz4)
 
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 984ce39cc7..3657f74de9 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -99,7 +99,7 @@ bool		EnableHotStandby = false;
 bool		fullPageWrites = true;
 bool		wal_log_hints = false;
 bool		wal_compression = false;
-int			wal_compression_method = WAL_COMPRESSION_ZLIB;
+int			wal_compression_method = WAL_COMPRESSION_LZ4;
 char	   *wal_consistency_checking_string = NULL;
 bool	   *wal_consistency_checking = NULL;
 bool		wal_init_zero = true;
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index c37a8313d3..52f9cd0242 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -4728,7 +4728,7 @@ static struct config_enum ConfigureNamesEnum[] =
 			NULL
 		},
 		&wal_compression_method,
-		WAL_COMPRESSION_ZLIB, wal_compression_options,
+		WAL_COMPRESSION_LZ4, wal_compression_options,
 		NULL, NULL, NULL
 	},
 
-- 
2.17.0


--XsQoSWH+UP9D9v3l
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="0007-add-wal_compression_method-zstd.patch"



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

* Suppress generating WAL records during the upgrade
@ 2023-08-08 10:43 Hayato Kuroda (Fujitsu) <[email protected]>
  0 siblings, 0 replies; 2+ messages in thread

From: Hayato Kuroda (Fujitsu) @ 2023-08-08 10:43 UTC (permalink / raw)
  To: PostgreSQL Hackers <[email protected]>; +Cc: Julien Rouhaud <[email protected]>; Masahiko Sawada <[email protected]>; Amit Kapila <[email protected]>

Dear hackers,
(CC: Julien, Sawada-san, Amit)

This is a fork thread from "[PoC] pg_upgrade: allow to upgrade publisher node" [1].

# Background

[1] is the patch which allows to replicate logical replication slots from old to new node.
Followings describe the rough steps:

1. Boot old node as binary-upgrade mode
2. Check confirmed_lsn of all the slots, and confirm all WALs are replicated to downstream
3. Dump slot info to sql file
4. Stop old node
5. Boot new node as binary-upgrade mode
...

Here, step 2 was introduced for avoiding data loss. If there are some WAL records
ahead confirmed_lsn, such records would not be replicated anymore - it may be dangerous.

So in the current patch, pg_upgrade fails if other records than SHUTDOWN_CHECKPOINT
exits after any confirmed_flush_lsn.

# Problem

We found that following three records might be generated during the upgrade.

* RUNNING_XACT
* CHECKPOINT_ONLINE
* XLOG_FPI_FOR_HINT

RUNNING_XACT might be written by the background writer. Conditions for the generation are:

a. Elapsed 15 seconds since the last WAL creation or bootstraping of the process, and either of them:
b-1. The process had never create the RUNNING_XACT record, or
b-2. Some "important WALs" were created after the last RUNNING_XACT record


CHECKPOINT_ONLINE might be written by the checkpointer. Conditions for the generation are:

a. Elapsed checkpoint_timeout seconds since the last creation or bootstraping, and either of them:
b-1. The process had never create the CHECKPOINT_ONLINE record, or
b-2. Some "important WALs" were created after the last CHECKPOINT record


XLOG_FPI_FOR_HINT, which is raised by Sawada-san, might be generated by backend processes.
Conditions for the generation are:

a. Backend processes scanned any tuples (even if it was the system catalog), or either of them:
b-1. Data checksum was enabled, or
b-2. wal_log_hints was set to on

# Solution

I wanted to suppress generations of WALs during the upgrade, because of the "# Background".

Regarding the RUNNING_XACT and CHECKPOINT_ONLINE, it might be OK by removing the
condition b-1. The duration between bootstrap and initial {RUNNING_XACT|CHECKPOINT_ONLINE}
becomes longer, but I could not find impacts by it.

As for the XLOG_FPI_FOR_HINT, the simplest way I came up with is not to call
XLogSaveBufferForHint() during binary upgrade. Considerations may be not enough,
but I attached the patch for the fix. It passed CI on my repository.


Do you have any other considerations about it?
An approach, which adds "if (IsBinaryUpgare)" in XLogInsertAllowed(), was proposed in [2]. 
But I'm not sure it could really solve the issue - e.g., XLogInsertRecord() just
raised an ERROR if !XLogInsertAllowed().

[1]: https://commitfest.postgresql.org/44/4273/
[2]: https://www.postgresql.org/message-id/flat/20210121152357.s6eflhqyh4g5e6dv%40dalibo.com

Best Regards,
Hayato Kuroda
FUJITSU LIMITED



Attachments:

  [application/octet-stream] 0001-Suppress-generating-WAL-records-during-the-upgrade.patch (2.6K, ../../TYAPR01MB58660273EACEFC5BF256B133F50DA@TYAPR01MB5866.jpnprd01.prod.outlook.com/2-0001-Suppress-generating-WAL-records-during-the-upgrade.patch)
  download | inline diff:
From ffa72cd4067b5d31ce6dc6bc2818607425181913 Mon Sep 17 00:00:00 2001
From: Hayato Kuroda <[email protected]>
Date: Tue, 8 Aug 2023 08:08:31 +0000
Subject: [PATCH] Suppress generating WAL records during the upgrade

---
 src/backend/access/transam/xlog.c   | 3 ++-
 src/backend/postmaster/bgwriter.c   | 5 +++--
 src/backend/storage/buffer/bufmgr.c | 5 +++++
 3 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 60c0b7ec3a..2f1cbc6898 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -6555,7 +6555,8 @@ CreateCheckPoint(int flags)
 	if ((flags & (CHECKPOINT_IS_SHUTDOWN | CHECKPOINT_END_OF_RECOVERY |
 				  CHECKPOINT_FORCE)) == 0)
 	{
-		if (last_important_lsn == ControlFile->checkPoint)
+		if (XLogRecPtrIsInvalid(last_important_lsn) ||
+			last_important_lsn == ControlFile->checkPoint)
 		{
 			WALInsertLockRelease();
 			END_CRIT_SECTION();
diff --git a/src/backend/postmaster/bgwriter.c b/src/backend/postmaster/bgwriter.c
index caad642ec9..95d88ac8bc 100644
--- a/src/backend/postmaster/bgwriter.c
+++ b/src/backend/postmaster/bgwriter.c
@@ -276,6 +276,7 @@ BackgroundWriterMain(void)
 		{
 			TimestampTz timeout = 0;
 			TimestampTz now = GetCurrentTimestamp();
+			XLogRecPtr last_important = GetLastImportantRecPtr();
 
 			timeout = TimestampTzPlusMilliseconds(last_snapshot_ts,
 												  LOG_SNAPSHOT_INTERVAL_MS);
@@ -287,8 +288,8 @@ BackgroundWriterMain(void)
 			 * start of a record, whereas last_snapshot_lsn points just past
 			 * the end of the record.
 			 */
-			if (now >= timeout &&
-				last_snapshot_lsn <= GetLastImportantRecPtr())
+			if (now >= timeout && last_snapshot_lsn <= last_important &&
+				!XLogRecPtrIsInvalid(last_important))
 			{
 				last_snapshot_lsn = LogStandbySnapshot();
 				last_snapshot_ts = now;
diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c
index df22aaa1c5..db0fc4ef96 100644
--- a/src/backend/storage/buffer/bufmgr.c
+++ b/src/backend/storage/buffer/bufmgr.c
@@ -4588,8 +4588,13 @@ MarkBufferDirtyHint(Buffer buffer, bool buffer_std)
 		 *
 		 * We don't check full_page_writes here because that logic is included
 		 * when we call XLogInsert() since the value changes dynamically.
+		 *
+		 * XXX: avoid to write WAL if we are in the binary-upgrade mode. Some
+		 * catalogs are read during the upgrade, but it may trigger to generate
+		 * XLOG_FPI_FOR_HINT. It may become the "WAL hole".
 		 */
 		if (XLogHintBitIsNeeded() &&
+			!IsBinaryUpgrade &&
 			(pg_atomic_read_u32(&bufHdr->state) & BM_PERMANENT))
 		{
 			/*
-- 
2.27.0



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


end of thread, other threads:[~2023-08-08 10:43 UTC | newest]

Thread overview: 2+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2021-03-12 21:35 [PATCH 06/10] Default to LZ4.. Justin Pryzby <[email protected]>
2023-08-08 10:43 Suppress generating WAL records during the upgrade Hayato Kuroda (Fujitsu) <[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