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

* [PATCH 08/10] Default to zstd..
@ 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)

for CI, not for merge
---
 configure                         | 6 ++++--
 configure.ac                      | 2 +-
 src/backend/access/transam/xlog.c | 2 +-
 src/backend/utils/misc/guc.c      | 2 +-
 4 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/configure b/configure
index 81e23418b2..253f028fc4 100755
--- a/configure
+++ b/configure
@@ -1582,7 +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
+  --without-zstd          build without 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
@@ -8740,7 +8740,9 @@ $as_echo "#define USE_ZSTD 1" >>confdefs.h
   esac
 
 else
-  with_zstd=no
+  with_zstd=yes
+
+$as_echo "#define USE_ZSTD 1" >>confdefs.h
 
 fi
 
diff --git a/configure.ac b/configure.ac
index d6f6349067..8d72710fa7 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1005,7 +1005,7 @@ fi
 # ZSTD
 #
 AC_MSG_CHECKING([whether to build with zstd support])
-PGAC_ARG_BOOL(with, zstd, no, [build with Zstd compression library],
+PGAC_ARG_BOOL(with, zstd, yes, [build without 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)
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 307eee6626..92023de9f5 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_LZ4;
+int			wal_compression_method = WAL_COMPRESSION_ZSTD;
 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 52f9cd0242..8031e027aa 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_LZ4, wal_compression_options,
+		WAL_COMPRESSION_ZSTD, wal_compression_options,
 		NULL, NULL, NULL
 	},
 
-- 
2.17.0


--jozmn01XJZjDjM3N
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="0009-Add-zstd-compression-levels.patch"



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

* Re: pipe_read_line for reading arbitrary strings
@ 2023-05-17 11:49  Daniel Gustafsson <[email protected]>
  0 siblings, 0 replies; 2+ messages in thread

From: Daniel Gustafsson @ 2023-05-17 11:49 UTC (permalink / raw)
  To: Postgres hackers <[email protected]>

> On 7 Mar 2023, at 23:05, Daniel Gustafsson <[email protected]> wrote:
> 
> When skimming through pg_rewind during a small review I noticed the use of
> pipe_read_line for reading arbitrary data from a pipe, the mechanics of which
> seemed odd.

A rebase of this for the CFBot since I realized I had forgotten to add this to
the July CF.

--
Daniel Gustafsson



Attachments:

  [application/octet-stream] v2-0001-Refactor-pipe_read_line-to-return-the-full-line.patch (6.1K, ../../[email protected]/2-v2-0001-Refactor-pipe_read_line-to-return-the-full-line.patch)
  download | inline diff:
From a66759e24b3cc597f964ec9f9189251dab5f3ca6 Mon Sep 17 00:00:00 2001
From: Daniel Gustafsson <[email protected]>
Date: Wed, 17 May 2023 12:55:55 +0200
Subject: [PATCH v2] Refactor pipe_read_line to return the full line

Commit 5b2f4afffe6 refactored find_other_exec() and in the process broke
out pipe_read_line() into a static routine for reading a single line of
output, aimed at reading a version number.  Commit a7e8ece41 later exposed
it externally in order to read a GUC from postgresql.conf using "postgres
-C ..".  f06b1c598 also make use of it for reading a version string much
like find_other_exec().  The internal variable remained "pgver" though.

Since the function requires passing a buffer/size, and at most size - 1
bytes will be read via fgets(), there is a truncation risk when using this
for reading GUCs (like how pg_rewind does, though the risk in this case is
marginal).

To keep this as generic functionality for reading a line from a pipe, this
refactors pipe_read_line into returning an allocated buffer containing all
of the line to remove the risk of silent truncation.

Discussion: https://postgr.es/m/[email protected]
---
 src/bin/pg_rewind/pg_rewind.c | 14 ++++++--------
 src/bin/pg_upgrade/exec.c     |  6 ++++--
 src/common/exec.c             | 36 +++++++++++++++++++----------------
 src/include/port.h            |  2 +-
 4 files changed, 31 insertions(+), 27 deletions(-)

diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c
index f7f3b8227f..c9fbe740e7 100644
--- a/src/bin/pg_rewind/pg_rewind.c
+++ b/src/bin/pg_rewind/pg_rewind.c
@@ -1042,8 +1042,7 @@ static void
 getRestoreCommand(const char *argv0)
 {
 	int			rc;
-	char		postgres_exec_path[MAXPGPATH],
-				cmd_output[MAXPGPATH];
+	char		postgres_exec_path[MAXPGPATH];
 	PQExpBuffer postgres_cmd;
 
 	if (!restore_wal)
@@ -1092,16 +1091,15 @@ getRestoreCommand(const char *argv0)
 	/* add -C switch, for restore_command */
 	appendPQExpBufferStr(postgres_cmd, " -C restore_command");
 
-	if (!pipe_read_line(postgres_cmd->data, cmd_output, sizeof(cmd_output)))
-		exit(1);
+	restore_command = pipe_read_line(postgres_cmd->data);
+	if (restore_command == NULL)
+		pg_fatal("unable to read restore_command from target cluster");
 
-	(void) pg_strip_crlf(cmd_output);
+	(void) pg_strip_crlf(restore_command);
 
-	if (strcmp(cmd_output, "") == 0)
+	if (strcmp(restore_command, "") == 0)
 		pg_fatal("restore_command is not set in the target cluster");
 
-	restore_command = pg_strdup(cmd_output);
-
 	pg_log_debug("using for rewind restore_command = \'%s\'",
 				 restore_command);
 
diff --git a/src/bin/pg_upgrade/exec.c b/src/bin/pg_upgrade/exec.c
index 5b2edebe41..f56f22af4e 100644
--- a/src/bin/pg_upgrade/exec.c
+++ b/src/bin/pg_upgrade/exec.c
@@ -431,7 +431,7 @@ static void
 check_exec(const char *dir, const char *program, bool check_version)
 {
 	char		path[MAXPGPATH];
-	char		line[MAXPGPATH];
+	char	   *line;
 	char		cmd[MAXPGPATH];
 	char		versionstr[128];
 
@@ -442,7 +442,7 @@ check_exec(const char *dir, const char *program, bool check_version)
 
 	snprintf(cmd, sizeof(cmd), "\"%s\" -V", path);
 
-	if (!pipe_read_line(cmd, line, sizeof(line)))
+	if ((line = pipe_read_line(cmd)) == NULL)
 		pg_fatal("check for \"%s\" failed: cannot execute",
 				 path);
 
@@ -456,4 +456,6 @@ check_exec(const char *dir, const char *program, bool check_version)
 			pg_fatal("check for \"%s\" failed: incorrect version: found \"%s\", expected \"%s\"",
 					 path, line, versionstr);
 	}
+
+	pg_free(line);
 }
diff --git a/src/common/exec.c b/src/common/exec.c
index f209b934df..70c9397643 100644
--- a/src/common/exec.c
+++ b/src/common/exec.c
@@ -42,6 +42,8 @@
 #endif
 #endif
 
+#include "common/string.h"
+
 /* Inhibit mingw CRT's auto-globbing of command line arguments */
 #if defined(WIN32) && !defined(_MSC_VER)
 extern int	_CRT_glob = 0;		/* 0 turns off globbing; 1 turns it on */
@@ -328,7 +330,7 @@ find_other_exec(const char *argv0, const char *target,
 				const char *versionstr, char *retpath)
 {
 	char		cmd[MAXPGPATH];
-	char		line[MAXPGPATH];
+	char	   *line;
 
 	if (find_my_exec(argv0, retpath) < 0)
 		return -1;
@@ -346,46 +348,48 @@ find_other_exec(const char *argv0, const char *target,
 
 	snprintf(cmd, sizeof(cmd), "\"%s\" -V", retpath);
 
-	if (!pipe_read_line(cmd, line, sizeof(line)))
+	if ((line = pipe_read_line(cmd)) == NULL)
 		return -1;
 
 	if (strcmp(line, versionstr) != 0)
+	{
+		pfree(line);
 		return -2;
+	}
 
+	pfree(line);
 	return 0;
 }
 
 
 /*
- * Execute a command in a pipe and read the first line from it.
+ * Execute a command in a pipe and read the first line from it. The returned
+ * string is allocated, the caller is responsible for freeing.
  */
 char *
-pipe_read_line(char *cmd, char *line, int maxsize)
+pipe_read_line(char *cmd)
 {
-	FILE	   *pgver;
+	FILE	   *pipe_cmd;
+	char	   *line;
 
 	fflush(NULL);
 
 	errno = 0;
-	if ((pgver = popen(cmd, "r")) == NULL)
+	if ((pipe_cmd = popen(cmd, "r")) == NULL)
 	{
 		perror("popen failure");
 		return NULL;
 	}
 
-	errno = 0;
-	if (fgets(line, maxsize, pgver) == NULL)
+	line = pg_get_line(pipe_cmd, NULL);
+
+	if (line == NULL)
 	{
-		if (feof(pgver))
-			fprintf(stderr, "no data was returned by command \"%s\"\n", cmd);
-		else
-			perror("fgets failure");
-		pclose(pgver);			/* no error checking */
-		return NULL;
+		log_error(errcode_for_file_access(),
+				  _("no data was returned by command \"%s\""), cmd);
 	}
 
-	if (pclose_check(pgver))
-		return NULL;
+	(void) pclose_check(pipe_cmd);
 
 	return line;
 }
diff --git a/src/include/port.h b/src/include/port.h
index a88d403483..76ef003a69 100644
--- a/src/include/port.h
+++ b/src/include/port.h
@@ -137,7 +137,7 @@ extern int	validate_exec(const char *path);
 extern int	find_my_exec(const char *argv0, char *retpath);
 extern int	find_other_exec(const char *argv0, const char *target,
 							const char *versionstr, char *retpath);
-extern char *pipe_read_line(char *cmd, char *line, int maxsize);
+extern char *pipe_read_line(char *cmd);
 
 /* Doesn't belong here, but this is used with find_other_exec(), so... */
 #define PG_BACKEND_VERSIONSTR "postgres (PostgreSQL) " PG_VERSION "\n"
-- 
2.32.1 (Apple Git-133)



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


end of thread, other threads:[~2023-05-17 11:49 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 08/10] Default to zstd.. Justin Pryzby <[email protected]>
2023-05-17 11:49 Re: pipe_read_line for reading arbitrary strings Daniel Gustafsson <[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