public inbox for [email protected]
help / color / mirror / Atom feed[PATCH v6 08/10] doc: backup manifests
43+ messages / 8 participants
[nested] [flat]
* [PATCH v6 08/10] doc: backup manifests
@ 2020-04-03 21:17 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 43+ messages in thread
From: Justin Pryzby @ 2020-04-03 21:17 UTC (permalink / raw)
commit 0d8c9c1210c44b36ec2efcb223a1dfbe897a3661
Author: Robert Haas <[email protected]>
Previously reported here:
https://www.postgresql.org/message-id/20200403212445.GB12283%40telsasoft.com
---
doc/src/sgml/protocol.sgml | 4 ++--
doc/src/sgml/ref/pg_basebackup.sgml | 4 ++--
doc/src/sgml/ref/pg_verifybackup.sgml | 6 +++---
3 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/doc/src/sgml/protocol.sgml b/doc/src/sgml/protocol.sgml
index 8b00235a51..c402b97903 100644
--- a/doc/src/sgml/protocol.sgml
+++ b/doc/src/sgml/protocol.sgml
@@ -2586,7 +2586,7 @@ The commands accepted in replication mode are:
and sent along with the backup. The manifest is a list of every
file present in the backup with the exception of any WAL files that
may be included. It also stores the size, last modification time, and
- an optional checksum for each file.
+ optionally a checksum for each file.
A value of <literal>force-encode</literal> forces all filenames
to be hex-encoded; otherwise, this type of encoding is performed only
for files whose names are non-UTF8 octet sequences.
@@ -2602,7 +2602,7 @@ The commands accepted in replication mode are:
<term><literal>MANIFEST_CHECKSUMS</literal> <replaceable>checksum_algorithm</replaceable></term>
<listitem>
<para>
- Specifies the algorithm that should be applied to each file included
+ Specifies the checksum algorithm that should be applied to each file included
in the backup manifest. Currently, the available
algorithms are <literal>NONE</literal>, <literal>CRC32C</literal>,
<literal>SHA224</literal>, <literal>SHA256</literal>,
diff --git a/doc/src/sgml/ref/pg_basebackup.sgml b/doc/src/sgml/ref/pg_basebackup.sgml
index aa0b27c9f3..024adcb0b5 100644
--- a/doc/src/sgml/ref/pg_basebackup.sgml
+++ b/doc/src/sgml/ref/pg_basebackup.sgml
@@ -530,7 +530,7 @@ PostgreSQL documentation
not contain any checksums. Otherwise, it will contain a checksum
of each file in the backup using the specified algorithm. In addition,
the manifest will always contain a <literal>SHA256</literal>
- checksum of its own contents. The <literal>SHA</literal> algorithms
+ checksum of its own content. The <literal>SHA</literal> algorithms
are significantly more CPU-intensive than <literal>CRC32C</literal>,
so selecting one of them may increase the time required to complete
the backup.
@@ -540,7 +540,7 @@ PostgreSQL documentation
of each file for users who wish to verify that the backup has not been
tampered with, while the CRC32C algorithm provides a checksum that is
much faster to calculate; it is good at catching errors due to accidental
- changes but is not resistant to targeted modifications. Note that, to
+ changes but is not resistant to malicious modifications. Note that, to
be useful against an adversary who has access to the backup, the backup
manifest would need to be stored securely elsewhere or otherwise
verified not to have been modified since the backup was taken.
diff --git a/doc/src/sgml/ref/pg_verifybackup.sgml b/doc/src/sgml/ref/pg_verifybackup.sgml
index c160992e6d..b12e134a44 100644
--- a/doc/src/sgml/ref/pg_verifybackup.sgml
+++ b/doc/src/sgml/ref/pg_verifybackup.sgml
@@ -44,7 +44,7 @@ PostgreSQL documentation
every check which will be performed by a running server when attempting
to make use of the backup. Even if you use this tool, you should still
perform test restores and verify that the resulting databases work as
- expected and that they appear to contain the correct data. However,
+ expected and that they contain the correct data. However,
<application>pg_verifybackup</application> can detect many problems
that commonly occur due to storage problems or user error.
</para>
@@ -82,7 +82,7 @@ PostgreSQL documentation
for any files for which the computed checksum does not match the
checksum stored in the manifest. This step is not performed for any files
which produced errors in the previous step, since they are already known
- to have problems. Also, files which were ignored in the previous step are
+ to have problems. Files which were ignored in the previous step are
also ignored in this step.
</para>
@@ -121,7 +121,7 @@ PostgreSQL documentation
<title>Options</title>
<para>
- The following command-line options control the behavior.
+ The following command-line options control the behavior of this program.
<variablelist>
<varlistentry>
--
2.17.0
--C+ts3FVlLX8+P6JN
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v6-0009-Say-it-more-naturally.patch"
^ permalink raw reply [nested|flat] 43+ messages in thread
* CI and test improvements
@ 2022-05-28 15:37 Justin Pryzby <[email protected]>
0 siblings, 3 replies; 43+ messages in thread
From: Justin Pryzby @ 2022-05-28 15:37 UTC (permalink / raw)
To: Andres Freund <[email protected]>; Andrew Dunstan <[email protected]>; Thomas Munro <[email protected]>; +Cc: Noah Misch <[email protected]>; Michael Paquier <[email protected]>; Jacob Champion <[email protected]>; Anastasia Lubennikova <[email protected]>; Tom Lane <[email protected]>; Robert Haas <[email protected]>; Melanie Plageman <[email protected]>; Peter Eisentraut <[email protected]>; Daniel Gustafsson <[email protected]>; [email protected]
I'm "joining" a bunch of unresolved threads hoping to present them better since
they're related and I'm maintaining them together anyway.
https://www.postgresql.org/message-id/flat/20220219234148.GC9008%40telsasoft.com
- set TESTDIR from perl rather than Makefile
https://www.postgresql.org/message-id/flat/20220522232606.GZ19626%40telsasoft.com
- ccache, MSVC, and meson
https://www.postgresql.org/message-id/20220416144454.GX26620%40telsasoft.com
- Re: convert libpq uri-regress tests to tap test
https://www.postgresql.org/message-id/CA%2BhUKGLneD%2Bq%2BE7upHGwn41KGvbxhsKbJ%2BM-y9nvv7_Xjv8Qog%40...
- Re: A test for replay of regression tests
https://www.postgresql.org/message-id/flat/[email protected]
- cfbot requests
See the commit messages for more thread references.
I'm anticipating the need to further re-arrange the patch set - it's not clear
which patches should go first. Maybe some patches should be dropped in favour
of the meson project. I guess some patches will have to be re-implemented with
meson (msvc warnings).
I think there was some confusion about the vcregress "alltaptests" target.
I said that it's okay to add it and make cirrus use it (and that the buildfarm
could use it too). Andrew responded that the buildfarm wants to run different
tests separately. But Andres seems to have interpretted that as an objection
to the addition of an "alltaptests" target, which I think isn't what's
intended - it's fine if the buildfarm prefers not to use it.
Maybe the improvements to vcregress should go into v15 ? CI should run all the
tests (which also serves to document *how* to run all the tests, even if there
isn't a literal check-world target).
^ permalink raw reply [nested|flat] 43+ messages in thread
* Re: CI and test improvements
@ 2022-06-23 19:31 Justin Pryzby <[email protected]>
parent: Justin Pryzby <[email protected]>
2 siblings, 1 reply; 43+ messages in thread
From: Justin Pryzby @ 2022-06-23 19:31 UTC (permalink / raw)
To: [email protected]
On Sat, May 28, 2022 at 10:37:41AM -0500, Justin Pryzby wrote:
> I'm "joining" a bunch of unresolved threads hoping to present them better since
> they're related and I'm maintaining them together anyway.
This resolves an error with libpq tests in an intermediate commit, probably
caused by rebasing (and maybe hidden by the fact that the tests weren't being
run).
And updates ccache to avoid CCACHE_COMPILER.
Should any of the test changes go into v15 ?
> Subject: [PATCH 02/19] cirrus/vcregress: test modules/contrib with NO_INSTALLCHECK=1
> Subject: [PATCH 08/19] vcregress: add alltaptests
> Subject: [PATCH 14/19] Move libpq_pipeline test into src/interfaces/libpq.
> Subject: [PATCH 15/19] msvc: do not install libpq test tools by default
I also added: cirrus/ccache: disable compression and show stats
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).
https://cirrus-ci.com/build/5194596123672576
debian/bullseye has ccache 4.2; cirrus says 92MB cache after a single compilation; cache_size_kibibyte 102000
macos: has 4.5.1: 46MB cache; cache_size_kibibyte 51252
freebsd: has 3.7.12: 41MB cache; cache_size_kibibyte 130112
For some reason, mac's ccache uses 2x less space than either freesbsd or linux.
Linux is ~30% larger.
Freebsd uploads an artifact 3x smaller than the size ccache reports, because
its ccache is old so doesn't enable compression by default.
I've also sent some patches to Thomas for cfbot to help progress some of these
patches (code coverage and documentation upload as artifacts).
https://github.com/justinpryzby/cfbot/commits/master
--
Justin
^ permalink raw reply [nested|flat] 43+ messages in thread
* Re: CI and test improvements
@ 2022-06-23 20:38 Thomas Munro <[email protected]>
parent: Justin Pryzby <[email protected]>
0 siblings, 0 replies; 43+ messages in thread
From: Thomas Munro @ 2022-06-23 20:38 UTC (permalink / raw)
To: Justin Pryzby <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>
[Resending -- sorry if you receive this twice. Jacob's mail server
has apparently offended the list management software so emails bounce
if he's in CC.]
On Fri, Jun 24, 2022 at 7:23 AM Justin Pryzby <[email protected]> wrote:
> Freebsd uploads an artifact 3x smaller than the size ccache reports, because
> its ccache is old so doesn't enable compression by default.
That port/package got stuck on 3.x because of some dependency problems
when using in bootstrapping FreeBSD itself (or other ports?),
apparently. I didn't follow the details but the recent messages here
sound hopeful and I'm keeping my eye on it to see if 4.x lands as a
separate package we'd need to install, or something. Fingers crossed!
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=234971
> I've also sent some patches to Thomas for cfbot to help progress some of these
> patches (code coverage and documentation upload as artifacts).
> https://github.com/justinpryzby/cfbot/commits/master
Thanks, sorry for lack of action, will get to these soon.
^ permalink raw reply [nested|flat] 43+ messages in thread
* Re: CI and test improvements
@ 2022-07-08 00:22 Justin Pryzby <[email protected]>
parent: Justin Pryzby <[email protected]>
2 siblings, 0 replies; 43+ messages in thread
From: Justin Pryzby @ 2022-07-08 00:22 UTC (permalink / raw)
To: Andres Freund <[email protected]>; Thomas Munro <[email protected]>; +Cc: Noah Misch <[email protected]>; Michael Paquier <[email protected]>; Andrew Dunstan <[email protected]>; Anastasia Lubennikova <[email protected]>; Tom Lane <[email protected]>; Robert Haas <[email protected]>; Melanie Plageman <[email protected]>; Peter Eisentraut <[email protected]>; Daniel Gustafsson <[email protected]>; [email protected]
rebased on b6a5158f9 and 054325c5e
Also, cirrus/freebsd task can run 3x faster with more CPUs.
Subject: [PATCH 21/21] cirrus: run freebsd with more CPUs+RAM
https://cirrus-ci.com/task/4664440120410112
https://cirrus-ci.com/task/5100411110555648
In the past, I gather there was some undiagnosed issue when using more CPUs
(cirrus now enforces >5GB RAM when using 6 CPUs - maybe you tried to use too
little RAM, or maybe hit bad performance involving NUMA?)
https://www.postgresql.org/message-id/flat/[email protected]#f36c0b1...
^ permalink raw reply [nested|flat] 43+ messages in thread
* Re: CI and test improvements
@ 2022-08-28 14:44 Justin Pryzby <[email protected]>
parent: Justin Pryzby <[email protected]>
2 siblings, 1 reply; 43+ messages in thread
From: Justin Pryzby @ 2022-08-28 14:44 UTC (permalink / raw)
To: Andres Freund <[email protected]>; Andrew Dunstan <[email protected]>; Thomas Munro <[email protected]>; [email protected]; +Cc: Noah Misch <[email protected]>; Michael Paquier <[email protected]>; Anastasia Lubennikova <[email protected]>; Tom Lane <[email protected]>; Robert Haas <[email protected]>; Melanie Plageman <[email protected]>; Peter Eisentraut <[email protected]>; Daniel Gustafsson <[email protected]>
Resending with a problematic address removed...
On Sat, May 28, 2022 at 10:37:41AM -0500, Justin Pryzby wrote:
> I'm anticipating the need to further re-arrange the patch set - it's not clear
> which patches should go first. Maybe some patches should be dropped in favour
> of the meson project. I guess some patches will have to be re-implemented with
> meson (msvc warnings).
> Maybe the improvements to vcregress should go into v15 ? CI should run all the
> tests (which also serves to document *how* to run all the tests, even if there
> isn't a literal check-world target).
On Thu, Jun 23, 2022 at 02:31:25PM -0500, Justin Pryzby wrote:
> Should any of the test changes go into v15 ?
On Thu, Jul 07, 2022 at 07:22:32PM -0500, Justin Pryzby wrote:
> Also, cirrus/freebsd task can run 3x faster with more CPUs.
Checking if there's interest in any/none of these patches ?
I have added several more.
Do you have an idea when the meson branch might be merged ?
Will vcregress remain for a while, or will it go away for v16 ?
--
Justin
^ permalink raw reply [nested|flat] 43+ messages in thread
* Re: CI and test improvements
@ 2022-08-28 16:07 Andres Freund <[email protected]>
parent: Justin Pryzby <[email protected]>
0 siblings, 1 reply; 43+ messages in thread
From: Andres Freund @ 2022-08-28 16:07 UTC (permalink / raw)
To: Justin Pryzby <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; Thomas Munro <[email protected]>; [email protected]; Noah Misch <[email protected]>; Michael Paquier <[email protected]>; Anastasia Lubennikova <[email protected]>; Tom Lane <[email protected]>; Robert Haas <[email protected]>; Melanie Plageman <[email protected]>; Peter Eisentraut <[email protected]>; Daniel Gustafsson <[email protected]>
Hi,
On 2022-08-28 09:44:47 -0500, Justin Pryzby wrote:
> On Sat, May 28, 2022 at 10:37:41AM -0500, Justin Pryzby wrote:
> > I'm anticipating the need to further re-arrange the patch set - it's not clear
> > which patches should go first. Maybe some patches should be dropped in favour
> > of the meson project. I guess some patches will have to be re-implemented with
> > meson (msvc warnings).
>
> > Maybe the improvements to vcregress should go into v15 ? CI should run all the
> > tests (which also serves to document *how* to run all the tests, even if there
> > isn't a literal check-world target).
>
> On Thu, Jun 23, 2022 at 02:31:25PM -0500, Justin Pryzby wrote:
> > Should any of the test changes go into v15 ?
>
> On Thu, Jul 07, 2022 at 07:22:32PM -0500, Justin Pryzby wrote:
> > Also, cirrus/freebsd task can run 3x faster with more CPUs.
>
> Checking if there's interest in any/none of these patches ?
> I have added several more.
>
> Do you have an idea when the meson branch might be merged ?
I hope to do that fairly soon, but it's of course dependant on review etc. The
plan was to merge it early and mature it in tree to some degree. There's only
so much we can do "from the outside"...
> Will vcregress remain for a while, or will it go away for v16 ?
The plan was for the windows stuff to go away fairly quickly.
> From 99ee0bef5054539aad0e23a24dd9c9cc9ccee788 Mon Sep 17 00:00:00 2001
> From: Justin Pryzby <[email protected]>
> Date: Wed, 25 May 2022 21:53:22 -0500
> Subject: [PATCH 01/25] cirrus/windows: add compiler_warnings_script
Looks good.
> - MSBFLAGS: -m -verbosity:minimal "-consoleLoggerParameters:Summary;ForceNoAlign" /p:TrackFileAccess=false -nologo
> + # -fileLoggerParameters1: write warnings to msbuild.warn.log.
> + MSBFLAGS: -m -verbosity:minimal "-consoleLoggerParameters:Summary;ForceNoAlign" /p:TrackFileAccess=false -nologo -fileLoggerParameters1:warningsonly;logfile=msbuild.warn.log
Except, I think it'd be good to split this line. What do you think about using
something like
MSBFLAGS: >-
-nologo
-m -verbosity:minimal
/p:TrackFileAccess=false
"-consoleLoggerParameters:Summary;ForceNoAlign"
-fileLoggerParameters1:warningsonly;logfile=msbuild.warn.log
I think that should work?
> # If tests hang forever, cirrus eventually times out. In that case log
> # output etc is not uploaded, making the problem hard to debug. Of course
> @@ -450,6 +451,11 @@ task:
> cd src/tools/msvc
> %T_C% perl vcregress.pl ecpgcheck
>
> + # These should be last, so all the important checks are always run
> + always:
> + compiler_warnings_script:
> + - sh src\tools\ci\windows-compiler-warnings msbuild.warn.log
> +
> on_failure:
> <<: *on_failure
> crashlog_artifacts:
> diff --git a/src/tools/ci/windows-compiler-warnings b/src/tools/ci/windows-compiler-warnings
> new file mode 100755
> index 00000000000..d6f9a1fc569
> --- /dev/null
> +++ b/src/tools/ci/windows-compiler-warnings
> @@ -0,0 +1,16 @@
> +#! /bin/sh
> +# Success if the given file doesn't exist or is empty, else fail
> +# This is a separate file only to avoid dealing with windows shell quoting and escaping.
> +set -e
> +
> +fn=$1
> +
> +if [ -s "$fn" ]
> +then
> + # Display the file's content, then exit indicating failure
> + cat "$fn"
> + exit 1
> +else
> + # Success
> + exit 0
> +fi
> --
> 2.17.1
Wouldn't that be doable as something like
sh -c 'if test -s file; then cat file;exit 1; fi"
inside .cirrus.yml?
> From 1064a0794e85e06b3a0eca4ed3765f078795cb36 Mon Sep 17 00:00:00 2001
> From: Justin Pryzby <[email protected]>
> Date: Sun, 3 Apr 2022 00:10:20 -0500
> Subject: [PATCH 03/25] cirrus/ccache: disable compression and show stats
>
> ccache since 4.0 enables zstd compression by default.
>
> With default compression enabled (https://cirrus-ci.com/task/6692342840164352):
> linux 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.
I stared at this commit message for a while, trying to make sense of it, and
couldn't really. I assume you're saying that the cirrus compression is better
with ccache compression disabled, but it's extremely hard to parse out of it.
This does too much at once. Show stats, change cache sizes, disable
compression.
> From 01e9abd386a4e6cc0125b97617fb42e695898cbf Mon Sep 17 00:00:00 2001
> From: Justin Pryzby <[email protected]>
> Date: Tue, 26 Jul 2022 20:30:02 -0500
> Subject: [PATCH 04/25] cirrus/ccache: add explicit cache keys..
>
> Since otherwise, building with ci-os-only will probably fail to use the normal
> cache, since the cache key is computed using both the task name and its *index*
> in the list of caches (internal/executor/cache.go:184).
Hm, perhaps worth confirming and/or reporting to cirrus rather?
> From 8de5c977686270b0a4e666a924ebe820a245913a Mon Sep 17 00:00:00 2001
> From: Justin Pryzby <[email protected]>
> Date: Sun, 24 Jul 2022 23:09:12 -0500
> Subject: [PATCH 05/25] silence make distprep and generated-headers
>
> this saves vertical screen space.
>
> https://www.postgresql.org/message-id/[email protected]
I don't feel this should go in as a part of CI changes. Or rather, I feel
uncomfortable committing it when just discussed under this subject.
> From eaf263ccaa8310c5d9834b97e93ad8434d63296e Mon Sep 17 00:00:00 2001
> From: Justin Pryzby <[email protected]>
> Date: Sun, 24 Jul 2022 22:44:53 -0500
> Subject: [PATCH 06/25] pg_regress: run more quietly
>
> The number of lines of output should be closer to 1 per test, rather than 25 +
> 1 per test.
>
> https://www.postgresql.org/message-id/flat/20220221173109.yl6dqqu3ud52ripd%40alap3.anarazel.de
See above. There's also a dedicated thread about revising the output.
> From 6a6a97fc869fd1fd8b7ab5da5147f145581634f9 Mon Sep 17 00:00:00 2001
> From: Justin Pryzby <[email protected]>
> Date: Fri, 24 Jun 2022 00:09:12 -0500
> Subject: [PATCH 08/25] cirrus/freebsd: run with more CPUs+RAM and do not
> repartitiion
>
> There was some historic problem where tests under freebsd took 8+ minutes (and
> before 4a288a37f took 15 minutes).
>
> This reduces test time from 10min to 3min.
> 4 CPUs 4 tests https://cirrus-ci.com/task/4880240739614720
> 4 CPUs 6 tests https://cirrus-ci.com/task/4664440120410112 https://cirrus-ci.com/task/4586784884523008
> 4 CPUs 8 tests https://cirrus-ci.com/task/5001995491737600
>
> 6 CPUs https://cirrus-ci.com/task/6678321684545536
> 8 CPUs https://cirrus-ci.com/task/6264854121021440
>
> See also:
> https://www.postgresql.org/message-id/flat/[email protected]#f36c0b1...
> 8 jobs 7min https://cirrus-ci.com/task/6186376667332608
>
> xi-os-only: freebsd
Typo.
> @@ -71,8 +69,6 @@ task:
> fingerprint_key: ccache/freebsd
> reupload_on_changes: true
>
> - # Workaround around performance issues due to 32KB block size
> - repartition_script: src/tools/ci/gcp_freebsd_repartition.sh
> create_user_script: |
> pw useradd postgres
> chown -R postgres:postgres .
> --
What's the story there - at some point that was important for performance
because of the native block size triggering significant read-modify-write
cycles with postres' writes. You didn't comment on it in the commit message.
> From fd1c36a0bd8fa608ccdff5be3735dac5e3e48bf3 Mon Sep 17 00:00:00 2001
> From: Justin Pryzby <[email protected]>
> Date: Wed, 27 Jul 2022 16:54:47 -0500
> Subject: [PATCH 09/25] cirrus/freebsd: run build+check in a make vpath
> From 7052a32a21752b59632225684fc9426bb94e46e0 Mon Sep 17 00:00:00 2001
> From: Justin Pryzby <[email protected]>
> Date: Sun, 13 Feb 2022 17:56:40 -0600
> Subject: [PATCH 10/25] cirrus/windows: increase timeout to 25min
No explanation?
> From 602983b2cf37fc43465c62330b2e15e9d6d2035d Mon Sep 17 00:00:00 2001
> From: Justin Pryzby <[email protected]>
> Date: Fri, 26 Aug 2022 12:00:10 -0500
> Subject: [PATCH 15/25] f!and chdir
I don't see the point of pointing fixup commits to the list.
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 43+ messages in thread
* Re: CI and test improvements
@ 2022-08-28 17:10 Justin Pryzby <[email protected]>
parent: Andres Freund <[email protected]>
0 siblings, 1 reply; 43+ messages in thread
From: Justin Pryzby @ 2022-08-28 17:10 UTC (permalink / raw)
To: Andres Freund <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; Thomas Munro <[email protected]>; [email protected]; Noah Misch <[email protected]>; Michael Paquier <[email protected]>; Anastasia Lubennikova <[email protected]>; Tom Lane <[email protected]>; Robert Haas <[email protected]>; Melanie Plageman <[email protected]>; Peter Eisentraut <[email protected]>; Daniel Gustafsson <[email protected]>
On Sun, Aug 28, 2022 at 09:07:52AM -0700, Andres Freund wrote:
> > --- /dev/null
> > +++ b/src/tools/ci/windows-compiler-warnings
>
> Wouldn't that be doable as something like
> sh -c 'if test -s file; then cat file;exit 1; fi"
> inside .cirrus.yml?
I had written it inline in a couple ways, like
- sh -exc 'f=msbuild.warn.log; if [ -s "$f" ]; then cat "$f"; exit 1; else exit 0; fi'
but then separated it out as you suggested in
[email protected]
after I complained about cmd.exe requiring escaping for && and ||
That makes writing any shell script a bit perilous and a separate script
seems better.
> > Subject: [PATCH 03/25] cirrus/ccache: disable compression and show stats
> >
> > ccache since 4.0 enables zstd compression by default.
> >
> > With default compression enabled (https://cirrus-ci.com/task/6692342840164352):
> > linux 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.
>
> I stared at this commit message for a while, trying to make sense of it, and
> couldn't really. I assume you're saying that the cirrus compression is better
> with ccache compression disabled, but it's extremely hard to parse out of it.
Yes, because ccache uses zstd-1, and cirrus uses gzip, which it's going
to use no matter what ccache does, and gzip's default -6 is better than
ccache's zstd-1.
> This does too much at once. Show stats, change cache sizes, disable
> compression.
The cache size change is related to the compression level change; ccache
prunes based on the local size, which was compressed with zstd-1 and,
with this patch, not compressed (so ~2x larger). Also, it's more
interesting to control the size uploaded to cirrus (after compression
ith gzip-6).
> > From 01e9abd386a4e6cc0125b97617fb42e695898cbf Mon Sep 17 00:00:00 2001
> > From: Justin Pryzby <[email protected]>
> > Date: Tue, 26 Jul 2022 20:30:02 -0500
> > Subject: [PATCH 04/25] cirrus/ccache: add explicit cache keys..
> >
> > Since otherwise, building with ci-os-only will probably fail to use the normal
> > cache, since the cache key is computed using both the task name and its *index*
> > in the list of caches (internal/executor/cache.go:184).
>
> Hm, perhaps worth confirming and/or reporting to cirrus rather?
I know because of reading their source. Unfortunately, there's no
commit history indicating the intent or rationale.
https://github.com/cirruslabs/cirrus-ci-agent/blob/master/internal/executor/cache.go#L183
> > From 6a6a97fc869fd1fd8b7ab5da5147f145581634f9 Mon Sep 17 00:00:00 2001
> > From: Justin Pryzby <[email protected]>
> > Date: Fri, 24 Jun 2022 00:09:12 -0500
> > Subject: [PATCH 08/25] cirrus/freebsd: run with more CPUs+RAM and do not
> > repartitiion
> >
> > There was some historic problem where tests under freebsd took 8+ minutes (and
> > before 4a288a37f took 15 minutes).
> >
> > This reduces test time from 10min to 3min.
> > 4 CPUs 4 tests https://cirrus-ci.com/task/4880240739614720
> > 4 CPUs 6 tests https://cirrus-ci.com/task/4664440120410112 https://cirrus-ci.com/task/4586784884523008
> > 4 CPUs 8 tests https://cirrus-ci.com/task/5001995491737600
> >
> > 6 CPUs https://cirrus-ci.com/task/6678321684545536
> > 8 CPUs https://cirrus-ci.com/task/6264854121021440
> >
> > See also:
> > https://www.postgresql.org/message-id/flat/[email protected]#f36c0b1...
> > 8 jobs 7min https://cirrus-ci.com/task/6186376667332608
> >
> > xi-os-only: freebsd
>
> Typo.
No - it's deliberate so I can switch to and from "everything" to "this
only".
> > @@ -71,8 +69,6 @@ task:
> > fingerprint_key: ccache/freebsd
> > reupload_on_changes: true
> >
> > - # Workaround around performance issues due to 32KB block size
> > - repartition_script: src/tools/ci/gcp_freebsd_repartition.sh
> > create_user_script: |
> > pw useradd postgres
> > chown -R postgres:postgres .
> > --
>
> What's the story there - at some point that was important for performance
> because of the native block size triggering significant read-modify-write
> cycles with postres' writes. You didn't comment on it in the commit message.
Well, I don't know the history, but it seems to be unneeded now.
Is there a good description of the original problem ? Originally,
freebsd check-world took ~15min to run tests, and when we changed to use
-Og it took 10min. Since then, seems to have improved on its own, and
currently takes ~6min. This patch adds CPUs to make it run in ~4min,
and takes the opportuity to drop the historic repartition stuff.
> > From fd1c36a0bd8fa608ccdff5be3735dac5e3e48bf3 Mon Sep 17 00:00:00 2001
> > From: Justin Pryzby <[email protected]>
> > Date: Wed, 27 Jul 2022 16:54:47 -0500
> > Subject: [PATCH 09/25] cirrus/freebsd: run build+check in a make vpath
>
> > From 7052a32a21752b59632225684fc9426bb94e46e0 Mon Sep 17 00:00:00 2001
> > From: Justin Pryzby <[email protected]>
> > Date: Sun, 13 Feb 2022 17:56:40 -0600
> > Subject: [PATCH 10/25] cirrus/windows: increase timeout to 25min
>
> No explanation?
Because of the immediately following commit which makes it run all the
tests.
> > From 602983b2cf37fc43465c62330b2e15e9d6d2035d Mon Sep 17 00:00:00 2001
> > From: Justin Pryzby <[email protected]>
> > Date: Fri, 26 Aug 2022 12:00:10 -0500
> > Subject: [PATCH 15/25] f!and chdir
>
> I don't see the point of pointing fixup commits to the list.
It's a separate commit to make it easy to see the changes, separately,
since I imagine maybe the "chdir" part won't be desirable, or maybe the
PATH part won't. But I'm not sure, so I'm here soliciting feedback.
--
Justin
^ permalink raw reply [nested|flat] 43+ messages in thread
* Re: CI and test improvements
@ 2022-08-28 21:28 Andres Freund <[email protected]>
parent: Justin Pryzby <[email protected]>
0 siblings, 2 replies; 43+ messages in thread
From: Andres Freund @ 2022-08-28 21:28 UTC (permalink / raw)
To: Justin Pryzby <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; Thomas Munro <[email protected]>; [email protected]; Noah Misch <[email protected]>; Michael Paquier <[email protected]>; Anastasia Lubennikova <[email protected]>; Tom Lane <[email protected]>; Robert Haas <[email protected]>; Melanie Plageman <[email protected]>; Peter Eisentraut <[email protected]>; Daniel Gustafsson <[email protected]>
Hi,
On 2022-08-28 12:10:29 -0500, Justin Pryzby wrote:
> On Sun, Aug 28, 2022 at 09:07:52AM -0700, Andres Freund wrote:
> > > --- /dev/null
> > > +++ b/src/tools/ci/windows-compiler-warnings
> >
> > Wouldn't that be doable as something like
> > sh -c 'if test -s file; then cat file;exit 1; fi"
> > inside .cirrus.yml?
>
> I had written it inline in a couple ways, like
> - sh -exc 'f=msbuild.warn.log; if [ -s "$f" ]; then cat "$f"; exit 1; else exit 0; fi'
>
> but then separated it out as you suggested in
> [email protected]
>
> after I complained about cmd.exe requiring escaping for && and ||
> That makes writing any shell script a bit perilous and a separate script
> seems better.
I remember that I suggested it - but note that the way I wrote above doesn't
have anything needing escaping. Anyway, what do you think of the multiline
split I suggested?
> > > Subject: [PATCH 03/25] cirrus/ccache: disable compression and show stats
> > >
> > > ccache since 4.0 enables zstd compression by default.
> > >
> > > With default compression enabled (https://cirrus-ci.com/task/6692342840164352):
> > > linux 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.
> >
> > I stared at this commit message for a while, trying to make sense of it, and
> > couldn't really. I assume you're saying that the cirrus compression is better
> > with ccache compression disabled, but it's extremely hard to parse out of it.
>
> Yes, because ccache uses zstd-1, and cirrus uses gzip, which it's going
> to use no matter what ccache does, and gzip's default -6 is better than
> ccache's zstd-1.
>
> > This does too much at once. Show stats, change cache sizes, disable
> > compression.
>
> The cache size change is related to the compression level change; ccache
> prunes based on the local size, which was compressed with zstd-1 and,
> with this patch, not compressed (so ~2x larger). Also, it's more
> interesting to control the size uploaded to cirrus (after compression
> ith gzip-6).
That's what should have been in the commit message.
> > > From 6a6a97fc869fd1fd8b7ab5da5147f145581634f9 Mon Sep 17 00:00:00 2001
> > > From: Justin Pryzby <[email protected]>
> > > Date: Fri, 24 Jun 2022 00:09:12 -0500
> > > Subject: [PATCH 08/25] cirrus/freebsd: run with more CPUs+RAM and do not
> > > repartitiion
> > >
> > > There was some historic problem where tests under freebsd took 8+ minutes (and
> > > before 4a288a37f took 15 minutes).
> > >
> > > This reduces test time from 10min to 3min.
> > > 4 CPUs 4 tests https://cirrus-ci.com/task/4880240739614720
> > > 4 CPUs 6 tests https://cirrus-ci.com/task/4664440120410112 https://cirrus-ci.com/task/4586784884523008
> > > 4 CPUs 8 tests https://cirrus-ci.com/task/5001995491737600
> > >
> > > 6 CPUs https://cirrus-ci.com/task/6678321684545536
> > > 8 CPUs https://cirrus-ci.com/task/6264854121021440
> > >
> > > See also:
> > > https://www.postgresql.org/message-id/flat/[email protected]#f36c0b1...
> > > 8 jobs 7min https://cirrus-ci.com/task/6186376667332608
> > >
> > > xi-os-only: freebsd
> >
> > Typo.
>
> No - it's deliberate so I can switch to and from "everything" to "this
> only".
I don't see the point in posting patches to be applied if they contain lots of
such things that a potential committer would need to catch and include a lot
of of fixup patches.
> > > @@ -71,8 +69,6 @@ task:
> > > fingerprint_key: ccache/freebsd
> > > reupload_on_changes: true
> > >
> > > - # Workaround around performance issues due to 32KB block size
> > > - repartition_script: src/tools/ci/gcp_freebsd_repartition.sh
> > > create_user_script: |
> > > pw useradd postgres
> > > chown -R postgres:postgres .
> > > --
> >
> > What's the story there - at some point that was important for performance
> > because of the native block size triggering significant read-modify-write
> > cycles with postres' writes. You didn't comment on it in the commit message.
>
> Well, I don't know the history, but it seems to be unneeded now.
It's possible it was mainly needed for testing with aio + dio. But also
possible that an upgrade improved the situation since.
> > > From fd1c36a0bd8fa608ccdff5be3735dac5e3e48bf3 Mon Sep 17 00:00:00 2001
> > > From: Justin Pryzby <[email protected]>
> > > Date: Wed, 27 Jul 2022 16:54:47 -0500
> > > Subject: [PATCH 09/25] cirrus/freebsd: run build+check in a make vpath
> >
> > > From 7052a32a21752b59632225684fc9426bb94e46e0 Mon Sep 17 00:00:00 2001
> > > From: Justin Pryzby <[email protected]>
> > > Date: Sun, 13 Feb 2022 17:56:40 -0600
> > > Subject: [PATCH 10/25] cirrus/windows: increase timeout to 25min
> >
> > No explanation?
>
> Because of the immediately following commit which makes it run all the
> tests.
Mention that in the commit message then. Especially when dealing with 25
commits I don't think you can expect others to infer such things.
> > > From 602983b2cf37fc43465c62330b2e15e9d6d2035d Mon Sep 17 00:00:00 2001
> > > From: Justin Pryzby <[email protected]>
> > > Date: Fri, 26 Aug 2022 12:00:10 -0500
> > > Subject: [PATCH 15/25] f!and chdir
> >
> > I don't see the point of pointing fixup commits to the list.
>
> It's a separate commit to make it easy to see the changes, separately,
> since I imagine maybe the "chdir" part won't be desirable, or maybe the
> PATH part won't. But I'm not sure, so I'm here soliciting feedback.
Shrug, I doubt you'll get much if asked that way.
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 43+ messages in thread
* Re: CI and test improvements
@ 2022-09-10 20:05 Justin Pryzby <[email protected]>
parent: Andres Freund <[email protected]>
1 sibling, 2 replies; 43+ messages in thread
From: Justin Pryzby @ 2022-09-10 20:05 UTC (permalink / raw)
To: Andres Freund <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; Thomas Munro <[email protected]>; [email protected]; Noah Misch <[email protected]>; Michael Paquier <[email protected]>; Anastasia Lubennikova <[email protected]>; Tom Lane <[email protected]>; Robert Haas <[email protected]>; Melanie Plageman <[email protected]>; Peter Eisentraut <[email protected]>; Daniel Gustafsson <[email protected]>
On Sun, Aug 28, 2022 at 02:28:02PM -0700, Andres Freund wrote:
> On 2022-08-28 12:10:29 -0500, Justin Pryzby wrote:
> > On Sun, Aug 28, 2022 at 09:07:52AM -0700, Andres Freund wrote:
> > > > --- /dev/null
> > > > +++ b/src/tools/ci/windows-compiler-warnings
> > >
> > > Wouldn't that be doable as something like
> > > sh -c 'if test -s file; then cat file;exit 1; fi"
> > > inside .cirrus.yml?
> >
> > I had written it inline in a couple ways, like
> > - sh -exc 'f=msbuild.warn.log; if [ -s "$f" ]; then cat "$f"; exit 1; else exit 0; fi'
> >
> > but then separated it out as you suggested in
> > [email protected]
> >
> > after I complained about cmd.exe requiring escaping for && and ||
> > That makes writing any shell script a bit perilous and a separate script
> > seems better.
>
> I remember that I suggested it - but note that the way I wrote above doesn't
> have anything needing escaping.
It doesn't require it, but that still gives the impression that it's
normally possible to write one-liner shell scripts there, which is
misleading/wrong, and the reason why I took your suggestion to use a
separate script file.
> Anyway, what do you think of the multiline split I suggested?
Done, and sorted.
> That's what should have been in the commit message.
Sure. I copied into the commit message the explanation that I had
written in June's email.
> > > > xi-os-only: freebsd
> > >
> > > Typo.
> >
> > No - it's deliberate so I can switch to and from "everything" to "this
> > only".
>
> I don't see the point in posting patches to be applied if they contain lots of
> such things that a potential committer would need to catch and include a lot
> of of fixup patches.
I get that you disliked that I disabled the effect of a CI tag by
munging "c" to "x". I've amended the message to avoid confusion. But,
lots of what such things ? "ci-os-only" would be removed before being
pushed anyway.
"catching things" is the first part of the review process, which (as I
understand) is intended to help patch authors to improve their patches.
If you found lots of problems in my patches, I'd need to know about
them; but most of what I heard seem like quibbles about the presentation
of the patches. It's true that some parts are dirty/unclear, and that
seems reasonable for patches most of which haven't yet received review,
for which I asked whether to pursue the patch at all, and how best to
present them. This is (or could be) an opportunity to make
improvements.
I renamed the two, related patches to Cluser.pm which said "f!", which
are deliberately separate but looked like "fixup" patches. Are you
interested in any combination of those three, related changes to move
logic from Makefile to perl ? If not, we don't need to debate the
merits of spliting the patch.
What about the three, related changes for ccache compression ?
Should these be dropped in favour of meson ?
- cirrus/vcregress: test modules/contrib with NO_INSTALLCHECK=1
- vcregress: add alltaptests
I added: "WIP: skip building if only docs have changed"
changesInclude() didn't seem to work right when I first tried to use it.
Eventually, I realized that it seems to use something like "git log",
and not "git diff" (as I'd thought). It seems to work fine now that I
know what to expect.
git commit --amend --no-edit
git diff --stat @{1}..@{0} # this outputs nothing
git log --stat @{1}..@{0} # this lists the files changed by the tip commit
It'd be nice to be have cfbot inject this patch into each commitfest
patch for awhile, to make sure everything works as expected. Same for
the code coverage patch and the doc artifacts patch. (These patches
currently assume that the base commit is HEAD~1, which is correct for
cfbot, and that would also provide code coverage and docs until such
time as cfbot is updated to apply and preserve the original series of
patches).
--
Justin
^ permalink raw reply [nested|flat] 43+ messages in thread
* Re: CI and test improvements
@ 2022-09-22 21:07 Justin Pryzby <[email protected]>
parent: Andres Freund <[email protected]>
1 sibling, 2 replies; 43+ messages in thread
From: Justin Pryzby @ 2022-09-22 21:07 UTC (permalink / raw)
To: Andres Freund <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; Thomas Munro <[email protected]>; [email protected]; Noah Misch <[email protected]>; Michael Paquier <[email protected]>; Anastasia Lubennikova <[email protected]>; Tom Lane <[email protected]>; Robert Haas <[email protected]>; Melanie Plageman <[email protected]>; Peter Eisentraut <[email protected]>; Daniel Gustafsson <[email protected]>
On Sun, Aug 28, 2022 at 02:28:02PM -0700, Andres Freund wrote:
> > > > @@ -71,8 +69,6 @@ task:
> > > > fingerprint_key: ccache/freebsd
> > > > reupload_on_changes: true
> > > >
> > > > - # Workaround around performance issues due to 32KB block size
> > > > - repartition_script: src/tools/ci/gcp_freebsd_repartition.sh
> > > > create_user_script: |
> > > > pw useradd postgres
> > > > chown -R postgres:postgres .
> > > > --
> > >
> > > What's the story there - at some point that was important for performance
> > > because of the native block size triggering significant read-modify-write
> > > cycles with postres' writes. You didn't comment on it in the commit message.
> >
> > Well, I don't know the history, but it seems to be unneeded now.
>
> It's possible it was mainly needed for testing with aio + dio. But also
> possible that an upgrade improved the situation since.
Maybe freebsd got faster as a result of the TAU CPUs?
https://mobile.twitter.com/cirrus_labs/status/1534982111568052240
I noticed because it's been *slower* the last ~24h since cirrusci
disabled TAU, as Thomas commit mentioned.
https://twitter.com/cirrus_labs/status/1572657320093712384
For example this CF entry:
https://cirrus-ci.com/github/postgresql-cfbot/postgresql/commitfest/39/3736
https://cirrus-ci.com/task/4670794365140992 5m36s - 4days ago
https://cirrus-ci.com/task/4974926233862144 5m25s - 3days ago
https://cirrus-ci.com/task/5561409034518528 5m29s - 2days ago
https://cirrus-ci.com/task/6432442008469504 9m19s - yesterday
CF_BOT's latest tasks seem to be fast again, since 1-2h ago.
https://cirrus-ci.com/build/5178906041909248 9m1s
https://cirrus-ci.com/build/4593160281128960 5m8s
https://cirrus-ci.com/build/4539845644124160 5m22s
The logs for July show when freebsd started "being fast":
https://cirrus-ci.com/github/postgresql-cfbot/postgresql/commitfest/38/3708
https://cirrus-ci.com/task/6316073015312384 10m25s Jul 13
https://cirrus-ci.com/task/5662878987452416 5m48s Jul 15
Maybe that changed in July rather than June because the TAU CPUs were
still not available in every region/zone (?)
https://cloud.google.com/compute/docs/regions-zones/
I have no idea if the TAU CPUs eliminate/mitigate the original
performance issue you had with AIO. But they have such a large effect
on freebsd that it could now be the fastest task, if given more than 2
CPUs.
--
Justin
^ permalink raw reply [nested|flat] 43+ messages in thread
* Re: CI and test improvements
@ 2022-09-22 21:53 Andres Freund <[email protected]>
parent: Justin Pryzby <[email protected]>
1 sibling, 0 replies; 43+ messages in thread
From: Andres Freund @ 2022-09-22 21:53 UTC (permalink / raw)
To: Justin Pryzby <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; Thomas Munro <[email protected]>; [email protected]; Noah Misch <[email protected]>; Michael Paquier <[email protected]>; Anastasia Lubennikova <[email protected]>; Tom Lane <[email protected]>; Robert Haas <[email protected]>; Melanie Plageman <[email protected]>; Peter Eisentraut <[email protected]>; Daniel Gustafsson <[email protected]>
Hi,
On 2022-09-22 16:07:02 -0500, Justin Pryzby wrote:
> On Sun, Aug 28, 2022 at 02:28:02PM -0700, Andres Freund wrote:
> > > > > @@ -71,8 +69,6 @@ task:
> > > > > fingerprint_key: ccache/freebsd
> > > > > reupload_on_changes: true
> > > > >
> > > > > - # Workaround around performance issues due to 32KB block size
> > > > > - repartition_script: src/tools/ci/gcp_freebsd_repartition.sh
> > > > > create_user_script: |
> > > > > pw useradd postgres
> > > > > chown -R postgres:postgres .
> > > > > --
> > > >
> > > > What's the story there - at some point that was important for performance
> > > > because of the native block size triggering significant read-modify-write
> > > > cycles with postres' writes. You didn't comment on it in the commit message.
> > >
> > > Well, I don't know the history, but it seems to be unneeded now.
> >
> > It's possible it was mainly needed for testing with aio + dio. But also
> > possible that an upgrade improved the situation since.
>
> Maybe freebsd got faster as a result of the TAU CPUs?
> https://mobile.twitter.com/cirrus_labs/status/1534982111568052240
>
> I noticed because it's been *slower* the last ~24h since cirrusci
> disabled TAU, as Thomas commit mentioned.
> https://twitter.com/cirrus_labs/status/1572657320093712384
Yea, I noticed that as well. It's entirely possible that something in the
"hardware" stack improved sufficiently to avoid problems.
> I have no idea if the TAU CPUs eliminate/mitigate the original
> performance issue you had with AIO. But they have such a large effect
> on freebsd that it could now be the fastest task, if given more than 2
> CPUs.
I'm planning to rebase early next week and try that out.
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 43+ messages in thread
* Re: CI and test improvements
@ 2022-09-23 03:32 Thomas Munro <[email protected]>
parent: Justin Pryzby <[email protected]>
1 sibling, 0 replies; 43+ messages in thread
From: Thomas Munro @ 2022-09-23 03:32 UTC (permalink / raw)
To: Justin Pryzby <[email protected]>; +Cc: Andres Freund <[email protected]>; Andrew Dunstan <[email protected]>; [email protected]; Noah Misch <[email protected]>; Michael Paquier <[email protected]>; Anastasia Lubennikova <[email protected]>; Tom Lane <[email protected]>; Robert Haas <[email protected]>; Melanie Plageman <[email protected]>; Peter Eisentraut <[email protected]>; Daniel Gustafsson <[email protected]>
On Fri, Sep 23, 2022 at 9:07 AM Justin Pryzby <[email protected]> wrote:
> On Sun, Aug 28, 2022 at 02:28:02PM -0700, Andres Freund wrote:
> > > > > - # Workaround around performance issues due to 32KB block size
> > > > > - repartition_script: src/tools/ci/gcp_freebsd_repartition.sh
> > > > > create_user_script: |
> > > > > pw useradd postgres
> > > > > chown -R postgres:postgres .
> > > >
> > > > What's the story there - at some point that was important for performance
> > > > because of the native block size triggering significant read-modify-write
> > > > cycles with postres' writes. You didn't comment on it in the commit message.
> > >
> > > Well, I don't know the history, but it seems to be unneeded now.
> >
> > It's possible it was mainly needed for testing with aio + dio. But also
> > possible that an upgrade improved the situation since.
Yeah, it is very important for direct I/O (patches soon...), because
every 8KB random write becomes a read-32KB/wait/write-32KB without it.
It's slightly less important for buffered I/O, because the kernel
caches hide that, but it still triggers I/O bandwidth amplification,
and we definitely saw positive effects earlier on the CI system back
on the previous generation. FWIW I am planning to see about getting
the FreeBSD installer to create the root file system the way we want,
instead of this ugliness.
> Maybe freebsd got faster as a result of the TAU CPUs?
> [data]
Very interesting. Would be good to find the exact instance types +
storage types to see if there has been a massive IOPS boost, perhaps
via local SSD. The newer times are getting closer to a local
developer machine.
^ permalink raw reply [nested|flat] 43+ messages in thread
* Re: CI and test improvements
@ 2022-10-02 00:45 Andres Freund <[email protected]>
parent: Justin Pryzby <[email protected]>
1 sibling, 1 reply; 43+ messages in thread
From: Andres Freund @ 2022-10-02 00:45 UTC (permalink / raw)
To: Justin Pryzby <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; Thomas Munro <[email protected]>; [email protected]; Noah Misch <[email protected]>; Michael Paquier <[email protected]>; Anastasia Lubennikova <[email protected]>; Tom Lane <[email protected]>; Robert Haas <[email protected]>; Melanie Plageman <[email protected]>; Peter Eisentraut <[email protected]>; Daniel Gustafsson <[email protected]>
Hi,
On 2022-09-10 15:05:42 -0500, Justin Pryzby wrote:
> From 4ed5eb427de4508a4c3422e60891b45c8512814a Mon Sep 17 00:00:00 2001
> From: Justin Pryzby <[email protected]>
> Date: Sun, 3 Apr 2022 00:10:20 -0500
> Subject: [PATCH 03/23] cirrus/ccache: disable compression and show stats
>
> 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).
I wonder whether we could instead change CCACHE_COMPRESSLEVEL (maybe 3, zstd's
default IIRC). It'd be good if we could increase cache utilization.
> From 0bd5f51b8c143ed87a867987309d66b8554b1fd6 Mon Sep 17 00:00:00 2001
> From: Justin Pryzby <[email protected]>
> Date: Thu, 14 Apr 2022 06:27:07 -0500
> Subject: [PATCH 05/23] cirrus: enable various runtime checks on macos and
> freebsd
>
> windows is slower than freebsd and mac, so it's okay to enable options which
> will slow them down some. Also, the cirrusci mac instances always have lot of
> cores available.
> See:
> https://www.postgresql.org/message-id/[email protected]
> https://www.postgresql.org/message-id/20211213211223.vkgg3wwiss2tragj%40alap3.anarazel.de
> https://www.postgresql.org/message-id/CAH2-WzmevBhKNEtqX3N-Tkb0gVBHH62C0KfeTxXzqYES_PiFiA%40mail.gma...
> https://www.postgresql.org/message-id/[email protected]
>
> ci-os-only: freebsd, macos
> ---
> .cirrus.yml | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/.cirrus.yml b/.cirrus.yml
> index 183e8746ce6..4ad20892eeb 100644
> --- a/.cirrus.yml
> +++ b/.cirrus.yml
> @@ -113,7 +113,9 @@ task:
> \
> CC="ccache cc" \
> CXX="ccache c++" \
> - CFLAGS="-Og -ggdb"
> + CPPFLAGS="-DRELCACHE_FORCE_RELEASE -DCOPY_PARSE_PLAN_TREES -DWRITE_READ_PARSE_PLAN_TREES -DRAW_EXPRESSION_COVERAGE_TEST" \
> + CXXFLAGS="-Og -ggdb -march=native -mtune=native" \
> + CFLAGS="-Og -ggdb -march=native -mtune=native"
What's reason for -march=native -mtune=native here?
> EOF
> build_script: |
> su postgres -c "ccache --zero-stats"
> @@ -336,8 +338,8 @@ task:
> CC="ccache cc" \
> CXX="ccache c++" \
> CLANG="ccache ${brewpath}/llvm/bin/ccache" \
> - CFLAGS="-Og -ggdb" \
> - CXXFLAGS="-Og -ggdb" \
> + CFLAGS="-Og -ggdb -DRANDOMIZE_ALLOCATED_MEMORY" \
> + CXXFLAGS="-Og -ggdb -DRANDOMIZE_ALLOCATED_MEMORY" \
> \
> LLVM_CONFIG=${brewpath}/llvm/bin/llvm-config \
> PYTHON=python3
I'd also use CPPFLAGS here, given you'd used it above...
I'm planning to commit an updated version of this change soon, without the
-march=native -mtune=native bit, unless somebody protests...
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 43+ messages in thread
* Re: CI and test improvements
@ 2022-10-02 00:58 Justin Pryzby <[email protected]>
parent: Andres Freund <[email protected]>
0 siblings, 1 reply; 43+ messages in thread
From: Justin Pryzby @ 2022-10-02 00:58 UTC (permalink / raw)
To: Andres Freund <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; Thomas Munro <[email protected]>; [email protected]; Noah Misch <[email protected]>; Michael Paquier <[email protected]>; Anastasia Lubennikova <[email protected]>; Tom Lane <[email protected]>; Robert Haas <[email protected]>; Melanie Plageman <[email protected]>; Peter Eisentraut <[email protected]>; Daniel Gustafsson <[email protected]>
On Sat, Oct 01, 2022 at 05:45:01PM -0700, Andres Freund wrote:
> Hi,
>
> On 2022-09-10 15:05:42 -0500, Justin Pryzby wrote:
> > From 4ed5eb427de4508a4c3422e60891b45c8512814a Mon Sep 17 00:00:00 2001
> > From: Justin Pryzby <[email protected]>
> > Date: Sun, 3 Apr 2022 00:10:20 -0500
> > Subject: [PATCH 03/23] cirrus/ccache: disable compression and show stats
> >
> > 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).
>
> I wonder whether we could instead change CCACHE_COMPRESSLEVEL (maybe 3, zstd's
> default IIRC). It'd be good if we could increase cache utilization.
I considered that (and I think that's what I wrote initially).
I figured that if cirrus is going to use gzip-6 (tar.gz) in any case, we
might as well disable compression. Then, all the tasks are also doing
the same thing (half the tasks have ccache before 4.0).
> > From 0bd5f51b8c143ed87a867987309d66b8554b1fd6 Mon Sep 17 00:00:00 2001
> > From: Justin Pryzby <[email protected]>
> > Date: Thu, 14 Apr 2022 06:27:07 -0500
> > Subject: [PATCH 05/23] cirrus: enable various runtime checks on macos and
> > freebsd
> >
> > windows is slower than freebsd and mac, so it's okay to enable options which
> > will slow them down some. Also, the cirrusci mac instances always have lot of
> > cores available.
>
> > See:
> > https://www.postgresql.org/message-id/[email protected]
> > https://www.postgresql.org/message-id/20211213211223.vkgg3wwiss2tragj%40alap3.anarazel.de
> > https://www.postgresql.org/message-id/CAH2-WzmevBhKNEtqX3N-Tkb0gVBHH62C0KfeTxXzqYES_PiFiA%40mail.gma...
> > https://www.postgresql.org/message-id/[email protected]
> >
> > ci-os-only: freebsd, macos
> > ---
> > .cirrus.yml | 8 +++++---
> > 1 file changed, 5 insertions(+), 3 deletions(-)
> >
> > diff --git a/.cirrus.yml b/.cirrus.yml
> > index 183e8746ce6..4ad20892eeb 100644
> > --- a/.cirrus.yml
> > +++ b/.cirrus.yml
> > @@ -113,7 +113,9 @@ task:
> > \
> > CC="ccache cc" \
> > CXX="ccache c++" \
> > - CFLAGS="-Og -ggdb"
> > + CPPFLAGS="-DRELCACHE_FORCE_RELEASE -DCOPY_PARSE_PLAN_TREES -DWRITE_READ_PARSE_PLAN_TREES -DRAW_EXPRESSION_COVERAGE_TEST" \
> > + CXXFLAGS="-Og -ggdb -march=native -mtune=native" \
> > + CFLAGS="-Og -ggdb -march=native -mtune=native"
>
> What's reason for -march=native -mtune=native here?
No particular reason, and my initial patch didn't have it.
I suppose I added it to test its effect and never got rid of it.
> > EOF
> > build_script: |
> > su postgres -c "ccache --zero-stats"
> > @@ -336,8 +338,8 @@ task:
> > CC="ccache cc" \
> > CXX="ccache c++" \
> > CLANG="ccache ${brewpath}/llvm/bin/ccache" \
> > - CFLAGS="-Og -ggdb" \
> > - CXXFLAGS="-Og -ggdb" \
> > + CFLAGS="-Og -ggdb -DRANDOMIZE_ALLOCATED_MEMORY" \
> > + CXXFLAGS="-Og -ggdb -DRANDOMIZE_ALLOCATED_MEMORY" \
> > \
> > LLVM_CONFIG=${brewpath}/llvm/bin/llvm-config \
> > PYTHON=python3
>
> I'd also use CPPFLAGS here, given you'd used it above...
>
> I'm planning to commit an updated version of this change soon, without the
> -march=native -mtune=native bit, unless somebody protests...
One other thing is that your -m32 changes caused the linux/meson task to
take an additional 3+ minutes (total ~8). That's no issue, except that
the Warnings task depends on the linux/mason task, and itself can take
up to 15 minutes.
So those two potentially take as long as the windows task.
I suggested that CompileWarnings could instead "Depend on: Freebsd",
which currently takes 6-7min (and could take 4-5min if given more CPUs).
--
Justin
^ permalink raw reply [nested|flat] 43+ messages in thread
* Re: CI and test improvements
@ 2022-10-02 01:36 Andres Freund <[email protected]>
parent: Justin Pryzby <[email protected]>
0 siblings, 1 reply; 43+ messages in thread
From: Andres Freund @ 2022-10-02 01:36 UTC (permalink / raw)
To: Justin Pryzby <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; Thomas Munro <[email protected]>; [email protected]; Noah Misch <[email protected]>; Michael Paquier <[email protected]>; Anastasia Lubennikova <[email protected]>; Tom Lane <[email protected]>; Robert Haas <[email protected]>; Melanie Plageman <[email protected]>; Peter Eisentraut <[email protected]>; Daniel Gustafsson <[email protected]>
Hi,
On 2022-10-01 19:58:01 -0500, Justin Pryzby wrote:
> One other thing is that your -m32 changes caused the linux/meson task to
> take an additional 3+ minutes (total ~8). That's no issue, except that
> the Warnings task depends on the linux/mason task, and itself can take
> up to 15 minutes.
> So those two potentially take as long as the windows task.
> I suggested that CompileWarnings could instead "Depend on: Freebsd",
> which currently takes 6-7min (and could take 4-5min if given more CPUs).
I am wondering if we should instead introduce a new "quickcheck" task that
just compiles and runs maybe one test and have *all* other tests depend on
that. Wasting a precious available windows instance to just fail to build or
immediately fail during tests doesn't really make sense.
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 43+ messages in thread
* Re: CI and test improvements
@ 2022-10-02 20:52 Andres Freund <[email protected]>
parent: Andres Freund <[email protected]>
0 siblings, 2 replies; 43+ messages in thread
From: Andres Freund @ 2022-10-02 20:52 UTC (permalink / raw)
To: Justin Pryzby <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; Thomas Munro <[email protected]>; [email protected]; Noah Misch <[email protected]>; Michael Paquier <[email protected]>; Anastasia Lubennikova <[email protected]>; Tom Lane <[email protected]>; Robert Haas <[email protected]>; Melanie Plageman <[email protected]>; Peter Eisentraut <[email protected]>; Daniel Gustafsson <[email protected]>
Hi,
On 2022-10-01 18:36:41 -0700, Andres Freund wrote:
> I am wondering if we should instead introduce a new "quickcheck" task that
> just compiles and runs maybe one test and have *all* other tests depend on
> that. Wasting a precious available windows instance to just fail to build or
> immediately fail during tests doesn't really make sense.
Attached is an implementation of that idea.
I fairly randomly chose two quick tests to execute as part of the sanity
check, cube/regress pg_ctl/001_start_stop. I wanted to have coverage for
initdb, a pg_regress style test, a tap test, some other client binary.
With a primed cache this takes ~32s, not too bad imo. 12s of that is cloning
the repo.
What do you think?
We could bake a bare repo into the images to make the clone step in faster,
but that'd be for later anyway.
set -e
rm -rf /tmp/pg-clone-better
mkdir /tmp/pg-clone-better
cd /tmp/pg-clone-better
git init --bare
git remote add origin https://github.com/postgres/postgres.git --no-tags -t 'REL_*' -t master
git fetch -v
git repack -ad -f
du -sh
results in a 227MB repo.
git clone https://github.com/anarazel/postgres.git -v --depth 1000 -b ci-sanitycheck --reference /tmp/pg-clone-better /tmp/pg-clone-better-clone
clones an example branch in ~1.35s.
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 43+ messages in thread
* Re: CI and test improvements
@ 2022-10-02 21:35 Justin Pryzby <[email protected]>
parent: Andres Freund <[email protected]>
1 sibling, 2 replies; 43+ messages in thread
From: Justin Pryzby @ 2022-10-02 21:35 UTC (permalink / raw)
To: Andres Freund <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; Thomas Munro <[email protected]>; [email protected]; Noah Misch <[email protected]>; Michael Paquier <[email protected]>; Anastasia Lubennikova <[email protected]>; Tom Lane <[email protected]>; Robert Haas <[email protected]>; Melanie Plageman <[email protected]>; Peter Eisentraut <[email protected]>; Daniel Gustafsson <[email protected]>
On Sun, Oct 02, 2022 at 01:52:01PM -0700, Andres Freund wrote:
> Hi,
>
> On 2022-10-01 18:36:41 -0700, Andres Freund wrote:
> > I am wondering if we should instead introduce a new "quickcheck" task that
> > just compiles and runs maybe one test and have *all* other tests depend on
> > that. Wasting a precious available windows instance to just fail to build or
> > immediately fail during tests doesn't really make sense.
> With a primed cache this takes ~32s, not too bad imo. 12s of that is
> cloning the repo.
Maybe - that would avoid waiting 4 minutes for a windows instance to
start in the (hopefully atypical) case of a patch that fails in 1-2
minutes under linux/freebsd.
If the patch were completely broken, the windows task would take ~4min
to start, plus up to ~4min before failing to compile or failing an early
test. 6-8 minutes isn't nothing, but doesn't seem worth the added
complexity.
Also, this would mean that in the common case, the slowest task would be
delayed until after the SanityCheck task instance starts, compiles, and
runs some test :( Your best case is 32sec, but I doubt that's going to
be typical.
I was thinking about the idea of cfbot handling "tasks" separately,
similar to what it used to do with travis/appveyor. The logic for
"windows tasks are only run if linux passes tests" could live there.
That could also be useful if there's ever the possibility of running an
additional OS on another CI provider, or if another provider can run
windows tasks faster, or if we need to reduce our load/dependency on
cirrus. I realized that goes backwards in some ways to the direction
we've gone with cirrus, and I'm not sure how exactly it would do that (I
suppose it might add ci-os-only tags to its commit message).
> + # no options enabled, should be small
> + CCACHE_MAXSIZE: "150M"
Actually, tasks can share caches if the "cache key" is set.
If there was a separate "Sanity" task, I think it should use whatever
flags linux (or freebsd) use to avoid doing two compilations (with lots
of cache misses for patches which modify *.h files, which would then
happen twice, in serial).
> + # use always: to continue after failures. Task that did not run count as a
> + # success, so we need to recheck SanityChecks's condition here ...
> - # task that did not run, count as a success, so we need to recheck Linux'
> - # condition here ...
Another/better justification/description is that "cirrus warns if the
depending task has different only_if conditions than the dependant task".
--
Justin
^ permalink raw reply [nested|flat] 43+ messages in thread
* Re: CI and test improvements
@ 2022-10-02 21:51 Andres Freund <[email protected]>
parent: Justin Pryzby <[email protected]>
1 sibling, 0 replies; 43+ messages in thread
From: Andres Freund @ 2022-10-02 21:51 UTC (permalink / raw)
To: Justin Pryzby <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; Thomas Munro <[email protected]>; [email protected]; Noah Misch <[email protected]>; Michael Paquier <[email protected]>; Anastasia Lubennikova <[email protected]>; Tom Lane <[email protected]>; Robert Haas <[email protected]>; Melanie Plageman <[email protected]>; Peter Eisentraut <[email protected]>; Daniel Gustafsson <[email protected]>
Hi,
On 2022-10-02 16:35:06 -0500, Justin Pryzby wrote:
> Maybe - that would avoid waiting 4 minutes for a windows instance to
> start in the (hopefully atypical) case of a patch that fails in 1-2
> minutes under linux/freebsd.
>
> If the patch were completely broken, the windows task would take ~4min
> to start, plus up to ~4min before failing to compile or failing an early
> test. 6-8 minutes isn't nothing, but doesn't seem worth the added
> complexity.
Avoiding 6-8mins of wasted windows time would, I think, allow us to crank
cfbot's concurrency up a notch or two.
> Also, this would mean that in the common case, the slowest task would be
> delayed until after the SanityCheck task instance starts, compiles, and
> runs some test :( Your best case is 32sec, but I doubt that's going to
> be typical.
Even the worst case isn't that bad, the uncached minimal build is 67s.
> I was thinking about the idea of cfbot handling "tasks" separately,
> similar to what it used to do with travis/appveyor. The logic for
> "windows tasks are only run if linux passes tests" could live there.
I don't really see the advantage of doing that over just increasing
concurrency by a bit.
> > + # no options enabled, should be small
> > + CCACHE_MAXSIZE: "150M"
>
> Actually, tasks can share caches if the "cache key" is set.
> If there was a separate "Sanity" task, I think it should use whatever
> flags linux (or freebsd) use to avoid doing two compilations (with lots
> of cache misses for patches which modify *.h files, which would then
> happen twice, in serial).
I think the price of using exactly the same flags is higher than the gain. And
it'll rarely work if we use the container task for the sanity check, as the
timestamps of the compiler, system headers etc will be different.
> > + # use always: to continue after failures. Task that did not run count as a
> > + # success, so we need to recheck SanityChecks's condition here ...
>
> > - # task that did not run, count as a success, so we need to recheck Linux'
> > - # condition here ...
>
> Another/better justification/description is that "cirrus warns if the
> depending task has different only_if conditions than the dependant task".
That doesn't really seem easier to understand to me.
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 43+ messages in thread
* Re: CI and test improvements
@ 2022-10-02 21:54 Andres Freund <[email protected]>
parent: Justin Pryzby <[email protected]>
1 sibling, 2 replies; 43+ messages in thread
From: Andres Freund @ 2022-10-02 21:54 UTC (permalink / raw)
To: Justin Pryzby <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; Thomas Munro <[email protected]>; [email protected]; Noah Misch <[email protected]>; Michael Paquier <[email protected]>; Anastasia Lubennikova <[email protected]>; Tom Lane <[email protected]>; Robert Haas <[email protected]>; Melanie Plageman <[email protected]>; Peter Eisentraut <[email protected]>; Daniel Gustafsson <[email protected]>
Hi,
On 2022-10-02 16:35:06 -0500, Justin Pryzby wrote:
> On Sun, Oct 02, 2022 at 01:52:01PM -0700, Andres Freund wrote:
> > On 2022-10-01 18:36:41 -0700, Andres Freund wrote:
> > > I am wondering if we should instead introduce a new "quickcheck" task that
> > > just compiles and runs maybe one test and have *all* other tests depend on
> > > that. Wasting a precious available windows instance to just fail to build or
> > > immediately fail during tests doesn't really make sense.
>
> > With a primed cache this takes ~32s, not too bad imo. 12s of that is
> > cloning the repo.
>
> Maybe - that would avoid waiting 4 minutes for a windows instance to
> start in the (hopefully atypical) case of a patch that fails in 1-2
> minutes under linux/freebsd.
>
> If the patch were completely broken, the windows task would take ~4min
> to start, plus up to ~4min before failing to compile or failing an early
> test. 6-8 minutes isn't nothing, but doesn't seem worth the added
> complexity.
Btw, the motivation to work on this just now was that I'd like to enable more
sanitizers (undefined,alignment for linux-meson, address for
linux-autoconf). Yes, we could make the dependency on freebsd instead, but I'd
like to try to enable the clang-only memory sanitizer there (if it works on
freebsd)...
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 43+ messages in thread
* Re: CI and test improvements
@ 2022-10-03 02:15 Justin Pryzby <[email protected]>
parent: Andres Freund <[email protected]>
1 sibling, 0 replies; 43+ messages in thread
From: Justin Pryzby @ 2022-10-03 02:15 UTC (permalink / raw)
To: Andres Freund <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; Thomas Munro <[email protected]>; [email protected]; Noah Misch <[email protected]>; Michael Paquier <[email protected]>; Anastasia Lubennikova <[email protected]>; Tom Lane <[email protected]>; Robert Haas <[email protected]>; Melanie Plageman <[email protected]>; Peter Eisentraut <[email protected]>; Daniel Gustafsson <[email protected]>
On Sun, Oct 02, 2022 at 02:54:21PM -0700, Andres Freund wrote:
> the clang-only memory sanitizer there (if it works on freebsd)...
Have you looked at this much ? I think it'll require a bunch of
exclusions, right ?
--
Justin
^ permalink raw reply [nested|flat] 43+ messages in thread
* Re: CI and test improvements
@ 2022-11-04 23:54 Justin Pryzby <[email protected]>
parent: Justin Pryzby <[email protected]>
1 sibling, 1 reply; 43+ messages in thread
From: Justin Pryzby @ 2022-11-04 23:54 UTC (permalink / raw)
To: Andres Freund <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; Thomas Munro <[email protected]>; [email protected]; Noah Misch <[email protected]>; Michael Paquier <[email protected]>; Anastasia Lubennikova <[email protected]>; Tom Lane <[email protected]>; Robert Haas <[email protected]>; Melanie Plageman <[email protected]>; Peter Eisentraut <[email protected]>; Daniel Gustafsson <[email protected]>; samay sharma <[email protected]>
On Sat, Sep 10, 2022 at 03:05:42PM -0500, Justin Pryzby wrote:
> On Sun, Aug 28, 2022 at 02:28:02PM -0700, Andres Freund wrote:
> > On 2022-08-28 12:10:29 -0500, Justin Pryzby wrote:
> > > On Sun, Aug 28, 2022 at 09:07:52AM -0700, Andres Freund wrote:
> > > > > --- /dev/null
> > > > > +++ b/src/tools/ci/windows-compiler-warnings
> > > >
> > > > Wouldn't that be doable as something like
> > > > sh -c 'if test -s file; then cat file;exit 1; fi"
> > > > inside .cirrus.yml?
> > >
> > > I had written it inline in a couple ways, like
> > > - sh -exc 'f=msbuild.warn.log; if [ -s "$f" ]; then cat "$f"; exit 1; else exit 0; fi'
> > >
> > > but then separated it out as you suggested in
> > > [email protected]
> > >
> > > after I complained about cmd.exe requiring escaping for && and ||
> > > That makes writing any shell script a bit perilous and a separate script
> > > seems better.
> >
> > I remember that I suggested it - but note that the way I wrote above doesn't
> > have anything needing escaping.
>
> It doesn't require it, but that still gives the impression that it's
> normally possible to write one-liner shell scripts there, which is
> misleading/wrong, and the reason why I took your suggestion to use a
> separate script file.
>
> > Anyway, what do you think of the multiline split I suggested?
>
> Done, and sorted.
Rewrote this and rebased some of the other stuff on top of the meson
commit, for which I also include some new patches.
^ permalink raw reply [nested|flat] 43+ messages in thread
* Re: CI and test improvements
@ 2022-11-05 01:59 Andres Freund <[email protected]>
parent: Justin Pryzby <[email protected]>
0 siblings, 1 reply; 43+ messages in thread
From: Andres Freund @ 2022-11-05 01:59 UTC (permalink / raw)
To: Justin Pryzby <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; Thomas Munro <[email protected]>; [email protected]; Noah Misch <[email protected]>; Michael Paquier <[email protected]>; Anastasia Lubennikova <[email protected]>; Tom Lane <[email protected]>; Robert Haas <[email protected]>; Melanie Plageman <[email protected]>; Peter Eisentraut <[email protected]>; Daniel Gustafsson <[email protected]>; samay sharma <[email protected]>
Hi,
On 2022-11-04 18:54:12 -0500, Justin Pryzby wrote:
> Subject: [PATCH 1/8] meson: PROVE is not required
> Subject: [PATCH 3/8] meson: rename 'main' tasks to 'regress' and 'isolation'
Pushed, thanks for the patches.
> Subject: [PATCH 2/8] meson: other fixes for cygwin
>
> XXX: what about HAVE_BUGGY_STRTOF ?
What are you wondering about here? Shouldn't that continue to be set via
src/include/port/cygwin.h?
> diff --git a/src/test/regress/meson.build b/src/test/regress/meson.build
> index 3dcfc11278f..6ec3c77af53 100644
> --- a/src/test/regress/meson.build
> +++ b/src/test/regress/meson.build
> @@ -10,7 +10,7 @@ regress_sources = pg_regress_c + files(
> # patterns like ".*-.*-mingw.*". We probably can do better, but for now just
> # replace 'gcc' with 'mingw' on windows.
> host_tuple_cc = cc.get_id()
> -if host_system == 'windows' and host_tuple_cc == 'gcc'
> +if host_system in ['windows', 'cygwin'] and host_tuple_cc == 'gcc'
> host_tuple_cc = 'mingw'
> endif
This doesn't quite seem right - shouldn't it say cywin? Not that it makes a
difference right now, given the contents of resultmap:
float4:out:.*-.*-cygwin.*=float4-misrounded-input.out
float4:out:.*-.*-mingw.*=float4-misrounded-input.out
> From 0acbbd2fdd97bbafc5c4552e26f92d52c597eea9 Mon Sep 17 00:00:00 2001
> From: Justin Pryzby <[email protected]>
> Date: Wed, 25 May 2022 21:53:22 -0500
> Subject: [PATCH 4/8] cirrus/windows: add compiler_warnings_script
>
> I'm not sure how to write this test in windows shell; it's also not easy to
> write it in posix sh, since windows shell is somehow interpretting && and ||...
>
> https://www.postgresql.org/message-id/20220212212310.f645c6vw3njkgxka%40alap3.anarazel.de
>
> See also:
> 8a1ce5e54f6d144e4f8e19af7c767b026ee0c956
> https://cirrus-ci.com/task/6241060062494720
> https://cirrus-ci.com/task/6496366607204352
>
> ci-os-only: windows
> ---
> .cirrus.yml | 10 +++++++++-
> src/tools/ci/windows-compiler-warnings | 24 ++++++++++++++++++++++++
> 2 files changed, 33 insertions(+), 1 deletion(-)
> create mode 100755 src/tools/ci/windows-compiler-warnings
>
> diff --git a/.cirrus.yml b/.cirrus.yml
> index 9f2282471a9..99ac09dc679 100644
> --- a/.cirrus.yml
> +++ b/.cirrus.yml
> @@ -451,12 +451,20 @@ task:
>
> build_script: |
> vcvarsall x64
> - ninja -C build
> + ninja -C build |tee build/meson-logs/build.txt
> + REM Since pipes lose exit status of the preceding command, rerun compilation,
> + REM without the pipe exiting now if it fails, rather than trying to run checks
> + ninja -C build > nul
This seems mighty grotty :(. but I guess it's quick enough not worry about,
and I can't come up with a better plan.
It doesn't seem quite right to redirect into meson-logs/ to me, my
interpretation is that that's "meson's namespace". Why not just store it in
build/?
> From e85fe83fd8a4b4c79a96d2bf66cd6a5e1bdfcd1e Mon Sep 17 00:00:00 2001
> From: Justin Pryzby <[email protected]>
> Date: Sat, 26 Feb 2022 19:34:35 -0600
> Subject: [PATCH 5/8] cirrus: build docs as a separate task..
>
> This will run the doc build if any docs have changed, even if Linux
> fails, to allow catch doc build failures.
>
> This'll automatically show up as a separate "column" on cfbot.
>
> Also, in the future, this will hopefully upload each patch's changed HTML docs
> as an artifact, for easy review.
>
> Note that this is currently building docs with both autoconf and meson.
>
> ci-os-only: html
> ---
> .cirrus.yml | 62 +++++++++++++++++++++++++++++++++++++----------------
> 1 file changed, 44 insertions(+), 18 deletions(-)
>
> diff --git a/.cirrus.yml b/.cirrus.yml
> index 99ac09dc679..37fd79e5b77 100644
> --- a/.cirrus.yml
> +++ b/.cirrus.yml
> @@ -472,6 +472,9 @@ task:
> type: text/plain
>
>
> +###
> +# Test that code can be built with gcc/clang without warnings
> +###
> task:
> name: CompilerWarnings
>
> @@ -515,10 +518,6 @@ task:
> #apt-get update
> #DEBIAN_FRONTEND=noninteractive apt-get -y install ...
>
> - ###
> - # Test that code can be built with gcc/clang without warnings
> - ###
> -
Why remove this?
> setup_script: echo "COPT=-Werror" > src/Makefile.custom
>
> # Trace probes have a history of getting accidentally broken. Use the
> @@ -580,20 +579,6 @@ task:
> make -s -j${BUILD_JOBS} clean
> time make -s -j${BUILD_JOBS} world-bin
>
> - ###
> - # Verify docs can be built
> - ###
> - # XXX: Only do this if there have been changes in doc/ since last build
> - always:
> - docs_build_script: |
> - time ./configure \
> - --cache gcc.cache \
> - CC="ccache gcc" \
> - CXX="ccache g++" \
> - CLANG="ccache clang"
> - make -s -j${BUILD_JOBS} clean
> - time make -s -j${BUILD_JOBS} -C doc
> -
> ###
> # Verify headerscheck / cpluspluscheck succeed
> #
> @@ -617,3 +602,44 @@ task:
>
> always:
> upload_caches: ccache
> +
> +
> +###
> +# Verify docs can be built
> +# changesInclude() will skip this task if none of the commits since
> +# CIRRUS_LAST_GREEN_CHANGE touched any relevant files. The comparison appears
> +# to be like "git log a..b -- ./file", not "git diff a..b -- ./file"
> +###
> +
> +task:
> + name: Documentation
> +
> + env:
> + CPUS: 1
> + BUILD_JOBS: 1
> +
> + only_if: $CIRRUS_CHANGE_MESSAGE !=~ '.*\nci-os-only:.*' || $CIRRUS_CHANGE_MESSAGE =~ '.*\nci-os-only:[^\n]*(docs|html).*'
> + skip: "!changesInclude('.cirrus.yml', 'doc/**')"
Perhaps we should introduce something other than ci-os-only if we want that to
include things like "docs and html". At least this should update
src/tools/ci/README.
> + sysinfo_script: |
> + id
> + uname -a
> + cat /proc/cmdline
> + ulimit -a -H && ulimit -a -S
> + export
I think we can skip this here.
> + # Exercise HTML and other docs:
> + ninja_docs_build_script: |
> + mkdir build.ninja
> + cd build.ninja
Perhaps build-ninja instead? build.ninja is the filename for ninja's build
instructions, so it seems a bit confusing.
> From adebe93a4409990e929f2775d45c6613134a4243 Mon Sep 17 00:00:00 2001
> From: Justin Pryzby <[email protected]>
> Date: Tue, 26 Jul 2022 20:30:02 -0500
> Subject: [PATCH 6/8] cirrus/ccache: add explicit cache keys..
>
> Since otherwise, building with ci-os-only will probably fail to use the
> normal cache, since the cache key is computed using both the task name
> and its *index* in the list of caches (internal/executor/cache.go:184).
Seems like this would potentially better addressed by reporting a bug to the
cirrus folks?
> ccache_cache:
> folder: ${CCACHE_DIR}
> + fingerprint_key: ccache/linux
> + reupload_on_changes: true
There's enough copies of this to make it worth deduplicating. If we use
something like
fingerprint_script: echo ccache/$CIRRUS_BRANCH/$CIRRUS_OS
we can use a yaml ref?
I think you experimented with creating a 'base' ccache dir (e.g. on the master
branch) and then using branch specific secondar caches? How did that turn out?
I think cfbot's caches constantly get removed due to overrunning the global
space.
> From f16739bc5d2087847129baf663aa25fa9edb8449 Mon Sep 17 00:00:00 2001
> From: Justin Pryzby <[email protected]>
> Date: Sun, 3 Apr 2022 00:10:20 -0500
> Subject: [PATCH 7/8] cirrus/ccache: disable compression and show stats
> 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
I'm still somewhat doubtful this is a good idea. The mingw cache is huge, for
example, and all that additional IO and memory usage is bound to show up.
> 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.
I assume we don't have a new enough ccache everywhere yet?
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 43+ messages in thread
* Re: CI and test improvements
@ 2022-11-13 23:53 Justin Pryzby <[email protected]>
parent: Andres Freund <[email protected]>
0 siblings, 1 reply; 43+ messages in thread
From: Justin Pryzby @ 2022-11-13 23:53 UTC (permalink / raw)
To: Andres Freund <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; Thomas Munro <[email protected]>; [email protected]; Noah Misch <[email protected]>; Michael Paquier <[email protected]>; Anastasia Lubennikova <[email protected]>; Tom Lane <[email protected]>; Robert Haas <[email protected]>; Melanie Plageman <[email protected]>; Peter Eisentraut <[email protected]>; Daniel Gustafsson <[email protected]>; samay sharma <[email protected]>
On Fri, Nov 04, 2022 at 06:59:46PM -0700, Andres Freund wrote:
> Hi,
>
> On 2022-11-04 18:54:12 -0500, Justin Pryzby wrote:
> > Subject: [PATCH 1/8] meson: PROVE is not required
> > Subject: [PATCH 3/8] meson: rename 'main' tasks to 'regress' and 'isolation'
>
> Pushed, thanks for the patches.
Thanks.
> > diff --git a/.cirrus.yml b/.cirrus.yml
> > index 9f2282471a9..99ac09dc679 100644
> > --- a/.cirrus.yml
> > +++ b/.cirrus.yml
> > @@ -451,12 +451,20 @@ task:
> >
> > build_script: |
> > vcvarsall x64
> > - ninja -C build
> > + ninja -C build |tee build/meson-logs/build.txt
> > + REM Since pipes lose exit status of the preceding command, rerun compilation,
> > + REM without the pipe exiting now if it fails, rather than trying to run checks
> > + ninja -C build > nul
>
> This seems mighty grotty :(. but I guess it's quick enough not worry about,
> and I can't come up with a better plan.
>
> It doesn't seem quite right to redirect into meson-logs/ to me, my
> interpretation is that that's "meson's namespace". Why not just store it in
> build/?
I put it there so it'd be included with the build artifacts.
Maybe it's worth adding a separate line to artifacts for stuff like
this, and ccache log ?
> > From e85fe83fd8a4b4c79a96d2bf66cd6a5e1bdfcd1e Mon Sep 17 00:00:00 2001
> > From: Justin Pryzby <[email protected]>
> > Date: Sat, 26 Feb 2022 19:34:35 -0600
> > Subject: [PATCH 5/8] cirrus: build docs as a separate task..
> > + # Exercise HTML and other docs:
> > + ninja_docs_build_script: |
> > + mkdir build.ninja
> > + cd build.ninja
>
> Perhaps build-ninja instead? build.ninja is the filename for ninja's build
> instructions, so it seems a bit confusing.
Sure.
Do you think building docs with both autoconf and meson is what's
desirable here ?
I'm not sure if this ought to be combined with/before/after your "move
compilerwarnings task to meson" patch? (Regarding that patch: I
mentioned that it shouldn't use ccache -C, and it should use
meson_log_artifacts.)
> > From: Justin Pryzby <[email protected]>
> > Date: Tue, 26 Jul 2022 20:30:02 -0500
> > Subject: [PATCH 6/8] cirrus/ccache: add explicit cache keys..
> >
> > Since otherwise, building with ci-os-only will probably fail to use the
> > normal cache, since the cache key is computed using both the task name
> > and its *index* in the list of caches (internal/executor/cache.go:184).
>
> Seems like this would potentially better addressed by reporting a bug to the
> cirrus folks?
You said that before, but I don't think so - since they wrote code to do
that, it's odd to file a bug that says that the behavior is wrong. I am
curious why, but it seems delibrate.
https://www.postgresql.org/message-id/20220828171029.GO2342%40telsasoft.com
> There's enough copies of this to make it worth deduplicating. If we
> use something like fingerprint_script: echo
> ccache/$CIRRUS_BRANCH/$CIRRUS_OS we can use a yaml ref?
I'll look into it...
> I think you experimented with creating a 'base' ccache dir (e.g. on the master
> branch) and then using branch specific secondar caches?
I have to revisit that sometime.
That's a new feaure in ccache 4.4, which is currently only in macos.
This is another thing that it'd be easier to test by having cfbot
clobber the cirrus.yml rather than committing to postgres repo.
(Technically, it should probably only do use the in-testing cirrus.yml
if the patch it's testing doesn't itself modify .cirrus.yml)
> How did that turn out? I think cfbot's caches constantly get removed
> due to overrunning the global space.
For cfbot, I don't know if there's much hope that any patch-specific
build artifacts will be cached from the previous run, typically ~24h
prior.
One idea I have, for the "Warnings" task (and maybe linux too), is to
defer pruning until after all the compilations. To avoid LRU pruning
during early tasks causing bad hit ratios of later tasks.
Another thing that probably happens is that task1 starts compiling
patch1, and then another instance of task1 starts compiling patch2. A
bit later, the first instance will upload its ccache result for patch1,
which will be summarily overwritten by the second instance's compilation
result, which doesn't include anything from the first instance.
Also, whenever ccache hits its MAXSIZE threshold, it prunes the cache
down to 80% of the configured size, which probably wipes away everything
from all but the most recent ~20 builds.
I also thought about having separate caches for each compilation in the
warnings task - but that requires too much repeated yaml just for that..
> > From: Justin Pryzby <[email protected]>
> > Date: Sun, 3 Apr 2022 00:10:20 -0500
> > Subject: [PATCH 7/8] cirrus/ccache: disable compression and show stats
> >
> > 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
>
> I'm still somewhat doubtful this is a good idea. The mingw cache is huge, for
> example, and all that additional IO and memory usage is bound to show up.
I think you're referring to the proposed mingw task which runs under
windows, and not the existing cross-compilation ?
And you're right - I remember this now (I think it's due to PCH?)
In my local copy I'd "unset CCACHE_NOCOMPRESS". But I view that as an
oddity of windows headers, rather than an argument against disabling
compression elsewhere. BTW, freebsd ccache is too old to use
compression.
What about using CCACHE_HARDLINK (which implies no compression) ?
> > 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.
>
> I assume we don't have a new enough ccache everywhere yet?
No - see above.
I've added patches to update macos.
--
Justin
^ permalink raw reply [nested|flat] 43+ messages in thread
* Re: CI and test improvements
@ 2022-11-17 03:48 Andres Freund <[email protected]>
parent: Andres Freund <[email protected]>
1 sibling, 1 reply; 43+ messages in thread
From: Andres Freund @ 2022-11-17 03:48 UTC (permalink / raw)
To: Justin Pryzby <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; Thomas Munro <[email protected]>; [email protected]; Noah Misch <[email protected]>; Michael Paquier <[email protected]>; Anastasia Lubennikova <[email protected]>; Tom Lane <[email protected]>; Robert Haas <[email protected]>; Melanie Plageman <[email protected]>; Peter Eisentraut <[email protected]>; Daniel Gustafsson <[email protected]>
Hi,
On 2022-10-02 14:54:21 -0700, Andres Freund wrote:
> On 2022-10-02 16:35:06 -0500, Justin Pryzby wrote:
> > On Sun, Oct 02, 2022 at 01:52:01PM -0700, Andres Freund wrote:
> > > On 2022-10-01 18:36:41 -0700, Andres Freund wrote:
> > > > I am wondering if we should instead introduce a new "quickcheck" task that
> > > > just compiles and runs maybe one test and have *all* other tests depend on
> > > > that. Wasting a precious available windows instance to just fail to build or
> > > > immediately fail during tests doesn't really make sense.
> >
> > > With a primed cache this takes ~32s, not too bad imo. 12s of that is
> > > cloning the repo.
> >
> > Maybe - that would avoid waiting 4 minutes for a windows instance to
> > start in the (hopefully atypical) case of a patch that fails in 1-2
> > minutes under linux/freebsd.
> >
> > If the patch were completely broken, the windows task would take ~4min
> > to start, plus up to ~4min before failing to compile or failing an early
> > test. 6-8 minutes isn't nothing, but doesn't seem worth the added
> > complexity.
>
> Btw, the motivation to work on this just now was that I'd like to enable more
> sanitizers (undefined,alignment for linux-meson, address for
> linux-autoconf). Yes, we could make the dependency on freebsd instead, but I'd
> like to try to enable the clang-only memory sanitizer there (if it works on
> freebsd)...
I've used this a bunch on personal branches, and I think it's the way to
go. It doesn't take long, saves a lot of cycles when one pushes something
broken. Starts to runs the CompilerWarnings task after a minimal amount of
sanity checking, instead of having to wait for a task running all tests,
without the waste of running it immediately and failing all the different
configurations, which takes forever.
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 43+ messages in thread
* Re: CI and test improvements
@ 2022-11-17 03:58 Justin Pryzby <[email protected]>
parent: Andres Freund <[email protected]>
0 siblings, 1 reply; 43+ messages in thread
From: Justin Pryzby @ 2022-11-17 03:58 UTC (permalink / raw)
To: Andres Freund <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; Thomas Munro <[email protected]>; [email protected]; Noah Misch <[email protected]>; Michael Paquier <[email protected]>; Anastasia Lubennikova <[email protected]>; Tom Lane <[email protected]>; Robert Haas <[email protected]>; Melanie Plageman <[email protected]>; Peter Eisentraut <[email protected]>; Daniel Gustafsson <[email protected]>
On Wed, Nov 16, 2022 at 07:48:14PM -0800, Andres Freund wrote:
> I've used this a bunch on personal branches, and I think it's the way to
> go. It doesn't take long, saves a lot of cycles when one pushes something
> broken. Starts to runs the CompilerWarnings task after a minimal amount of
> sanity checking, instead of having to wait for a task running all tests,
> without the waste of running it immediately and failing all the different
> configurations, which takes forever.
Well, I don't hate it.
But I don't think you should call "ccache -z":
On Tue, Oct 18, 2022 at 12:09:30PM -0500, Justin Pryzby wrote:
> I realized that ccache -z 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.
--
Justin
^ permalink raw reply [nested|flat] 43+ messages in thread
* Re: CI and test improvements
@ 2022-11-17 04:08 Andres Freund <[email protected]>
parent: Justin Pryzby <[email protected]>
0 siblings, 1 reply; 43+ messages in thread
From: Andres Freund @ 2022-11-17 04:08 UTC (permalink / raw)
To: Justin Pryzby <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; Thomas Munro <[email protected]>; [email protected]; Noah Misch <[email protected]>; Michael Paquier <[email protected]>; Anastasia Lubennikova <[email protected]>; Tom Lane <[email protected]>; Robert Haas <[email protected]>; Melanie Plageman <[email protected]>; Peter Eisentraut <[email protected]>; Daniel Gustafsson <[email protected]>
Hi,
On 2022-11-16 21:58:39 -0600, Justin Pryzby wrote:
> On Wed, Nov 16, 2022 at 07:48:14PM -0800, Andres Freund wrote:
> > I've used this a bunch on personal branches, and I think it's the way to
> > go. It doesn't take long, saves a lot of cycles when one pushes something
> > broken. Starts to runs the CompilerWarnings task after a minimal amount of
> > sanity checking, instead of having to wait for a task running all tests,
> > without the waste of running it immediately and failing all the different
> > configurations, which takes forever.
>
> Well, I don't hate it.
>
> But I don't think you should call "ccache -z":
Agreed - that was really just for "development" of the task.
I also don't like my "cores_script". Not quite sure yet how to do that
more cleanly.
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 43+ messages in thread
* Re: CI and test improvements
@ 2022-11-17 04:16 Justin Pryzby <[email protected]>
parent: Andres Freund <[email protected]>
0 siblings, 0 replies; 43+ messages in thread
From: Justin Pryzby @ 2022-11-17 04:16 UTC (permalink / raw)
To: Andres Freund <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; Thomas Munro <[email protected]>; [email protected]; Noah Misch <[email protected]>; Michael Paquier <[email protected]>; Anastasia Lubennikova <[email protected]>; Tom Lane <[email protected]>; Robert Haas <[email protected]>; Melanie Plageman <[email protected]>; Peter Eisentraut <[email protected]>; Daniel Gustafsson <[email protected]>
On Wed, Nov 16, 2022 at 08:08:32PM -0800, Andres Freund wrote:
> I also don't like my "cores_script". Not quite sure yet how to do that
> more cleanly.
I don't know which is cleaner:
ls /core* && mv /tmp/core* /tmp/cores
find / -maxdepth 1 -type f -name 'core*' -print0 |
xargs -r0 mv -vt /tmp/cores
for a in /core*; do [ ! -e "$a" ] || mv "$a" /tmp/cores; done
^ permalink raw reply [nested|flat] 43+ messages in thread
* Re: CI and test improvements
@ 2022-11-19 20:22 Justin Pryzby <[email protected]>
parent: Andres Freund <[email protected]>
1 sibling, 1 reply; 43+ messages in thread
From: Justin Pryzby @ 2022-11-19 20:22 UTC (permalink / raw)
To: Andres Freund <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; Thomas Munro <[email protected]>; [email protected]; Noah Misch <[email protected]>; Michael Paquier <[email protected]>; Anastasia Lubennikova <[email protected]>; Tom Lane <[email protected]>; Robert Haas <[email protected]>; Melanie Plageman <[email protected]>; Peter Eisentraut <[email protected]>; Daniel Gustafsson <[email protected]>
On Sun, Oct 02, 2022 at 01:52:01PM -0700, Andres Freund wrote:
>
> +# To avoid unnecessarily spinning up a lot of VMs / containers for entirely
> +# broken commits, have a very minimal test that all others depend on.
> +task:
> + name: SanityCheck
Maybe this should be named 00-SanityCheck, so it sorts first in cfbot ?
Also, if CompilerWarnings doesn't depend on Linux, that means those two
tasks will normally start and run simultaneously, which means a single
branch will use all 8 of the linux CPUs available from cirrus. Is that
intentional?
--
Justin
^ permalink raw reply [nested|flat] 43+ messages in thread
* Re: CI and test improvements
@ 2022-11-19 21:18 Andres Freund <[email protected]>
parent: Justin Pryzby <[email protected]>
0 siblings, 3 replies; 43+ messages in thread
From: Andres Freund @ 2022-11-19 21:18 UTC (permalink / raw)
To: Justin Pryzby <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; Thomas Munro <[email protected]>; [email protected]; Noah Misch <[email protected]>; Michael Paquier <[email protected]>; Anastasia Lubennikova <[email protected]>; Tom Lane <[email protected]>; Robert Haas <[email protected]>; Melanie Plageman <[email protected]>; Peter Eisentraut <[email protected]>; Daniel Gustafsson <[email protected]>
Hi,
On 2022-11-19 14:22:20 -0600, Justin Pryzby wrote:
> On Sun, Oct 02, 2022 at 01:52:01PM -0700, Andres Freund wrote:
> >
> > +# To avoid unnecessarily spinning up a lot of VMs / containers for entirely
> > +# broken commits, have a very minimal test that all others depend on.
> > +task:
> > + name: SanityCheck
>
> Maybe this should be named 00-SanityCheck, so it sorts first in cfbot ?
Hm. Perhaps cfbot could just use the sorting from cirrus? I don't really like
the idea of making the names more confusing with numbered prefixes,
particularly when only used for some but not other tasks.
> Also, if CompilerWarnings doesn't depend on Linux, that means those two
> tasks will normally start and run simultaneously, which means a single
> branch will use all 8 of the linux CPUs available from cirrus. Is that
> intentional?
I don't think that'd really make anything worse. But perhaps we could just
reduce the CPU count for linux autoconf by 1? I suspect that even with asan
enabled it'd still be roughly even with the rest.
I'll try to repost a version of the ubsan/asan patch together with the
sanitycheck patch and see how that looks.
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 43+ messages in thread
* Re: CI and test improvements
@ 2022-11-19 21:35 Andres Freund <[email protected]>
parent: Andres Freund <[email protected]>
2 siblings, 0 replies; 43+ messages in thread
From: Andres Freund @ 2022-11-19 21:35 UTC (permalink / raw)
To: Justin Pryzby <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; Thomas Munro <[email protected]>; [email protected]; Noah Misch <[email protected]>; Michael Paquier <[email protected]>; Anastasia Lubennikova <[email protected]>; Tom Lane <[email protected]>; Robert Haas <[email protected]>; Melanie Plageman <[email protected]>; Peter Eisentraut <[email protected]>; Daniel Gustafsson <[email protected]>
Hi,
On 2022-11-19 13:18:54 -0800, Andres Freund wrote:
> > Also, if CompilerWarnings doesn't depend on Linux, that means those two
> > tasks will normally start and run simultaneously, which means a single
> > branch will use all 8 of the linux CPUs available from cirrus. Is that
> > intentional?
>
> I don't think that'd really make anything worse. But perhaps we could just
> reduce the CPU count for linux autoconf by 1? I suspect that even with asan
> enabled it'd still be roughly even with the rest.
Hm, that doesn't suffice, because we allow 4 cores for the warnings task. The
limit for cirrus is 16 linux CPUs though, not 8. We'll temporarily go up to 12
due to CompilerWarnings after the change. But I think that's fine, because
we'd previously use the same amount of CPUs, just some of it
sequentially.
^ permalink raw reply [nested|flat] 43+ messages in thread
* Re: CI and test improvements
@ 2022-11-19 21:45 Justin Pryzby <[email protected]>
parent: Andres Freund <[email protected]>
2 siblings, 1 reply; 43+ messages in thread
From: Justin Pryzby @ 2022-11-19 21:45 UTC (permalink / raw)
To: Andres Freund <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; Thomas Munro <[email protected]>; [email protected]; Noah Misch <[email protected]>; Michael Paquier <[email protected]>; Anastasia Lubennikova <[email protected]>; Tom Lane <[email protected]>; Robert Haas <[email protected]>; Melanie Plageman <[email protected]>; Peter Eisentraut <[email protected]>; Daniel Gustafsson <[email protected]>
On Sat, Nov 19, 2022 at 01:18:54PM -0800, Andres Freund wrote:
> > Also, if CompilerWarnings doesn't depend on Linux, that means those two
> > tasks will normally start and run simultaneously, which means a single
> > branch will use all 8 of the linux CPUs available from cirrus. Is that
> > intentional?
>
> I don't think that'd really make anything worse. But perhaps we could just
> reduce the CPU count for linux autoconf by 1?
I didn't understand the goal of "reducing by one" ?
Up to now, most tasks are using half of the available CPUs, which seemed
deliberate. Like maybe to allow running two branches simultaneously
(that doesn't necessarily work well with ccache, though).
On Sat, Nov 19, 2022 at 01:35:17PM -0800, Andres Freund wrote:
> The limit for cirrus is 16 linux CPUs though, not 8.
Oh. Then I don't see any issue.
> We'll temporarily go up to 12 due to CompilerWarnings after the change.
What do you mean "temporarily" ? I think you're implying that the
Warnings task is fast but (at least right now) it is not.
Note that the most recent "code coverage" task is built into the
linux-autoconf task, and slows it down some more. That's because it's
the only remaining in-tree build, and I aimed to only show coverage for
changed files (I know you questioned whether that was okay, but to me it
still seems to be valuable, even though it obviously doesn't show
changes outside of those files). And I couldn't see how to map from
"object filename to source file" with meson, although I guess it's
possible with instrospection. I haven't re-sent that patch because it's
waiting on cfbot changes.
--
Justin
^ permalink raw reply [nested|flat] 43+ messages in thread
* Re: CI and test improvements
@ 2022-11-19 22:14 Andres Freund <[email protected]>
parent: Justin Pryzby <[email protected]>
0 siblings, 0 replies; 43+ messages in thread
From: Andres Freund @ 2022-11-19 22:14 UTC (permalink / raw)
To: Justin Pryzby <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; Thomas Munro <[email protected]>; [email protected]; Noah Misch <[email protected]>; Michael Paquier <[email protected]>; Anastasia Lubennikova <[email protected]>; Tom Lane <[email protected]>; Robert Haas <[email protected]>; Melanie Plageman <[email protected]>; Peter Eisentraut <[email protected]>; Daniel Gustafsson <[email protected]>
Hi,
On 2022-11-19 15:45:06 -0600, Justin Pryzby wrote:
> What do you mean "temporarily" ? I think you're implying that the
> Warnings task is fast but (at least right now) it is not.
In the sense that we don't need all CPUs until the whole commit has finished
testing (none of the tasks are the slowest task, even after ubsan/asan). As
soon as one of the linux tests has finished for one commit, another task in a
concurrently tested commit can start. Whereas that's not the case for macos,
due to the VM limit.
(cfbot has double the limits, because it has a 10$/mo account)
> Note that the most recent "code coverage" task is built into the
> linux-autoconf task, and slows it down some more. That's because it's
> the only remaining in-tree build, and I aimed to only show coverage for
> changed files (I know you questioned whether that was okay, but to me it
> still seems to be valuable, even though it obviously doesn't show
> changes outside of those files).
I think we shouldn't add further tests using autoconf, that'll just mean we'll
have to do the work changing that test at some later point.
> And I couldn't see how to map from
> "object filename to source file" with meson, although I guess it's
> possible with instrospection. I haven't re-sent that patch because it's
> waiting on cfbot changes.
The object files should have that in their metadata, fwiw.
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 43+ messages in thread
* Re: CI and test improvements
@ 2022-11-21 22:09 Andres Freund <[email protected]>
parent: Andres Freund <[email protected]>
2 siblings, 0 replies; 43+ messages in thread
From: Andres Freund @ 2022-11-21 22:09 UTC (permalink / raw)
To: Justin Pryzby <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; Thomas Munro <[email protected]>; [email protected]; Noah Misch <[email protected]>; Michael Paquier <[email protected]>; Anastasia Lubennikova <[email protected]>; Tom Lane <[email protected]>; Robert Haas <[email protected]>; Melanie Plageman <[email protected]>; Peter Eisentraut <[email protected]>; Daniel Gustafsson <[email protected]>
Hi,
On 2022-11-19 13:18:54 -0800, Andres Freund wrote:
> I'll try to repost a version of the ubsan/asan patch together with the
> sanitycheck patch and see how that looks.
I just pushed the prerequisite patch making UBSAN_OPTIONS work. Attached
is 1) addition of SanityCheck 2) use of asan and ubsan+alignment san to
the linux tasks.
I went with a variation of the find command for SanityCheck's
cores_script, but used -exec to invoke mv, as that results in a nicer
looking commandline imo.
Previously the SanityCheck patch did trigger warnings about only_if not
matching, despite SanityCheck not having an only_if, but I reported that
as a bug to cirrus-ci, and they fixed that.
Pretty happy with this.
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 43+ messages in thread
* Re: CI and test improvements
@ 2022-11-21 22:45 Andres Freund <[email protected]>
parent: Justin Pryzby <[email protected]>
0 siblings, 1 reply; 43+ messages in thread
From: Andres Freund @ 2022-11-21 22:45 UTC (permalink / raw)
To: Justin Pryzby <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; Thomas Munro <[email protected]>; [email protected]; Noah Misch <[email protected]>; Michael Paquier <[email protected]>; Anastasia Lubennikova <[email protected]>; Tom Lane <[email protected]>; Robert Haas <[email protected]>; Melanie Plageman <[email protected]>; Peter Eisentraut <[email protected]>; Daniel Gustafsson <[email protected]>; samay sharma <[email protected]>
Hi,
On 2022-11-13 17:53:04 -0600, Justin Pryzby wrote:
> On Fri, Nov 04, 2022 at 06:59:46PM -0700, Andres Freund wrote:
> > > diff --git a/.cirrus.yml b/.cirrus.yml
> > > index 9f2282471a9..99ac09dc679 100644
> > > --- a/.cirrus.yml
> > > +++ b/.cirrus.yml
> > > @@ -451,12 +451,20 @@ task:
> > >
> > > build_script: |
> > > vcvarsall x64
> > > - ninja -C build
> > > + ninja -C build |tee build/meson-logs/build.txt
> > > + REM Since pipes lose exit status of the preceding command, rerun compilation,
> > > + REM without the pipe exiting now if it fails, rather than trying to run checks
> > > + ninja -C build > nul
> >
> > This seems mighty grotty :(. but I guess it's quick enough not worry about,
> > and I can't come up with a better plan.
> >
> > It doesn't seem quite right to redirect into meson-logs/ to me, my
> > interpretation is that that's "meson's namespace". Why not just store it in
> > build/?
>
> I put it there so it'd be included with the build artifacts.
Wouldn't just naming it build-warnings.log suffice? I don't think we
want to actually upload build.txt - it already is captured.
> > > From e85fe83fd8a4b4c79a96d2bf66cd6a5e1bdfcd1e Mon Sep 17 00:00:00 2001
> > > From: Justin Pryzby <[email protected]>
> > > Date: Sat, 26 Feb 2022 19:34:35 -0600
> > > Subject: [PATCH 5/8] cirrus: build docs as a separate task..
>
> > > + # Exercise HTML and other docs:
> > > + ninja_docs_build_script: |
> > > + mkdir build.ninja
> > > + cd build.ninja
> >
> > Perhaps build-ninja instead? build.ninja is the filename for ninja's build
> > instructions, so it seems a bit confusing.
>
> Sure.
>
> Do you think building docs with both autoconf and meson is what's
> desirable here ?
Not sure.
> I'm not sure if this ought to be combined with/before/after your "move
> compilerwarnings task to meson" patch? (Regarding that patch: I
> mentioned that it shouldn't use ccache -C, and it should use
> meson_log_artifacts.)
TBH, I'm not quite sure a separate docs task does really still make
sense after the SanityCheck task. It's worth building the docs even if
some flappy test fails, but I don't think we should try to build the
docs if the code doesn't even compile, in all likelihood a lot more is
wrong in that case.
> > > From: Justin Pryzby <[email protected]>
> > > Date: Tue, 26 Jul 2022 20:30:02 -0500
> > > Subject: [PATCH 6/8] cirrus/ccache: add explicit cache keys..
> > >
> > > Since otherwise, building with ci-os-only will probably fail to use the
> > > normal cache, since the cache key is computed using both the task name
> > > and its *index* in the list of caches (internal/executor/cache.go:184).
> >
> > Seems like this would potentially better addressed by reporting a bug to the
> > cirrus folks?
>
> You said that before, but I don't think so - since they wrote code to do
> that, it's odd to file a bug that says that the behavior is wrong. I am
> curious why, but it seems delibrate.
>
> https://www.postgresql.org/message-id/20220828171029.GO2342%40telsasoft.com
I suspect this is just about dealing with unnamed tasks and could be
handled by just mixing in CI_NODE_INDEX if the task name isn't set.
I pushed a version of 0007-cirrus-clean-up-windows-task.patch. I didn't
rename the task as I would like to add a msbuild version of the task at
some point (it's pretty easy to break msbuild but not ninja
unfortunately). In additional I also removed NO_TEMP_INSTALL.
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 43+ messages in thread
* Re: CI and test improvements
@ 2022-11-22 22:57 Justin Pryzby <[email protected]>
parent: Andres Freund <[email protected]>
0 siblings, 2 replies; 43+ messages in thread
From: Justin Pryzby @ 2022-11-22 22:57 UTC (permalink / raw)
To: Andres Freund <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; Thomas Munro <[email protected]>; [email protected]; Noah Misch <[email protected]>; Michael Paquier <[email protected]>; Anastasia Lubennikova <[email protected]>; Tom Lane <[email protected]>; Robert Haas <[email protected]>; Melanie Plageman <[email protected]>; Peter Eisentraut <[email protected]>; Daniel Gustafsson <[email protected]>; samay sharma <[email protected]>
On Mon, Nov 21, 2022 at 02:45:42PM -0800, Andres Freund wrote:
> > > > + ninja -C build |tee build/meson-logs/build.txt
> > > > + REM Since pipes lose exit status of the preceding command, rerun compilation,
> > > > + REM without the pipe exiting now if it fails, rather than trying to run checks
> > > > + ninja -C build > nul
> > >
> > > This seems mighty grotty :(. but I guess it's quick enough not worry about,
> > > and I can't come up with a better plan.
> > >
> > > It doesn't seem quite right to redirect into meson-logs/ to me, my
> > > interpretation is that that's "meson's namespace". Why not just store it in
> > > build/?
> >
> > I put it there so it'd be included with the build artifacts.
>
> Wouldn't just naming it build-warnings.log suffice? I don't think we
> want to actually upload build.txt - it already is captured.
Originally, I wanted the input and the output to be available as files
and not just in cirrus' web GUI, but maybe that's not important anymore.
I rewrote it again.
> > I'm not sure if this ought to be combined with/before/after your "move
> > compilerwarnings task to meson" patch? (Regarding that patch: I
> > mentioned that it shouldn't use ccache -C, and it should use
> > meson_log_artifacts.)
>
> TBH, I'm not quite sure a separate docs task does really still make
> sense after the SanityCheck task. It's worth building the docs even if
> some flappy test fails, but I don't think we should try to build the
> docs if the code doesn't even compile, in all likelihood a lot more is
> wrong in that case.
It'd be okay either way. I had split it out to 1) isolate the changes
in the "upload changed docs as artifacts" patch; and, 2) so the docs
artifacts are visible in a cfbot link called "Documentation"; and, 3) so
the docs task runs without a dependency on "Linux", since (as you said)
docs/errors are worth showing/reviewing/reporting/addressing separately
from test errors (perhaps similar to compiler warnings...).
I shuffled my branch around and sending now the current "docs" patches,
but I suppose this is waiting on the "convert CompilerWarnings task to
meson" patch.
--
Justin
^ permalink raw reply [nested|flat] 43+ messages in thread
* Re: CI and test improvements
@ 2022-11-26 15:21 Andrew Dunstan <[email protected]>
0 siblings, 0 replies; 43+ messages in thread
From: Andrew Dunstan @ 2022-11-26 15:21 UTC (permalink / raw)
To: pgsql-hackers
[ resending to -hackers because of list issues ]
On 2022-05-28 Sa 11:37, Justin Pryzby wrote:
> I'm "joining" a bunch of unresolved threads hoping to present them better since
> they're related and I'm maintaining them together anyway.
>
> https://www.postgresql.org/message-id/flat/20220219234148.GC9008%40telsasoft.com
> - set TESTDIR from perl rather than Makefile
I looked at the latest set here, patch 1 still doesn't look right, I
think vc_regress.pl should be setting PG_SUBDIR like the Makefile.global
does.
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
^ permalink raw reply [nested|flat] 43+ messages in thread
* Re: CI and test improvements
@ 2022-12-30 03:59 Thomas Munro <[email protected]>
parent: Justin Pryzby <[email protected]>
1 sibling, 0 replies; 43+ messages in thread
From: Thomas Munro @ 2022-12-30 03:59 UTC (permalink / raw)
To: Justin Pryzby <[email protected]>; +Cc: Andres Freund <[email protected]>; Andrew Dunstan <[email protected]>; [email protected]; Noah Misch <[email protected]>; Michael Paquier <[email protected]>; Anastasia Lubennikova <[email protected]>; Tom Lane <[email protected]>; Robert Haas <[email protected]>; Melanie Plageman <[email protected]>; Peter Eisentraut <[email protected]>; Daniel Gustafsson <[email protected]>; samay sharma <[email protected]>
On Wed, Nov 23, 2022 at 11:57 AM Justin Pryzby <[email protected]> wrote:
> [PATCH 02/10] cirrus/macos: switch to "macos_instance" / M1..
Duelling patches.
Bilal's patch[1] uses the matrix feature to run the tests on both
Intel and ARM, which made sense when he proposed it, but according to
Cirrus CI warnings, the Intel instances are about to go away. So I
think we just need your smaller change to switch the instance type.
As for the pathname change, there is another place that knows where
Homebrew lives, in ldap/001_auth. Fixed in the attached. That test
just SKIPs if it can't find the binary, making it harder to notice.
Standardising our approach here might make sense for a later patch.
As for the kerberos test, Bilal's patch may well be a better idea (it
adds MacPorts for one thing), and so I'll suggest rebasing that, but
here I just wanted the minimum mechanical fix to avoid breaking on the
1st of Jan.
I plan to push this soon if there are no objections. Then discussion
of Bilal's patch can continue.
> [PATCH 03/10] cirrus/macos: update to macos ventura
I don't know any reason not to push this one too, but it's not time critical.
[1] https://www.postgresql.org/message-id/flat/CAN55FZ2R%2BXufuVgJ8ew_yDBk48PgXEBvyKNvnNdTTVyczbQj0g%40m...
Attachments:
[text/x-patch] 0001-ci-Change-macOS-builds-from-Intel-to-ARM.patch (3.2K, ../../CA+hUKGL=z6-kU80nPTjWqV=Xo=mpM36Lca=ngTr4LqcCf=tHzQ@mail.gmail.com/2-0001-ci-Change-macOS-builds-from-Intel-to-ARM.patch)
download | inline diff:
From a355638fd9c1c16e54418d8374a374967081e0a0 Mon Sep 17 00:00:00 2001
From: Thomas Munro <[email protected]>
Date: Wed, 28 Dec 2022 16:58:09 +1300
Subject: [PATCH] ci: Change macOS builds from Intel to ARM.
Cirrus is about to shut down its macOS-on-Intel support, so it's time to
move our CI testing over to ARM instances. The Homebrew package manager
changed its default installation prefix for the new architecture, so a
couple of tests need tweaks to find binaries.
Author: Justin Pryzby <[email protected]>
Discussion: https://postgr.es/m/20221122225744.GF11463%40telsasoft.com
---
.cirrus.yml | 8 ++++----
src/test/kerberos/t/001_auth.pl | 9 ++++++++-
src/test/ldap/t/001_auth.pl | 8 +++++++-
3 files changed, 19 insertions(+), 6 deletions(-)
diff --git a/.cirrus.yml b/.cirrus.yml
index 993af88865..354102613d 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -407,7 +407,7 @@ task:
name: macOS - Monterey - Meson
env:
- CPUS: 12 # always get that much for cirrusci macOS instances
+ CPUS: 4 # always get that much for cirrusci macOS instances
BUILD_JOBS: $CPUS
# Test performance regresses noticably when using all cores. 8 seems to
# work OK. See
@@ -428,8 +428,8 @@ task:
depends_on: SanityCheck
only_if: $CIRRUS_CHANGE_MESSAGE !=~ '.*\nci-os-only:.*' || $CIRRUS_CHANGE_MESSAGE =~ '.*\nci-os-only:[^\n]*(macos|darwin|osx).*'
- osx_instance:
- image: monterey-base
+ macos_instance:
+ image: ghcr.io/cirruslabs/macos-monterey-base:latest
sysinfo_script: |
id
@@ -475,7 +475,7 @@ task:
ccache_cache:
folder: $CCACHE_DIR
configure_script: |
- brewpath="/usr/local"
+ brewpath="/opt/homebrew"
PKG_CONFIG_PATH="${brewpath}/lib/pkgconfig:${PKG_CONFIG_PATH}"
for pkg in icu4c krb5 openldap openssl zstd ; do
diff --git a/src/test/kerberos/t/001_auth.pl b/src/test/kerberos/t/001_auth.pl
index a2bc8a5351..298dc0c62b 100644
--- a/src/test/kerberos/t/001_auth.pl
+++ b/src/test/kerberos/t/001_auth.pl
@@ -32,8 +32,15 @@ elsif ($ENV{PG_TEST_EXTRA} !~ /\bkerberos\b/)
my ($krb5_bin_dir, $krb5_sbin_dir);
-if ($^O eq 'darwin')
+if ($^O eq 'darwin' && -d "/opt/homebrew" )
{
+ # typical paths for Homebrew on ARM
+ $krb5_bin_dir = '/opt/homebrew/opt/krb5/bin';
+ $krb5_sbin_dir = '/opt/homebrew/opt/krb5/sbin';
+}
+elsif ($^O eq 'darwin')
+{
+ # typical paths for Homebrew on Intel
$krb5_bin_dir = '/usr/local/opt/krb5/bin';
$krb5_sbin_dir = '/usr/local/opt/krb5/sbin';
}
diff --git a/src/test/ldap/t/001_auth.pl b/src/test/ldap/t/001_auth.pl
index 0ea274c383..39736e5116 100644
--- a/src/test/ldap/t/001_auth.pl
+++ b/src/test/ldap/t/001_auth.pl
@@ -21,9 +21,15 @@ elsif ($ENV{PG_TEST_EXTRA} !~ /\bldap\b/)
{
plan skip_all => 'Potentially unsafe test LDAP not enabled in PG_TEST_EXTRA';
}
+elsif ($^O eq 'darwin' && -d '/opt/homebrew/opt/openldap')
+{
+ # typical paths for Homebrew on ARM
+ $slapd = '/opt/homebrew/opt/openldap/libexec/slapd';
+ $ldap_schema_dir = '/opt/homebrew/etc/openldap/schema';
+}
elsif ($^O eq 'darwin' && -d '/usr/local/opt/openldap')
{
- # typical paths for Homebrew
+ # typical paths for Homebrew on Intel
$slapd = '/usr/local/opt/openldap/libexec/slapd';
$ldap_schema_dir = '/usr/local/etc/openldap/schema';
}
--
2.35.1
^ permalink raw reply [nested|flat] 43+ messages in thread
* Re: CI and test improvements
@ 2023-07-12 06:07 Michael Paquier <[email protected]>
parent: Justin Pryzby <[email protected]>
1 sibling, 1 reply; 43+ messages in thread
From: Michael Paquier @ 2023-07-12 06:07 UTC (permalink / raw)
To: Justin Pryzby <[email protected]>; +Cc: Andres Freund <[email protected]>; Andrew Dunstan <[email protected]>; Thomas Munro <[email protected]>; [email protected]; Noah Misch <[email protected]>; Anastasia Lubennikova <[email protected]>; Tom Lane <[email protected]>; Robert Haas <[email protected]>; Melanie Plageman <[email protected]>; Peter Eisentraut <[email protected]>; Daniel Gustafsson <[email protected]>; samay sharma <[email protected]>
On Wed, Jul 12, 2023 at 12:56:17AM -0500, Justin Pryzby wrote:
> And, since 681d9e462:
>
> security is missing from contrib/seg/meson.build
Ugh. Good catch!
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 43+ messages in thread
* Re: CI and test improvements
@ 2024-01-17 12:04 vignesh C <[email protected]>
parent: Michael Paquier <[email protected]>
0 siblings, 1 reply; 43+ messages in thread
From: vignesh C @ 2024-01-17 12:04 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; +Cc: Justin Pryzby <[email protected]>; Andres Freund <[email protected]>; Andrew Dunstan <[email protected]>; Thomas Munro <[email protected]>; [email protected]; Noah Misch <[email protected]>; Anastasia Lubennikova <[email protected]>; Tom Lane <[email protected]>; Robert Haas <[email protected]>; Melanie Plageman <[email protected]>; Peter Eisentraut <[email protected]>; Daniel Gustafsson <[email protected]>; samay sharma <[email protected]>
On Wed, 12 Jul 2023 at 11:38, Michael Paquier <[email protected]> wrote:
>
> On Wed, Jul 12, 2023 at 12:56:17AM -0500, Justin Pryzby wrote:
> > And, since 681d9e462:
> >
> > security is missing from contrib/seg/meson.build
>
> Ugh. Good catch!
Are we planning to do anything more in this thread, the thread has
been idle for more than 7 months. If nothing more is planned for this,
I'm planning to close this commitfest entry in this commitfest.
Regards,
Vignesh
^ permalink raw reply [nested|flat] 43+ messages in thread
* Re: CI and test improvements
@ 2024-01-18 01:16 Michael Paquier <[email protected]>
parent: vignesh C <[email protected]>
0 siblings, 1 reply; 43+ messages in thread
From: Michael Paquier @ 2024-01-18 01:16 UTC (permalink / raw)
To: vignesh C <[email protected]>; +Cc: Justin Pryzby <[email protected]>; Andres Freund <[email protected]>; Andrew Dunstan <[email protected]>; Thomas Munro <[email protected]>; [email protected]; Noah Misch <[email protected]>; Anastasia Lubennikova <[email protected]>; Tom Lane <[email protected]>; Robert Haas <[email protected]>; Melanie Plageman <[email protected]>; Peter Eisentraut <[email protected]>; Daniel Gustafsson <[email protected]>; samay sharma <[email protected]>
On Wed, Jan 17, 2024 at 05:34:00PM +0530, vignesh C wrote:
> Are we planning to do anything more in this thread, the thread has
> been idle for more than 7 months. If nothing more is planned for this,
> I'm planning to close this commitfest entry in this commitfest.
Oops, this went through the cracks. security was still missing in
seg's meson.build, so I've just applied a patch to take care of it.
I am not spotting any other holes..
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 43+ messages in thread
* Re: CI and test improvements
@ 2024-01-31 09:40 vignesh C <[email protected]>
parent: Michael Paquier <[email protected]>
0 siblings, 1 reply; 43+ messages in thread
From: vignesh C @ 2024-01-31 09:40 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; +Cc: Justin Pryzby <[email protected]>; Andres Freund <[email protected]>; Andrew Dunstan <[email protected]>; Thomas Munro <[email protected]>; [email protected]; Noah Misch <[email protected]>; Anastasia Lubennikova <[email protected]>; Tom Lane <[email protected]>; Robert Haas <[email protected]>; Melanie Plageman <[email protected]>; Peter Eisentraut <[email protected]>; Daniel Gustafsson <[email protected]>; samay sharma <[email protected]>
On Thu, 18 Jan 2024 at 06:46, Michael Paquier <[email protected]> wrote:
>
> On Wed, Jan 17, 2024 at 05:34:00PM +0530, vignesh C wrote:
> > Are we planning to do anything more in this thread, the thread has
> > been idle for more than 7 months. If nothing more is planned for this,
> > I'm planning to close this commitfest entry in this commitfest.
>
> Oops, this went through the cracks. security was still missing in
> seg's meson.build, so I've just applied a patch to take care of it.
> I am not spotting any other holes..
Are we planning to do anything more on this? I was not sure if we
should move this to next commitfest or return it.
Regards,
Vignesh
^ permalink raw reply [nested|flat] 43+ messages in thread
* Re: CI and test improvements
@ 2024-01-31 10:59 Alvaro Herrera <[email protected]>
parent: vignesh C <[email protected]>
0 siblings, 0 replies; 43+ messages in thread
From: Alvaro Herrera @ 2024-01-31 10:59 UTC (permalink / raw)
To: vignesh C <[email protected]>; +Cc: Michael Paquier <[email protected]>; Justin Pryzby <[email protected]>; Andres Freund <[email protected]>; Andrew Dunstan <[email protected]>; Thomas Munro <[email protected]>; [email protected]; Noah Misch <[email protected]>; Anastasia Lubennikova <[email protected]>; Tom Lane <[email protected]>; Robert Haas <[email protected]>; Melanie Plageman <[email protected]>; Peter Eisentraut <[email protected]>; Daniel Gustafsson <[email protected]>; samay sharma <[email protected]>
On 2024-Jan-31, vignesh C wrote:
> Are we planning to do anything more on this? I was not sure if we
> should move this to next commitfest or return it.
Well, the patches don't apply anymore since .cirrus.tasks.yml has been
created. However, I'm sure we still want [some of] the improvements
to the tests in [1]. I can volunteer to rebase the patches in time for the
March commitfest, if Justin is not available to do so. If you can
please move it forward to the March cf and set it WoA, I'd appreciate
that.
Thanks
[1] https://postgr.es/m/ZA/[email protected]
--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
"Tiene valor aquel que admite que es un cobarde" (Fernandel)
^ permalink raw reply [nested|flat] 43+ messages in thread
end of thread, other threads:[~2024-01-31 10:59 UTC | newest]
Thread overview: 43+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-04-03 21:17 [PATCH v6 08/10] doc: backup manifests Justin Pryzby <[email protected]>
2022-05-28 15:37 CI and test improvements Justin Pryzby <[email protected]>
2022-06-23 19:31 ` Re: CI and test improvements Justin Pryzby <[email protected]>
2022-06-23 20:38 ` Re: CI and test improvements Thomas Munro <[email protected]>
2022-07-08 00:22 ` Re: CI and test improvements Justin Pryzby <[email protected]>
2022-08-28 14:44 ` Re: CI and test improvements Justin Pryzby <[email protected]>
2022-08-28 16:07 ` Re: CI and test improvements Andres Freund <[email protected]>
2022-08-28 17:10 ` Re: CI and test improvements Justin Pryzby <[email protected]>
2022-08-28 21:28 ` Re: CI and test improvements Andres Freund <[email protected]>
2022-09-10 20:05 ` Re: CI and test improvements Justin Pryzby <[email protected]>
2022-10-02 00:45 ` Re: CI and test improvements Andres Freund <[email protected]>
2022-10-02 00:58 ` Re: CI and test improvements Justin Pryzby <[email protected]>
2022-10-02 01:36 ` Re: CI and test improvements Andres Freund <[email protected]>
2022-10-02 20:52 ` Re: CI and test improvements Andres Freund <[email protected]>
2022-10-02 21:35 ` Re: CI and test improvements Justin Pryzby <[email protected]>
2022-10-02 21:51 ` Re: CI and test improvements Andres Freund <[email protected]>
2022-10-02 21:54 ` Re: CI and test improvements Andres Freund <[email protected]>
2022-10-03 02:15 ` Re: CI and test improvements Justin Pryzby <[email protected]>
2022-11-17 03:48 ` Re: CI and test improvements Andres Freund <[email protected]>
2022-11-17 03:58 ` Re: CI and test improvements Justin Pryzby <[email protected]>
2022-11-17 04:08 ` Re: CI and test improvements Andres Freund <[email protected]>
2022-11-17 04:16 ` Re: CI and test improvements Justin Pryzby <[email protected]>
2022-11-19 20:22 ` Re: CI and test improvements Justin Pryzby <[email protected]>
2022-11-19 21:18 ` Re: CI and test improvements Andres Freund <[email protected]>
2022-11-19 21:35 ` Re: CI and test improvements Andres Freund <[email protected]>
2022-11-19 21:45 ` Re: CI and test improvements Justin Pryzby <[email protected]>
2022-11-19 22:14 ` Re: CI and test improvements Andres Freund <[email protected]>
2022-11-21 22:09 ` Re: CI and test improvements Andres Freund <[email protected]>
2022-11-04 23:54 ` Re: CI and test improvements Justin Pryzby <[email protected]>
2022-11-05 01:59 ` Re: CI and test improvements Andres Freund <[email protected]>
2022-11-13 23:53 ` Re: CI and test improvements Justin Pryzby <[email protected]>
2022-11-21 22:45 ` Re: CI and test improvements Andres Freund <[email protected]>
2022-11-22 22:57 ` Re: CI and test improvements Justin Pryzby <[email protected]>
2022-12-30 03:59 ` Re: CI and test improvements Thomas Munro <[email protected]>
2023-07-12 06:07 ` Re: CI and test improvements Michael Paquier <[email protected]>
2024-01-17 12:04 ` Re: CI and test improvements vignesh C <[email protected]>
2024-01-18 01:16 ` Re: CI and test improvements Michael Paquier <[email protected]>
2024-01-31 09:40 ` Re: CI and test improvements vignesh C <[email protected]>
2024-01-31 10:59 ` Re: CI and test improvements Alvaro Herrera <[email protected]>
2022-09-22 21:07 ` Re: CI and test improvements Justin Pryzby <[email protected]>
2022-09-22 21:53 ` Re: CI and test improvements Andres Freund <[email protected]>
2022-09-23 03:32 ` Re: CI and test improvements Thomas Munro <[email protected]>
2022-11-26 15:21 Re: CI and test improvements Andrew Dunstan <[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