public inbox for [email protected]
help / color / mirror / Atom feed[PATCH 08/10] Default to zstd..
6+ messages / 6 participants
[nested] [flat]
* [PATCH 08/10] Default to zstd..
@ 2021-03-12 21:35 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 6+ 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] 6+ messages in thread
* A modest proposal: make parser/rewriter/planner inputs read-only
@ 2025-04-05 16:46 Tom Lane <[email protected]>
2025-04-06 02:30 ` Re: A modest proposal: make parser/rewriter/planner inputs read-only Andres Freund <[email protected]>
2025-04-15 09:20 ` Re: A modest proposal: make parser/rewriter/planner inputs read-only Richard Guo <[email protected]>
2025-04-15 10:00 ` Re: A modest proposal: make parser/rewriter/planner inputs read-only Andrei Lepikhov <[email protected]>
0 siblings, 3 replies; 6+ messages in thread
From: Tom Lane @ 2025-04-05 16:46 UTC (permalink / raw)
To: [email protected]
The parser, rewriter, and planner all have a bad habit of scribbling
on their input parsetrees. At this point I've lost count of how many
bugs that's caused (but e33f2335a and 0f43083d1 are the two latest
examples), and of how many places there are that are brute-forcing a
solution to the problem by copying a whole parsetree before letting
one of these subsystems see it.
Years ago we fixed the executor to treat its input Plan trees as
read-only. It seems like we really ought to do the same for these
upstream subsystems. Surely, whatever benefit we get from changing
Node contents in-place is lost due to all these other hacks.
While I've had this thought in the back of my head for a long while,
I haven't pursued it because I couldn't think of a practical way
to find all the places that are violating such a policy, much less
prevent future violations. However, today I thought of this sketch
of a solution:
1. Invent a way to make a particular memory context read-only
after putting some data into it.
2. In debug builds, after we've built a tree that should be considered
read-only, copy it into such a context and make it read-only. Or
perhaps build it there in the first place.
3. Fix the resulting crashes.
4. Profit! (In particular, nuke a lot of no-longer-needed copyObject
calls.)
My first thought about implementing #1 was to seek Valgrind's help,
but so far as I can find out there's no VALGRIND_MAKE_MEM_READ_ONLY.
Step #3 would be pretty tedious anyway if it required running under
Valgrind. However, all modern hardware has the ability to mark
memory read-only at the page level, and most platforms expose that
in some way or other. So it doesn't seem unreasonable to invent
a memory context option (or whole new context type, if that seems
easier) that is careful to align its allocation blocks on page
boundaries and then can set or clear the hardware R/O flag on
demand. It'd be enough if the R/O enforcement worked on popular
development platforms, we don't have to make it work absolutely
everywhere.
I'm not planning to pursue this idea Right Now, but it seems like
something that could happen for v19 or so. In the meantime I wanted
to get the ideas down on electrons.
Thoughts?
regards, tom lane
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: A modest proposal: make parser/rewriter/planner inputs read-only
2025-04-05 16:46 A modest proposal: make parser/rewriter/planner inputs read-only Tom Lane <[email protected]>
@ 2025-04-06 02:30 ` Andres Freund <[email protected]>
2025-04-16 00:06 ` Re: A modest proposal: make parser/rewriter/planner inputs read-only Ashwin Agrawal <[email protected]>
2 siblings, 1 reply; 6+ messages in thread
From: Andres Freund @ 2025-04-06 02:30 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: [email protected]
Hi,
On 2025-04-05 12:46:37 -0400, Tom Lane wrote:
> 1. Invent a way to make a particular memory context read-only
> after putting some data into it.
>
> 2. In debug builds, after we've built a tree that should be considered
> read-only, copy it into such a context and make it read-only. Or
> perhaps build it there in the first place.
> 3. Fix the resulting crashes.
>
> 4. Profit! (In particular, nuke a lot of no-longer-needed copyObject
> calls.)
>
> My first thought about implementing #1 was to seek Valgrind's help,
> but so far as I can find out there's no VALGRIND_MAKE_MEM_READ_ONLY.
> Step #3 would be pretty tedious anyway if it required running under
> Valgrind. However, all modern hardware has the ability to mark
> memory read-only at the page level, and most platforms expose that
> in some way or other. So it doesn't seem unreasonable to invent
> a memory context option (or whole new context type, if that seems
> easier) that is careful to align its allocation blocks on page
> boundaries and then can set or clear the hardware R/O flag on
> demand. It'd be enough if the R/O enforcement worked on popular
> development platforms, we don't have to make it work absolutely
> everywhere.
FWIW, while hacking on patch to making hint bit writes not happening while IO
is going on (so we don't need to copy the page anymore and don't cause
filesystem level issues with DIO), I hacked up protection for shared buffers
using mprotect() - it worked way better than I thought it would. The overhead
ended up surprisingly low:
base:
real 1m4.613s
user 4m31.409s
sys 3m20.445s
ENFORCE_BUFFER_PROT
real 1m11.912s
user 4m27.332s
sys 3m28.063s
See https://postgr.es/m/043c8b50-d183-46e5-b054-145cc0f6f908%40iki.fi
I'm mostly sharing that to say that
a) yes, mprotect() is viable and works surprisingly well
b) it might be worth inventing some common platform abstraction for mprotect
That prototype patch already worked on most platforms, windows should be
entirely doable.
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: A modest proposal: make parser/rewriter/planner inputs read-only
2025-04-05 16:46 A modest proposal: make parser/rewriter/planner inputs read-only Tom Lane <[email protected]>
2025-04-06 02:30 ` Re: A modest proposal: make parser/rewriter/planner inputs read-only Andres Freund <[email protected]>
@ 2025-04-16 00:06 ` Ashwin Agrawal <[email protected]>
0 siblings, 0 replies; 6+ messages in thread
From: Ashwin Agrawal @ 2025-04-16 00:06 UTC (permalink / raw)
To: Andres Freund <[email protected]>; +Cc: Tom Lane <[email protected]>; [email protected]
On Sat, Apr 5, 2025 at 7:31 PM Andres Freund <[email protected]> wrote:
> Hi,
>
> On 2025-04-05 12:46:37 -0400, Tom Lane wrote:
> > 1. Invent a way to make a particular memory context read-only
> > after putting some data into it.
> >
> > 2. In debug builds, after we've built a tree that should be considered
> > read-only, copy it into such a context and make it read-only. Or
> > perhaps build it there in the first place.
>
> > 3. Fix the resulting crashes.
> >
> > 4. Profit! (In particular, nuke a lot of no-longer-needed copyObject
> > calls.)
> >
> > My first thought about implementing #1 was to seek Valgrind's help,
> > but so far as I can find out there's no VALGRIND_MAKE_MEM_READ_ONLY.
> > Step #3 would be pretty tedious anyway if it required running under
> > Valgrind. However, all modern hardware has the ability to mark
> > memory read-only at the page level, and most platforms expose that
> > in some way or other. So it doesn't seem unreasonable to invent
> > a memory context option (or whole new context type, if that seems
> > easier) that is careful to align its allocation blocks on page
> > boundaries and then can set or clear the hardware R/O flag on
> > demand. It'd be enough if the R/O enforcement worked on popular
> > development platforms, we don't have to make it work absolutely
> > everywhere.
>
> FWIW, while hacking on patch to making hint bit writes not happening while
> IO
> is going on (so we don't need to copy the page anymore and don't cause
> filesystem level issues with DIO), I hacked up protection for shared
> buffers
> using mprotect() - it worked way better than I thought it would. The
> overhead
> ended up surprisingly low:
>
> base:
> real 1m4.613s
> user 4m31.409s
> sys 3m20.445s
>
> ENFORCE_BUFFER_PROT
>
> real 1m11.912s
> user 4m27.332s
> sys 3m28.063s
>
>
> See https://postgr.es/m/043c8b50-d183-46e5-b054-145cc0f6f908%40iki.fi
>
>
> I'm mostly sharing that to say that
> a) yes, mprotect() is viable and works surprisingly well
> b) it might be worth inventing some common platform abstraction for
> mprotect
>
> That prototype patch already worked on most platforms, windows should be
> entirely doable.
Also, I would like to provide reference to this old thread [1] in favor of
mprotect().
In that thread, and in Greenplum using mprotect helps detect Shared buffer
access rule violations.
[1]
https://www.postgresql.org/message-id/CANXE4Tfxmjv8Z-kcwWUCYxx6zCFS%3DGeBPm-ZjjLucvrGewjUtg%40mail.g...
--
This electronic communication and the information and any files transmitted
with it, or attached to it, are confidential and are intended solely for
the use of the individual or entity to whom it is addressed and may contain
information that is confidential, legally privileged, protected by privacy
laws, or otherwise restricted from disclosure to anyone else. If you are
not the intended recipient or the person responsible for delivering the
e-mail to the intended recipient, you are hereby notified that any use,
copying, distributing, dissemination, forwarding, printing, or copying of
this e-mail is strictly prohibited. If you received this e-mail in error,
please return the e-mail to the sender, delete it from your computer, and
destroy any printed copy of it.
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: A modest proposal: make parser/rewriter/planner inputs read-only
2025-04-05 16:46 A modest proposal: make parser/rewriter/planner inputs read-only Tom Lane <[email protected]>
@ 2025-04-15 09:20 ` Richard Guo <[email protected]>
2 siblings, 0 replies; 6+ messages in thread
From: Richard Guo @ 2025-04-15 09:20 UTC (permalink / raw)
To: Tom Lane <[email protected]>; +Cc: [email protected]
On Sun, Apr 6, 2025 at 1:46 AM Tom Lane <[email protected]> wrote:
> The parser, rewriter, and planner all have a bad habit of scribbling
> on their input parsetrees. At this point I've lost count of how many
> bugs that's caused (but e33f2335a and 0f43083d1 are the two latest
> examples), and of how many places there are that are brute-forcing a
> solution to the problem by copying a whole parsetree before letting
> one of these subsystems see it.
>
> Years ago we fixed the executor to treat its input Plan trees as
> read-only. It seems like we really ought to do the same for these
> upstream subsystems. Surely, whatever benefit we get from changing
> Node contents in-place is lost due to all these other hacks.
While I'm in favor of this idea, it seems that it will require lots of
code changes. In particular, within the planner, the parsetree goes
through considerable transformations during the preprocessing phase,
such as subquery pull-up, constant folding, and so on. Would this
proposal mean we'd need to refactor all those places to treat the
input parsetrees as read-only? Just trying to understand the scope of
what would be involved.
Thanks
Richard
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: A modest proposal: make parser/rewriter/planner inputs read-only
2025-04-05 16:46 A modest proposal: make parser/rewriter/planner inputs read-only Tom Lane <[email protected]>
@ 2025-04-15 10:00 ` Andrei Lepikhov <[email protected]>
2 siblings, 0 replies; 6+ messages in thread
From: Andrei Lepikhov @ 2025-04-15 10:00 UTC (permalink / raw)
To: Tom Lane <[email protected]>; [email protected]
On 4/5/25 18:46, Tom Lane wrote:
> I'm not planning to pursue this idea Right Now, but it seems like
> something that could happen for v19 or so. In the meantime I wanted
> to get the ideas down on electrons.
>
> Thoughts?
I generally like the idea because, for now, I need to be sure that no
one touched the parse tree before copying it to do additional
transformations before the optimisation phase.
But what is the way you are proposing here? Do you mean that one more
entity will be explicitly introduced: a transformed parse tree?
It would open an opportunity for extensions to build a set of
alternative transformed trees, pass them through the optimisation phase
and choose the best plan.
--
regards, Andrei Lepikhov
^ permalink raw reply [nested|flat] 6+ messages in thread
end of thread, other threads:[~2025-04-16 00:06 UTC | newest]
Thread overview: 6+ 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]>
2025-04-05 16:46 A modest proposal: make parser/rewriter/planner inputs read-only Tom Lane <[email protected]>
2025-04-06 02:30 ` Re: A modest proposal: make parser/rewriter/planner inputs read-only Andres Freund <[email protected]>
2025-04-16 00:06 ` Re: A modest proposal: make parser/rewriter/planner inputs read-only Ashwin Agrawal <[email protected]>
2025-04-15 09:20 ` Re: A modest proposal: make parser/rewriter/planner inputs read-only Richard Guo <[email protected]>
2025-04-15 10:00 ` Re: A modest proposal: make parser/rewriter/planner inputs read-only Andrei Lepikhov <[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