agora inbox for [email protected]
help / color / mirror / Atom feed[PATCH v3 05/17] meson: prereq: move snowball_create.sql creation into perl file.
10+ messages / 7 participants
[nested] [flat]
* [PATCH v3 05/17] meson: prereq: move snowball_create.sql creation into perl file.
@ 2021-03-08 22:59 Andres Freund <[email protected]>
0 siblings, 0 replies; 10+ messages in thread
From: Andres Freund @ 2021-03-08 22:59 UTC (permalink / raw)
FIXME: deduplicate with Install.pm
---
src/backend/snowball/Makefile | 27 +-----
src/backend/snowball/snowball_create.pl | 110 ++++++++++++++++++++++++
2 files changed, 113 insertions(+), 24 deletions(-)
create mode 100644 src/backend/snowball/snowball_create.pl
diff --git a/src/backend/snowball/Makefile b/src/backend/snowball/Makefile
index 50b9199910c..259104f8eb3 100644
--- a/src/backend/snowball/Makefile
+++ b/src/backend/snowball/Makefile
@@ -119,29 +119,8 @@ all: all-shared-lib $(SQLSCRIPT)
include $(top_srcdir)/src/Makefile.shlib
-$(SQLSCRIPT): Makefile snowball_func.sql.in snowball.sql.in
- echo '-- Language-specific snowball dictionaries' > $@
- cat $(srcdir)/snowball_func.sql.in >> $@
- @set -e; \
- set $(LANGUAGES) ; \
- while [ "$$#" -gt 0 ] ; \
- do \
- lang=$$1; shift; \
- nonascdictname=$$lang; \
- ascdictname=$$1; shift; \
- if [ -s $(srcdir)/stopwords/$${lang}.stop ] ; then \
- stop=", StopWords=$${lang}" ; \
- else \
- stop=""; \
- fi; \
- cat $(srcdir)/snowball.sql.in | \
- sed -e "s#_LANGNAME_#$$lang#g" | \
- sed -e "s#_DICTNAME_#$${lang}_stem#g" | \
- sed -e "s#_CFGNAME_#$$lang#g" | \
- sed -e "s#_ASCDICTNAME_#$${ascdictname}_stem#g" | \
- sed -e "s#_NONASCDICTNAME_#$${nonascdictname}_stem#g" | \
- sed -e "s#_STOPWORDS_#$$stop#g" ; \
- done >> $@
+$(SQLSCRIPT): snowball_create.pl Makefile snowball_func.sql.in snowball.sql.in
+ $(PERL) $< --input ${srcdir} --output .
install: all installdirs install-lib
$(INSTALL_DATA) $(SQLSCRIPT) '$(DESTDIR)$(datadir)'
@@ -171,4 +150,4 @@ uninstall: uninstall-lib
done
clean distclean maintainer-clean: clean-lib
- rm -f $(OBJS) $(SQLSCRIPT)
+ rm -f $(OBJS) $(SQLSCRIPT) snowball_create.dep
diff --git a/src/backend/snowball/snowball_create.pl b/src/backend/snowball/snowball_create.pl
new file mode 100644
index 00000000000..d9d79f3668f
--- /dev/null
+++ b/src/backend/snowball/snowball_create.pl
@@ -0,0 +1,110 @@
+#!/usr/bin/perl
+
+use strict;
+use warnings;
+use Getopt::Long;
+
+my $output_path = '';
+my $makefile_path = '';
+my $input_path = '';
+
+GetOptions(
+ 'output:s' => \$output_path,
+ 'input:s' => \$input_path) || usage();
+
+# Make sure input_path ends in a slash if needed.
+if ($input_path ne '' && substr($input_path, -1) ne '/')
+{
+ $output_path .= '/';
+}
+
+# Make sure output_path ends in a slash if needed.
+if ($output_path ne '' && substr($output_path, -1) ne '/')
+{
+ $output_path .= '/';
+}
+
+GenerateTsearchFiles();
+
+sub usage
+{
+ die <<EOM;
+Usage: snowball_create.pl --input/-i <path> --input <path>
+ --output Output directory (default '.')
+ --input Input directory
+
+snowball_create.pl creates snowball.sql from snowball.sql.in
+EOM
+}
+
+sub GenerateTsearchFiles
+{
+ my $target = shift;
+ my $output_file = "$output_path/snowball_create.sql";
+
+ print "Generating tsearch script...";
+ my $F;
+ my $D;
+ my $tmpl = read_file("$input_path/snowball.sql.in");
+ my $mf = read_file("$input_path/Makefile");
+
+ open($D, '>', "$output_path/snowball_create.dep")
+ || die "Could not write snowball_create.dep";
+
+ print $D "$output_file: $input_path/Makefile\n";
+ print $D "$output_file: $input_path/snowball.sql.in\n";
+ print $D "$output_file: $input_path/snowball_func.sql.in\n";
+
+ $mf =~ s{\\\r?\n}{}g;
+ $mf =~ /^LANGUAGES\s*=\s*(.*)$/m
+ || die "Could not find LANGUAGES line in snowball Makefile\n";
+ my @pieces = split /\s+/, $1;
+ open($F, '>', $output_file)
+ || die "Could not write snowball_create.sql";
+
+ print $F "-- Language-specific snowball dictionaries\n";
+
+ print $F read_file("$input_path/snowball_func.sql.in");
+
+ while ($#pieces > 0)
+ {
+ my $lang = shift @pieces || last;
+ my $asclang = shift @pieces || last;
+ my $txt = $tmpl;
+ my $stop = '';
+ my $stopword_path = "$input_path/stopwords/$lang.stop";
+
+ if (-s "$stopword_path")
+ {
+ $stop = ", StopWords=$lang";
+
+ print $D "$output_file: $stopword_path\n";
+ }
+
+ $txt =~ s#_LANGNAME_#${lang}#gs;
+ $txt =~ s#_DICTNAME_#${lang}_stem#gs;
+ $txt =~ s#_CFGNAME_#${lang}#gs;
+ $txt =~ s#_ASCDICTNAME_#${asclang}_stem#gs;
+ $txt =~ s#_NONASCDICTNAME_#${lang}_stem#gs;
+ $txt =~ s#_STOPWORDS_#$stop#gs;
+ print $F $txt;
+ print ".";
+ }
+ close($F);
+ close($D);
+ print "\n";
+ return;
+}
+
+
+sub read_file
+{
+ my $filename = shift;
+ my $F;
+ local $/ = undef;
+ open($F, '<', $filename) || die "Could not open file $filename\n";
+ my $txt = <$F>;
+ close($F);
+
+ return $txt;
+}
--
2.23.0.385.gbc12974a89
--qozqt6hocm2wibt2
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v3-0006-meson-prereq-add-output-path-arg-in-generate-lwlo.patch"
^ permalink raw reply [nested|flat] 10+ messages in thread
* [PATCH v5 05/16] meson: prereq: move snowball_create.sql creation into perl file.
@ 2021-03-08 22:59 Andres Freund <[email protected]>
0 siblings, 0 replies; 10+ messages in thread
From: Andres Freund @ 2021-03-08 22:59 UTC (permalink / raw)
FIXME: deduplicate with Install.pm
---
src/backend/snowball/Makefile | 27 +-----
src/backend/snowball/snowball_create.pl | 110 ++++++++++++++++++++++++
2 files changed, 113 insertions(+), 24 deletions(-)
create mode 100644 src/backend/snowball/snowball_create.pl
diff --git a/src/backend/snowball/Makefile b/src/backend/snowball/Makefile
index 50b9199910c..259104f8eb3 100644
--- a/src/backend/snowball/Makefile
+++ b/src/backend/snowball/Makefile
@@ -119,29 +119,8 @@ all: all-shared-lib $(SQLSCRIPT)
include $(top_srcdir)/src/Makefile.shlib
-$(SQLSCRIPT): Makefile snowball_func.sql.in snowball.sql.in
- echo '-- Language-specific snowball dictionaries' > $@
- cat $(srcdir)/snowball_func.sql.in >> $@
- @set -e; \
- set $(LANGUAGES) ; \
- while [ "$$#" -gt 0 ] ; \
- do \
- lang=$$1; shift; \
- nonascdictname=$$lang; \
- ascdictname=$$1; shift; \
- if [ -s $(srcdir)/stopwords/$${lang}.stop ] ; then \
- stop=", StopWords=$${lang}" ; \
- else \
- stop=""; \
- fi; \
- cat $(srcdir)/snowball.sql.in | \
- sed -e "s#_LANGNAME_#$$lang#g" | \
- sed -e "s#_DICTNAME_#$${lang}_stem#g" | \
- sed -e "s#_CFGNAME_#$$lang#g" | \
- sed -e "s#_ASCDICTNAME_#$${ascdictname}_stem#g" | \
- sed -e "s#_NONASCDICTNAME_#$${nonascdictname}_stem#g" | \
- sed -e "s#_STOPWORDS_#$$stop#g" ; \
- done >> $@
+$(SQLSCRIPT): snowball_create.pl Makefile snowball_func.sql.in snowball.sql.in
+ $(PERL) $< --input ${srcdir} --output .
install: all installdirs install-lib
$(INSTALL_DATA) $(SQLSCRIPT) '$(DESTDIR)$(datadir)'
@@ -171,4 +150,4 @@ uninstall: uninstall-lib
done
clean distclean maintainer-clean: clean-lib
- rm -f $(OBJS) $(SQLSCRIPT)
+ rm -f $(OBJS) $(SQLSCRIPT) snowball_create.dep
diff --git a/src/backend/snowball/snowball_create.pl b/src/backend/snowball/snowball_create.pl
new file mode 100644
index 00000000000..d9d79f3668f
--- /dev/null
+++ b/src/backend/snowball/snowball_create.pl
@@ -0,0 +1,110 @@
+#!/usr/bin/perl
+
+use strict;
+use warnings;
+use Getopt::Long;
+
+my $output_path = '';
+my $makefile_path = '';
+my $input_path = '';
+
+GetOptions(
+ 'output:s' => \$output_path,
+ 'input:s' => \$input_path) || usage();
+
+# Make sure input_path ends in a slash if needed.
+if ($input_path ne '' && substr($input_path, -1) ne '/')
+{
+ $output_path .= '/';
+}
+
+# Make sure output_path ends in a slash if needed.
+if ($output_path ne '' && substr($output_path, -1) ne '/')
+{
+ $output_path .= '/';
+}
+
+GenerateTsearchFiles();
+
+sub usage
+{
+ die <<EOM;
+Usage: snowball_create.pl --input/-i <path> --input <path>
+ --output Output directory (default '.')
+ --input Input directory
+
+snowball_create.pl creates snowball.sql from snowball.sql.in
+EOM
+}
+
+sub GenerateTsearchFiles
+{
+ my $target = shift;
+ my $output_file = "$output_path/snowball_create.sql";
+
+ print "Generating tsearch script...";
+ my $F;
+ my $D;
+ my $tmpl = read_file("$input_path/snowball.sql.in");
+ my $mf = read_file("$input_path/Makefile");
+
+ open($D, '>', "$output_path/snowball_create.dep")
+ || die "Could not write snowball_create.dep";
+
+ print $D "$output_file: $input_path/Makefile\n";
+ print $D "$output_file: $input_path/snowball.sql.in\n";
+ print $D "$output_file: $input_path/snowball_func.sql.in\n";
+
+ $mf =~ s{\\\r?\n}{}g;
+ $mf =~ /^LANGUAGES\s*=\s*(.*)$/m
+ || die "Could not find LANGUAGES line in snowball Makefile\n";
+ my @pieces = split /\s+/, $1;
+ open($F, '>', $output_file)
+ || die "Could not write snowball_create.sql";
+
+ print $F "-- Language-specific snowball dictionaries\n";
+
+ print $F read_file("$input_path/snowball_func.sql.in");
+
+ while ($#pieces > 0)
+ {
+ my $lang = shift @pieces || last;
+ my $asclang = shift @pieces || last;
+ my $txt = $tmpl;
+ my $stop = '';
+ my $stopword_path = "$input_path/stopwords/$lang.stop";
+
+ if (-s "$stopword_path")
+ {
+ $stop = ", StopWords=$lang";
+
+ print $D "$output_file: $stopword_path\n";
+ }
+
+ $txt =~ s#_LANGNAME_#${lang}#gs;
+ $txt =~ s#_DICTNAME_#${lang}_stem#gs;
+ $txt =~ s#_CFGNAME_#${lang}#gs;
+ $txt =~ s#_ASCDICTNAME_#${asclang}_stem#gs;
+ $txt =~ s#_NONASCDICTNAME_#${lang}_stem#gs;
+ $txt =~ s#_STOPWORDS_#$stop#gs;
+ print $F $txt;
+ print ".";
+ }
+ close($F);
+ close($D);
+ print "\n";
+ return;
+}
+
+
+sub read_file
+{
+ my $filename = shift;
+ my $F;
+ local $/ = undef;
+ open($F, '<', $filename) || die "Could not open file $filename\n";
+ my $txt = <$F>;
+ close($F);
+
+ return $txt;
+}
--
2.23.0.385.gbc12974a89
--q7iu5jwpuq7sbl3z
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v5-0006-meson-prereq-add-output-path-arg-in-generate-lwlo.patch"
^ permalink raw reply [nested|flat] 10+ messages in thread
* [PATCH v10 04/16] meson: prereq: Move snowball_create.sql creation into perl file
@ 2022-01-20 07:36 Andres Freund <[email protected]>
0 siblings, 0 replies; 10+ messages in thread
From: Andres Freund @ 2022-01-20 07:36 UTC (permalink / raw)
Author: Peter Eisentraut <[email protected]>
Author: Andres Freund <[email protected]>
---
src/backend/snowball/Makefile | 104 +++++------------
src/backend/snowball/snowball_create.pl | 148 ++++++++++++++++++++++++
src/tools/msvc/Install.pm | 36 +-----
3 files changed, 178 insertions(+), 110 deletions(-)
create mode 100644 src/backend/snowball/snowball_create.pl
diff --git a/src/backend/snowball/Makefile b/src/backend/snowball/Makefile
index 50b9199910c..29076371db7 100644
--- a/src/backend/snowball/Makefile
+++ b/src/backend/snowball/Makefile
@@ -72,40 +72,22 @@ OBJS += \
stem_UTF_8_turkish.o \
stem_UTF_8_yiddish.o
-# first column is language name and also name of dictionary for not-all-ASCII
-# words, second is name of dictionary for all-ASCII words
-# Note order dependency: use of some other language as ASCII dictionary
-# must come after creation of that language
-LANGUAGES= \
- arabic arabic \
- armenian armenian \
- basque basque \
- catalan catalan \
- danish danish \
- dutch dutch \
- english english \
- finnish finnish \
- french french \
- german german \
- greek greek \
- hindi english \
- hungarian hungarian \
- indonesian indonesian \
- irish irish \
- italian italian \
- lithuanian lithuanian \
- nepali nepali \
- norwegian norwegian \
- portuguese portuguese \
- romanian romanian \
- russian english \
- serbian serbian \
- spanish spanish \
- swedish swedish \
- tamil tamil \
- turkish turkish \
- yiddish yiddish
-
+stop_files = \
+ danish.stop \
+ dutch.stop \
+ english.stop \
+ finnish.stop \
+ french.stop \
+ german.stop \
+ hungarian.stop \
+ italian.stop \
+ nepali.stop \
+ norwegian.stop \
+ portuguese.stop \
+ russian.stop \
+ spanish.stop \
+ swedish.stop \
+ turkish.stop
SQLSCRIPT= snowball_create.sql
DICTDIR=tsearch_data
@@ -119,56 +101,24 @@ all: all-shared-lib $(SQLSCRIPT)
include $(top_srcdir)/src/Makefile.shlib
-$(SQLSCRIPT): Makefile snowball_func.sql.in snowball.sql.in
- echo '-- Language-specific snowball dictionaries' > $@
- cat $(srcdir)/snowball_func.sql.in >> $@
- @set -e; \
- set $(LANGUAGES) ; \
- while [ "$$#" -gt 0 ] ; \
- do \
- lang=$$1; shift; \
- nonascdictname=$$lang; \
- ascdictname=$$1; shift; \
- if [ -s $(srcdir)/stopwords/$${lang}.stop ] ; then \
- stop=", StopWords=$${lang}" ; \
- else \
- stop=""; \
- fi; \
- cat $(srcdir)/snowball.sql.in | \
- sed -e "s#_LANGNAME_#$$lang#g" | \
- sed -e "s#_DICTNAME_#$${lang}_stem#g" | \
- sed -e "s#_CFGNAME_#$$lang#g" | \
- sed -e "s#_ASCDICTNAME_#$${ascdictname}_stem#g" | \
- sed -e "s#_NONASCDICTNAME_#$${nonascdictname}_stem#g" | \
- sed -e "s#_STOPWORDS_#$$stop#g" ; \
- done >> $@
+$(SQLSCRIPT): snowball_create.pl snowball_func.sql.in snowball.sql.in
+ $(PERL) $< --input ${srcdir} --outdir .
+
+distprep: $(SQLSCRIPT)
install: all installdirs install-lib
$(INSTALL_DATA) $(SQLSCRIPT) '$(DESTDIR)$(datadir)'
- @set -e; \
- set $(LANGUAGES) ; \
- while [ "$$#" -gt 0 ] ; \
- do \
- lang=$$1; shift; shift; \
- if [ -s $(srcdir)/stopwords/$${lang}.stop ] ; then \
- $(INSTALL_DATA) $(srcdir)/stopwords/$${lang}.stop '$(DESTDIR)$(datadir)/$(DICTDIR)' ; \
- fi \
- done
+ $(INSTALL_DATA) $(addprefix $(srcdir)/stopwords/,$(stop_files)) '$(DESTDIR)$(datadir)/$(DICTDIR)'
installdirs: installdirs-lib
$(MKDIR_P) '$(DESTDIR)$(datadir)' '$(DESTDIR)$(datadir)/$(DICTDIR)'
uninstall: uninstall-lib
rm -f '$(DESTDIR)$(datadir)/$(SQLSCRIPT)'
- @set -e; \
- set $(LANGUAGES) ; \
- while [ "$$#" -gt 0 ] ; \
- do \
- lang=$$1; shift; shift; \
- if [ -s $(srcdir)/stopwords/$${lang}.stop ] ; then \
- rm -f '$(DESTDIR)$(datadir)/$(DICTDIR)/'$${lang}.stop ; \
- fi \
- done
+ rm -f $(addprefix '$(DESTDIR)$(datadir)/$(DICTDIR)/',$(stop_files))
-clean distclean maintainer-clean: clean-lib
- rm -f $(OBJS) $(SQLSCRIPT)
+clean distclean: clean-lib
+ rm -f $(OBJS)
+
+maintainer-clean: distclean
+ rm -f $(SQLSCRIPT)
diff --git a/src/backend/snowball/snowball_create.pl b/src/backend/snowball/snowball_create.pl
new file mode 100644
index 00000000000..f4b58ada1cb
--- /dev/null
+++ b/src/backend/snowball/snowball_create.pl
@@ -0,0 +1,148 @@
+#!/usr/bin/perl
+
+use strict;
+use warnings;
+use Getopt::Long;
+
+my $outdir_path = '';
+my $makefile_path = '';
+my $input_path = '';
+my $depfile;
+
+our @languages = qw(
+ arabic
+ armenian
+ basque
+ catalan
+ danish
+ dutch
+ english
+ finnish
+ french
+ german
+ greek
+ hindi
+ hungarian
+ indonesian
+ irish
+ italian
+ lithuanian
+ nepali
+ norwegian
+ portuguese
+ romanian
+ russian
+ serbian
+ spanish
+ swedish
+ tamil
+ turkish
+ yiddish
+);
+
+# Names of alternative dictionaries for all-ASCII words. If not
+# listed, the language itself is used. Note order dependency: Use of
+# some other language as ASCII dictionary must come after creation of
+# that language, so the "backup" language must be listed earlier in
+# @languages.
+
+our %ascii_languages = (
+ 'hindi' => 'english',
+ 'russian' => 'english',
+);
+
+GetOptions(
+ 'depfile' => \$depfile,
+ 'outdir:s' => \$outdir_path,
+ 'input:s' => \$input_path) || usage();
+
+# Make sure input_path ends in a slash if needed.
+if ($input_path ne '' && substr($input_path, -1) ne '/')
+{
+ $outdir_path .= '/';
+}
+
+# Make sure outdir_path ends in a slash if needed.
+if ($outdir_path ne '' && substr($outdir_path, -1) ne '/')
+{
+ $outdir_path .= '/';
+}
+
+GenerateTsearchFiles();
+
+sub usage
+{
+ die <<EOM;
+Usage: snowball_create.pl --input/-i <path> --outdir/-o <path>
+ --depfile Write dependency file
+ --outdir Output directory (default '.')
+ --input Input directory
+
+snowball_create.pl creates snowball.sql from snowball.sql.in
+EOM
+}
+
+sub GenerateTsearchFiles
+{
+ my $target = shift;
+ my $outdir_file = "$outdir_path/snowball_create.sql";
+
+ my $F;
+ my $D;
+ my $tmpl = read_file("$input_path/snowball.sql.in");
+
+ if ($depfile)
+ {
+ open($D, '>', "$outdir_path/snowball_create.dep")
+ || die "Could not write snowball_create.dep";
+ }
+
+ print $D "$outdir_file: $input_path/snowball.sql.in\n" if $depfile;
+ print $D "$outdir_file: $input_path/snowball_func.sql.in\n" if $depfile;
+
+ open($F, '>', $outdir_file)
+ || die "Could not write snowball_create.sql";
+
+ print $F "-- Language-specific snowball dictionaries\n";
+
+ print $F read_file("$input_path/snowball_func.sql.in");
+
+ foreach my $lang (@languages)
+ {
+ my $asclang = $ascii_languages{$lang} || $lang;
+ my $txt = $tmpl;
+ my $stop = '';
+ my $stopword_path = "$input_path/stopwords/$lang.stop";
+
+ if (-s "$stopword_path")
+ {
+ $stop = ", StopWords=$lang";
+
+ print $D "$outdir_file: $stopword_path\n" if $depfile;
+ }
+
+ $txt =~ s#_LANGNAME_#${lang}#gs;
+ $txt =~ s#_DICTNAME_#${lang}_stem#gs;
+ $txt =~ s#_CFGNAME_#${lang}#gs;
+ $txt =~ s#_ASCDICTNAME_#${asclang}_stem#gs;
+ $txt =~ s#_NONASCDICTNAME_#${lang}_stem#gs;
+ $txt =~ s#_STOPWORDS_#$stop#gs;
+ print $F $txt;
+ }
+ close($F);
+ close($D) if $depfile;
+ return;
+}
+
+
+sub read_file
+{
+ my $filename = shift;
+ my $F;
+ local $/ = undef;
+ open($F, '<', $filename) || die "Could not open file $filename\n";
+ my $txt = <$F>;
+ close($F);
+
+ return $txt;
+}
diff --git a/src/tools/msvc/Install.pm b/src/tools/msvc/Install.pm
index 8de79c618cb..5da299476eb 100644
--- a/src/tools/msvc/Install.pm
+++ b/src/tools/msvc/Install.pm
@@ -389,39 +389,9 @@ sub GenerateTsearchFiles
my $target = shift;
print "Generating tsearch script...";
- my $F;
- my $tmpl = read_file('src/backend/snowball/snowball.sql.in');
- my $mf = read_file('src/backend/snowball/Makefile');
- $mf =~ s{\\\r?\n}{}g;
- $mf =~ /^LANGUAGES\s*=\s*(.*)$/m
- || die "Could not find LANGUAGES line in snowball Makefile\n";
- my @pieces = split /\s+/, $1;
- open($F, '>', "$target/share/snowball_create.sql")
- || die "Could not write snowball_create.sql";
- print $F read_file('src/backend/snowball/snowball_func.sql.in');
-
- while ($#pieces > 0)
- {
- my $lang = shift @pieces || last;
- my $asclang = shift @pieces || last;
- my $txt = $tmpl;
- my $stop = '';
-
- if (-s "src/backend/snowball/stopwords/$lang.stop")
- {
- $stop = ", StopWords=$lang";
- }
-
- $txt =~ s#_LANGNAME_#${lang}#gs;
- $txt =~ s#_DICTNAME_#${lang}_stem#gs;
- $txt =~ s#_CFGNAME_#${lang}#gs;
- $txt =~ s#_ASCDICTNAME_#${asclang}_stem#gs;
- $txt =~ s#_NONASCDICTNAME_#${lang}_stem#gs;
- $txt =~ s#_STOPWORDS_#$stop#gs;
- print $F $txt;
- print ".";
- }
- close($F);
+ system('perl', 'src/backend/snowball/snowball_create.pl',
+ '--input', 'src/backend/snowball/',
+ '--outdir', "$target/share/");
print "\n";
return;
}
--
2.37.0.3.g30cc8d0f14
--mqd6wwweo5bfrq2e
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v10-0005-meson-prereq-Add-output-path-arg-in-generate-lwl.patch"
^ permalink raw reply [nested|flat] 10+ messages in thread
* Primary and standby setting cross-checks
@ 2024-08-29 18:52 Heikki Linnakangas <[email protected]>
0 siblings, 2 replies; 10+ messages in thread
From: Heikki Linnakangas @ 2024-08-29 18:52 UTC (permalink / raw)
To: pgsql-hackers
Currently, if you configure a hot standby server with a smaller
max_connections setting than the primary, the server refuses to start up:
LOG: entering standby mode
FATAL: recovery aborted because of insufficient parameter settings
DETAIL: max_connections = 10 is a lower setting than on the primary
server, where its value was 100.
HINT: You can restart the server after making the necessary
configuration changes.
Or if you change the setting in the primary while the standby is
running, replay pauses:
WARNING: hot standby is not possible because of insufficient parameter
settings
DETAIL: max_connections = 100 is a lower setting than on the primary
server, where its value was 200.
CONTEXT: WAL redo at 2/E10000D8 for XLOG/PARAMETER_CHANGE:
max_connections=200 max_worker_processes=8 max_wal_senders=10
max_prepared_xacts=0 max_locks_per_xact=64 wal_level=logical
wal_log_hints=off track_commit_timestamp=off
LOG: recovery has paused
DETAIL: If recovery is unpaused, the server will shut down.
HINT: You can then restart the server after making the necessary
configuration changes.
CONTEXT: WAL redo at 2/E10000D8 for XLOG/PARAMETER_CHANGE:
max_connections=200 max_worker_processes=8 max_wal_senders=10
max_prepared_xacts=0 max_locks_per_xact=64 wal_level=logical
wal_log_hints=off track_commit_timestamp=off
Both of these are rather unpleasant behavior.
I thought I could get rid of that limitation with my CSN snapshot patch
[1], because it gets rid of the fixed-size known-assigned XIDs array,
but there's a second reason for these limitations. It's also used to
ensure that the standby has enough space in the lock manager to hold
possible AccessExclusiveLocks taken by transactions in the primary.
So firstly, I think that's a bad tradeoff. In vast majority of cases,
you would not run out of lock space anyway, if you just started up the
system. Secondly, that cross-check of settings doesn't fully prevent the
problem. It ensures that the lock tables are large enough to accommodate
all the locks you could possibly hold in the primary, but that doesn't
take into account any additional locks held by read-only queries in the
hot standby. So if you have queries running in the standby that take a
lot of locks, this can happen anyway:
2024-08-29 21:44:32.634 EEST [668327] FATAL: out of shared memory
2024-08-29 21:44:32.634 EEST [668327] HINT: You might need to increase
"max_locks_per_transaction".
2024-08-29 21:44:32.634 EEST [668327] CONTEXT: WAL redo at 2/FD40FCC8
for Standby/LOCK: xid 996 db 5 rel 154045
2024-08-29 21:44:32.634 EEST [668327] WARNING: you don't own a lock of
type AccessExclusiveLock
2024-08-29 21:44:32.634 EEST [668327] LOG: RecoveryLockHash contains
entry for lock no longer recorded by lock manager: xid 996 database 5
relation 154045
TRAP: failed Assert("false"), File:
"../src/backend/storage/ipc/standby.c", Line: 1053, PID: 668327
postgres: startup recovering
0000000100000002000000FD(ExceptionalCondition+0x6e)[0x556a4588396e]
postgres: startup recovering
0000000100000002000000FD(+0x44156e)[0x556a4571356e]
postgres: startup recovering
0000000100000002000000FD(StandbyReleaseAllLocks+0x78)[0x556a45712738]
postgres: startup recovering
0000000100000002000000FD(ShutdownRecoveryTransactionEnvironment+0x15)[0x556a45712685]
postgres: startup recovering
0000000100000002000000FD(shmem_exit+0x111)[0x556a457062e1]
postgres: startup recovering
0000000100000002000000FD(+0x434132)[0x556a45706132]
postgres: startup recovering
0000000100000002000000FD(proc_exit+0x59)[0x556a45706079]
postgres: startup recovering
0000000100000002000000FD(errfinish+0x278)[0x556a45884708]
postgres: startup recovering
0000000100000002000000FD(LockAcquireExtended+0xa46)[0x556a45719386]
postgres: startup recovering
0000000100000002000000FD(StandbyAcquireAccessExclusiveLock+0x11d)[0x556a4571330d]
postgres: startup recovering
0000000100000002000000FD(standby_redo+0x70)[0x556a45713690]
postgres: startup recovering
0000000100000002000000FD(PerformWalRecovery+0x7b3)[0x556a4547d313]
postgres: startup recovering
0000000100000002000000FD(StartupXLOG+0xac3)[0x556a4546dae3]
postgres: startup recovering
0000000100000002000000FD(StartupProcessMain+0xe8)[0x556a45693558]
postgres: startup recovering
0000000100000002000000FD(+0x3ba95d)[0x556a4568c95d]
postgres: startup recovering
0000000100000002000000FD(+0x3bce41)[0x556a4568ee41]
postgres: startup recovering
0000000100000002000000FD(PostmasterMain+0x116e)[0x556a4568eaae]
postgres: startup recovering
0000000100000002000000FD(+0x2f960e)[0x556a455cb60e]
/lib/x86_64-linux-gnu/libc.so.6(+0x27c8a)[0x7f10ef042c8a]
/lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0x85)[0x7f10ef042d45]
postgres: startup recovering
0000000100000002000000FD(_start+0x21)[0x556a453af011]
2024-08-29 21:44:32.641 EEST [668324] LOG: startup process (PID 668327)
was terminated by signal 6: Aborted
2024-08-29 21:44:32.641 EEST [668324] LOG: terminating any other active
server processes
2024-08-29 21:44:32.654 EEST [668324] LOG: shutting down due to startup
process failure
2024-08-29 21:44:32.729 EEST [668324] LOG: database system is shut down
Granted, if you restart the server, it will probably succeed because
restarting the server will kill all the other queries that were holding
locks. But yuck. With assertions disabled, it looks a little less scary,
but not nice anyway.
So how to improve this? I see a few options:
a) Downgrade the error at startup to a warning, and allow starting the
standby with smaller settings in standby. At least with a smaller
max_locks_per_transactions. The other settings also affect the size of
known-assigned XIDs array, but if the CSN snapshots get committed, that
will get fixed. In most cases there is enough lock memory anyway, and it
will be fine. Just fix the assertion failure so that the error message
is a little nicer.
b) If you run out of lock space, kill running queries, and prevent new
ones from starting. Track the locks in startup process' private memory
until there is enough space in the lock manager, and then re-open for
queries. In essence, go from hot standby mode to warm standby, until
it's possible to go back to hot standby mode again.
Thoughts, better ideas?
[1] https://commitfest.postgresql.org/49/4912/
--
Heikki Linnakangas
Neon (https://neon.tech)
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: Primary and standby setting cross-checks
@ 2024-09-25 03:03 Noah Misch <[email protected]>
parent: Heikki Linnakangas <[email protected]>
1 sibling, 0 replies; 10+ messages in thread
From: Noah Misch @ 2024-09-25 03:03 UTC (permalink / raw)
To: Heikki Linnakangas <[email protected]>; +Cc: pgsql-hackers
On Thu, Aug 29, 2024 at 09:52:06PM +0300, Heikki Linnakangas wrote:
> Currently, if you configure a hot standby server with a smaller
> max_connections setting than the primary, the server refuses to start up:
>
> LOG: entering standby mode
> FATAL: recovery aborted because of insufficient parameter settings
> DETAIL: max_connections = 10 is a lower setting than on the primary server,
> where its value was 100.
> happen anyway:
>
> 2024-08-29 21:44:32.634 EEST [668327] FATAL: out of shared memory
> 2024-08-29 21:44:32.634 EEST [668327] HINT: You might need to increase
> "max_locks_per_transaction".
> 2024-08-29 21:44:32.634 EEST [668327] CONTEXT: WAL redo at 2/FD40FCC8 for
> Standby/LOCK: xid 996 db 5 rel 154045
> 2024-08-29 21:44:32.634 EEST [668327] WARNING: you don't own a lock of type
> AccessExclusiveLock
> 2024-08-29 21:44:32.634 EEST [668327] LOG: RecoveryLockHash contains entry
> for lock no longer recorded by lock manager: xid 996 database 5 relation
> 154045
> TRAP: failed Assert("false"), File: "../src/backend/storage/ipc/standby.c",
> Granted, if you restart the server, it will probably succeed because
> restarting the server will kill all the other queries that were holding
> locks. But yuck.
Agreed.
> So how to improve this? I see a few options:
>
> a) Downgrade the error at startup to a warning, and allow starting the
> standby with smaller settings in standby. At least with a smaller
> max_locks_per_transactions. The other settings also affect the size of
> known-assigned XIDs array, but if the CSN snapshots get committed, that will
> get fixed. In most cases there is enough lock memory anyway, and it will be
> fine. Just fix the assertion failure so that the error message is a little
> nicer.
>
> b) If you run out of lock space, kill running queries, and prevent new ones
> from starting. Track the locks in startup process' private memory until
> there is enough space in the lock manager, and then re-open for queries. In
> essence, go from hot standby mode to warm standby, until it's possible to go
> back to hot standby mode again.
Either seems fine. Having never encountered actual lock exhaustion from this,
I'd lean toward (a) for simplicity.
> Thoughts, better ideas?
I worry about future code assuming a MaxBackends-sized array suffices for
something. That could work almost all the time, breaking only when a standby
replays WAL from a server having a larger array. What could we do now to
catch that future mistake promptly? As a start, 027_stream_regress.pl could
use low settings on its standby.
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: Primary and standby setting cross-checks
@ 2025-03-12 18:09 Kirill Reshke <[email protected]>
parent: Heikki Linnakangas <[email protected]>
1 sibling, 1 reply; 10+ messages in thread
From: Kirill Reshke @ 2025-03-12 18:09 UTC (permalink / raw)
To: Heikki Linnakangas <[email protected]>; +Cc: pgsql-hackers
On Thu, 29 Aug 2024 at 23:52, Heikki Linnakangas <[email protected]> wrote:
>
> Currently, if you configure a hot standby server with a smaller
> max_connections setting than the primary, the server refuses to start up:
>
> LOG: entering standby mode
> FATAL: recovery aborted because of insufficient parameter settings
> DETAIL: max_connections = 10 is a lower setting than on the primary
> server, where its value was 100.
> HINT: You can restart the server after making the necessary
> configuration changes.
>
> Or if you change the setting in the primary while the standby is
> running, replay pauses:
>
> WARNING: hot standby is not possible because of insufficient parameter
> settings
> DETAIL: max_connections = 100 is a lower setting than on the primary
> server, where its value was 200.
> CONTEXT: WAL redo at 2/E10000D8 for XLOG/PARAMETER_CHANGE:
> max_connections=200 max_worker_processes=8 max_wal_senders=10
> max_prepared_xacts=0 max_locks_per_xact=64 wal_level=logical
> wal_log_hints=off track_commit_timestamp=off
> LOG: recovery has paused
> DETAIL: If recovery is unpaused, the server will shut down.
> HINT: You can then restart the server after making the necessary
> configuration changes.
> CONTEXT: WAL redo at 2/E10000D8 for XLOG/PARAMETER_CHANGE:
> max_connections=200 max_worker_processes=8 max_wal_senders=10
> max_prepared_xacts=0 max_locks_per_xact=64 wal_level=logical
> wal_log_hints=off track_commit_timestamp=off
>
> Both of these are rather unpleasant behavior.
>
> I thought I could get rid of that limitation with my CSN snapshot patch
> [1], because it gets rid of the fixed-size known-assigned XIDs array,
> but there's a second reason for these limitations. It's also used to
> ensure that the standby has enough space in the lock manager to hold
> possible AccessExclusiveLocks taken by transactions in the primary.
>
> So firstly, I think that's a bad tradeoff. In vast majority of cases,
> you would not run out of lock space anyway, if you just started up the
> system. Secondly, that cross-check of settings doesn't fully prevent the
> problem. It ensures that the lock tables are large enough to accommodate
> all the locks you could possibly hold in the primary, but that doesn't
> take into account any additional locks held by read-only queries in the
> hot standby. So if you have queries running in the standby that take a
> lot of locks, this can happen anyway:
>
> 2024-08-29 21:44:32.634 EEST [668327] FATAL: out of shared memory
> 2024-08-29 21:44:32.634 EEST [668327] HINT: You might need to increase
> "max_locks_per_transaction".
> 2024-08-29 21:44:32.634 EEST [668327] CONTEXT: WAL redo at 2/FD40FCC8
> for Standby/LOCK: xid 996 db 5 rel 154045
> 2024-08-29 21:44:32.634 EEST [668327] WARNING: you don't own a lock of
> type AccessExclusiveLock
> 2024-08-29 21:44:32.634 EEST [668327] LOG: RecoveryLockHash contains
> entry for lock no longer recorded by lock manager: xid 996 database 5
> relation 154045
> TRAP: failed Assert("false"), File:
> "../src/backend/storage/ipc/standby.c", Line: 1053, PID: 668327
> postgres: startup recovering
> 0000000100000002000000FD(ExceptionalCondition+0x6e)[0x556a4588396e]
> postgres: startup recovering
> 0000000100000002000000FD(+0x44156e)[0x556a4571356e]
> postgres: startup recovering
> 0000000100000002000000FD(StandbyReleaseAllLocks+0x78)[0x556a45712738]
> postgres: startup recovering
> 0000000100000002000000FD(ShutdownRecoveryTransactionEnvironment+0x15)[0x556a45712685]
> postgres: startup recovering
> 0000000100000002000000FD(shmem_exit+0x111)[0x556a457062e1]
> postgres: startup recovering
> 0000000100000002000000FD(+0x434132)[0x556a45706132]
> postgres: startup recovering
> 0000000100000002000000FD(proc_exit+0x59)[0x556a45706079]
> postgres: startup recovering
> 0000000100000002000000FD(errfinish+0x278)[0x556a45884708]
> postgres: startup recovering
> 0000000100000002000000FD(LockAcquireExtended+0xa46)[0x556a45719386]
> postgres: startup recovering
> 0000000100000002000000FD(StandbyAcquireAccessExclusiveLock+0x11d)[0x556a4571330d]
> postgres: startup recovering
> 0000000100000002000000FD(standby_redo+0x70)[0x556a45713690]
> postgres: startup recovering
> 0000000100000002000000FD(PerformWalRecovery+0x7b3)[0x556a4547d313]
> postgres: startup recovering
> 0000000100000002000000FD(StartupXLOG+0xac3)[0x556a4546dae3]
> postgres: startup recovering
> 0000000100000002000000FD(StartupProcessMain+0xe8)[0x556a45693558]
> postgres: startup recovering
> 0000000100000002000000FD(+0x3ba95d)[0x556a4568c95d]
> postgres: startup recovering
> 0000000100000002000000FD(+0x3bce41)[0x556a4568ee41]
> postgres: startup recovering
> 0000000100000002000000FD(PostmasterMain+0x116e)[0x556a4568eaae]
> postgres: startup recovering
> 0000000100000002000000FD(+0x2f960e)[0x556a455cb60e]
> /lib/x86_64-linux-gnu/libc.so.6(+0x27c8a)[0x7f10ef042c8a]
> /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0x85)[0x7f10ef042d45]
> postgres: startup recovering
> 0000000100000002000000FD(_start+0x21)[0x556a453af011]
> 2024-08-29 21:44:32.641 EEST [668324] LOG: startup process (PID 668327)
> was terminated by signal 6: Aborted
> 2024-08-29 21:44:32.641 EEST [668324] LOG: terminating any other active
> server processes
> 2024-08-29 21:44:32.654 EEST [668324] LOG: shutting down due to startup
> process failure
> 2024-08-29 21:44:32.729 EEST [668324] LOG: database system is shut down
>
> Granted, if you restart the server, it will probably succeed because
> restarting the server will kill all the other queries that were holding
> locks. But yuck. With assertions disabled, it looks a little less scary,
> but not nice anyway.
>
> So how to improve this? I see a few options:
>
> a) Downgrade the error at startup to a warning, and allow starting the
> standby with smaller settings in standby. At least with a smaller
> max_locks_per_transactions. The other settings also affect the size of
> known-assigned XIDs array, but if the CSN snapshots get committed, that
> will get fixed. In most cases there is enough lock memory anyway, and it
> will be fine. Just fix the assertion failure so that the error message
> is a little nicer.
>
> b) If you run out of lock space, kill running queries, and prevent new
> ones from starting. Track the locks in startup process' private memory
> until there is enough space in the lock manager, and then re-open for
> queries. In essence, go from hot standby mode to warm standby, until
> it's possible to go back to hot standby mode again.
>
> Thoughts, better ideas?
>
> [1] https://commitfest.postgresql.org/49/4912/
>
> --
> Heikki Linnakangas
> Neon (https://neon.tech)
>
>
Hello! Do you intend to pursue this further?
--
Best regards,
Kirill Reshke
^ permalink raw reply [nested|flat] 10+ messages in thread
* [PATCH] Avoid unnecessary code execution in Instrument.c when TIMING is FALSE
@ 2025-07-25 15:26 Hironobu SUZUKI <[email protected]>
0 siblings, 1 reply; 10+ messages in thread
From: Hironobu SUZUKI @ 2025-07-25 15:26 UTC (permalink / raw)
To: pgsql-hackers
Hi,
Even when using EXPLAIN ANALYZE with TIMING=FALSE, the functions
InstrStopNode(), InstrEndLoop(), and InstrAggNode() in Instrument.c
still execute code related to the "starttime", "counter", "firsttuple",
"startup", and "total" fields within the Instrumentation structure.
These operations are unnecessary when timing is disabled, and since
these functions are called very frequently, I have created a patch to
address this.
As far as I can tell, this change has no side effects and clarifies the
intent of each line, but please let me know if you notice any issues.
Best regards,
H.S.
diff --git a/src/backend/executor/instrument.c b/src/backend/executor/instrument.c
index 56e635f4700..e4274a28525 100644
--- a/src/backend/executor/instrument.c
+++ b/src/backend/executor/instrument.c
@@ -114,7 +114,8 @@ InstrStopNode(Instrumentation *instr, double nTuples)
if (!instr->running)
{
instr->running = true;
- instr->firsttuple = INSTR_TIME_GET_DOUBLE(instr->counter);
+ if (instr->need_timer)
+ instr->firsttuple = INSTR_TIME_GET_DOUBLE(instr->counter);
}
else
{
@@ -122,7 +123,7 @@ InstrStopNode(Instrumentation *instr, double nTuples)
* In async mode, if the plan node hadn't emitted any tuples before,
* this might be the first tuple
*/
- if (instr->async_mode && save_tuplecount < 1.0)
+ if (instr->need_timer && instr->async_mode && save_tuplecount < 1.0)
instr->firsttuple = INSTR_TIME_GET_DOUBLE(instr->counter);
}
}
@@ -149,18 +150,24 @@ InstrEndLoop(Instrumentation *instr)
elog(ERROR, "InstrEndLoop called on running node");
/* Accumulate per-cycle statistics into totals */
- totaltime = INSTR_TIME_GET_DOUBLE(instr->counter);
+ if (instr->need_timer)
+ {
+ totaltime = INSTR_TIME_GET_DOUBLE(instr->counter);
- instr->startup += instr->firsttuple;
- instr->total += totaltime;
+ instr->startup += instr->firsttuple;
+ instr->total += totaltime;
+ }
instr->ntuples += instr->tuplecount;
instr->nloops += 1;
/* Reset for next cycle (if any) */
instr->running = false;
- INSTR_TIME_SET_ZERO(instr->starttime);
- INSTR_TIME_SET_ZERO(instr->counter);
- instr->firsttuple = 0;
+ if (instr->need_timer)
+ {
+ INSTR_TIME_SET_ZERO(instr->starttime);
+ INSTR_TIME_SET_ZERO(instr->counter);
+ instr->firsttuple = 0;
+ }
instr->tuplecount = 0;
}
@@ -171,16 +178,21 @@ InstrAggNode(Instrumentation *dst, Instrumentation *add)
if (!dst->running && add->running)
{
dst->running = true;
- dst->firsttuple = add->firsttuple;
+ if (dst->need_timer && add->need_timer)
+ dst->firsttuple = add->firsttuple;
}
- else if (dst->running && add->running && dst->firsttuple > add->firsttuple)
+ else if (dst->need_timer && add->need_timer && dst->running && add->running
+ && dst->firsttuple > add->firsttuple)
dst->firsttuple = add->firsttuple;
- INSTR_TIME_ADD(dst->counter, add->counter);
+ if (dst->need_timer && add->need_timer)
+ {
+ INSTR_TIME_ADD(dst->counter, add->counter);
+ dst->startup += add->startup;
+ dst->total += add->total;
+ }
dst->tuplecount += add->tuplecount;
- dst->startup += add->startup;
- dst->total += add->total;
dst->ntuples += add->ntuples;
dst->ntuples2 += add->ntuples2;
dst->nloops += add->nloops;
Attachments:
[text/plain] v1_avoid_unnecessary_code_execution_in_instrumentation.patch (2.6K, ../../[email protected]/2-v1_avoid_unnecessary_code_execution_in_instrumentation.patch)
download | inline diff:
diff --git a/src/backend/executor/instrument.c b/src/backend/executor/instrument.c
index 56e635f4700..e4274a28525 100644
--- a/src/backend/executor/instrument.c
+++ b/src/backend/executor/instrument.c
@@ -114,7 +114,8 @@ InstrStopNode(Instrumentation *instr, double nTuples)
if (!instr->running)
{
instr->running = true;
- instr->firsttuple = INSTR_TIME_GET_DOUBLE(instr->counter);
+ if (instr->need_timer)
+ instr->firsttuple = INSTR_TIME_GET_DOUBLE(instr->counter);
}
else
{
@@ -122,7 +123,7 @@ InstrStopNode(Instrumentation *instr, double nTuples)
* In async mode, if the plan node hadn't emitted any tuples before,
* this might be the first tuple
*/
- if (instr->async_mode && save_tuplecount < 1.0)
+ if (instr->need_timer && instr->async_mode && save_tuplecount < 1.0)
instr->firsttuple = INSTR_TIME_GET_DOUBLE(instr->counter);
}
}
@@ -149,18 +150,24 @@ InstrEndLoop(Instrumentation *instr)
elog(ERROR, "InstrEndLoop called on running node");
/* Accumulate per-cycle statistics into totals */
- totaltime = INSTR_TIME_GET_DOUBLE(instr->counter);
+ if (instr->need_timer)
+ {
+ totaltime = INSTR_TIME_GET_DOUBLE(instr->counter);
- instr->startup += instr->firsttuple;
- instr->total += totaltime;
+ instr->startup += instr->firsttuple;
+ instr->total += totaltime;
+ }
instr->ntuples += instr->tuplecount;
instr->nloops += 1;
/* Reset for next cycle (if any) */
instr->running = false;
- INSTR_TIME_SET_ZERO(instr->starttime);
- INSTR_TIME_SET_ZERO(instr->counter);
- instr->firsttuple = 0;
+ if (instr->need_timer)
+ {
+ INSTR_TIME_SET_ZERO(instr->starttime);
+ INSTR_TIME_SET_ZERO(instr->counter);
+ instr->firsttuple = 0;
+ }
instr->tuplecount = 0;
}
@@ -171,16 +178,21 @@ InstrAggNode(Instrumentation *dst, Instrumentation *add)
if (!dst->running && add->running)
{
dst->running = true;
- dst->firsttuple = add->firsttuple;
+ if (dst->need_timer && add->need_timer)
+ dst->firsttuple = add->firsttuple;
}
- else if (dst->running && add->running && dst->firsttuple > add->firsttuple)
+ else if (dst->need_timer && add->need_timer && dst->running && add->running
+ && dst->firsttuple > add->firsttuple)
dst->firsttuple = add->firsttuple;
- INSTR_TIME_ADD(dst->counter, add->counter);
+ if (dst->need_timer && add->need_timer)
+ {
+ INSTR_TIME_ADD(dst->counter, add->counter);
+ dst->startup += add->startup;
+ dst->total += add->total;
+ }
dst->tuplecount += add->tuplecount;
- dst->startup += add->startup;
- dst->total += add->total;
dst->ntuples += add->ntuples;
dst->ntuples2 += add->ntuples2;
dst->nloops += add->nloops;
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: [PATCH] Avoid unnecessary code execution in Instrument.c when TIMING is FALSE
@ 2025-07-26 00:29 Michael Paquier <[email protected]>
parent: Hironobu SUZUKI <[email protected]>
0 siblings, 1 reply; 10+ messages in thread
From: Michael Paquier @ 2025-07-26 00:29 UTC (permalink / raw)
To: Hironobu SUZUKI <[email protected]>; +Cc: pgsql-hackers
On Sat, Jul 26, 2025 at 12:26:21AM +0900, Hironobu SUZUKI wrote:
> Even when using EXPLAIN ANALYZE with TIMING=FALSE, the functions
> InstrStopNode(), InstrEndLoop(), and InstrAggNode() in Instrument.c still
> execute code related to the "starttime", "counter", "firsttuple", "startup",
> and "total" fields within the Instrumentation structure.
> These operations are unnecessary when timing is disabled, and since these
> functions are called very frequently, I have created a patch to address
> this.
>
> As far as I can tell, this change has no side effects and clarifies the
> intent of each line, but please let me know if you notice any issues.
Spoiler: this has been discussed during a meetup attended by most of
the PostgreSQL hackers based in Tokyo and surroundings on the 18th of
July, where Suzuki-san has noticed that the work done by the backend
was pointless when using the instrumentation while hacking on an
extension that relied at least on the explain hook. One of the
remarks was that this seemed worth a submission to upstream when
timers are disabled. The performance really took a hit when the timer
was disabled, making the extension do a lot of unnecessary work for
nothing.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: [PATCH] Avoid unnecessary code execution in Instrument.c when TIMING is FALSE
@ 2025-07-26 08:16 Hironobu SUZUKI <[email protected]>
parent: Michael Paquier <[email protected]>
0 siblings, 0 replies; 10+ messages in thread
From: Hironobu SUZUKI @ 2025-07-26 08:16 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; +Cc: pgsql-hackers
On 2025/07/26 9:29, Michael Paquier wrote:
> On Sat, Jul 26, 2025 at 12:26:21AM +0900, Hironobu SUZUKI wrote:
>> Even when using EXPLAIN ANALYZE with TIMING=FALSE, the functions
>> InstrStopNode(), InstrEndLoop(), and InstrAggNode() in Instrument.c still
>> execute code related to the "starttime", "counter", "firsttuple", "startup",
>> and "total" fields within the Instrumentation structure.
>> These operations are unnecessary when timing is disabled, and since these
>> functions are called very frequently, I have created a patch to address
>> this.
>>
>> As far as I can tell, this change has no side effects and clarifies the
>> intent of each line, but please let me know if you notice any issues.
>
> Spoiler: this has been discussed during a meetup attended by most of
> the PostgreSQL hackers based in Tokyo and surroundings on the 18th of
> July, where Suzuki-san has noticed that the work done by the backend
> was pointless when using the instrumentation while hacking on an
> extension that relied at least on the explain hook. One of the
> remarks was that this seemed worth a submission to upstream when
> timers are disabled. The performance really took a hit when the timer
> was disabled, making the extension do a lot of unnecessary work for
> nothing.
> --
> Michael
Thanks for the spoilers!
Actually, I was confused while reading the Instrument module’s source
code since some sections ran even when timing was disabled, unlike
other parts guarded by if statements.
This patch mainly clarifies the coding intent, so I intentionally avoided
adding comments.
(The essential improvement proposals for internal monitoring will be
presented on another occasion.)
Best,
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: Primary and standby setting cross-checks
@ 2026-02-28 08:26 James Pang <[email protected]>
parent: Kirill Reshke <[email protected]>
0 siblings, 0 replies; 10+ messages in thread
From: James Pang @ 2026-02-28 08:26 UTC (permalink / raw)
To: Kirill Reshke <[email protected]>; +Cc: Heikki Linnakangas <[email protected]>; pgsql-hackers
May I know the progress? sometimes, standby only offload some query and
with lower cpu and ram. warning is flexible instead of fatal ? We
track_commit_timestamp =on and max_worker_process, wal_level=logical
change and restart primary, it lead to standby failed and recovery from
last restart point that lead to another issue, because old transaction
before turn on track_commit_timestamp, and failed in get that transaction
commit_ts.
FATAL: recovery aborted because of insufficient parameter settings
DETAIL: max_worker_processes = 10 is a lower setting than on the primary
server, where its value was 30.
HINT: You can restart the server after making the necessary configuration
changes.
CONTEXT: WAL redo at 1F4F/xxxxxxxx for XLOG/PARAMETER_CHANGE:
max_connections=xxx max_worker_processes=30 max_wal_sen
LOG: startup process (PID xxx) exited with exit code 1
LOG: terminating any other active server processes
LOG: shutting down due to startup process failure
LOG: database system is shut down
LOG: starting PostgreSQL 14.18 on aarch64-unknown-linux-gnu, compiled by
gcc (GCC) 7.3.1 20180712
Thanks,
James
Kirill Reshke <[email protected]> 於 2025年3月13日週四 上午2:09寫道：
> On Thu, 29 Aug 2024 at 23:52, Heikki Linnakangas <[email protected]> wrote:
> >
> > Currently, if you configure a hot standby server with a smaller
> > max_connections setting than the primary, the server refuses to start up:
> >
> > LOG: entering standby mode
> > FATAL: recovery aborted because of insufficient parameter settings
> > DETAIL: max_connections = 10 is a lower setting than on the primary
> > server, where its value was 100.
> > HINT: You can restart the server after making the necessary
> > configuration changes.
> >
> > Or if you change the setting in the primary while the standby is
> > running, replay pauses:
> >
> > WARNING: hot standby is not possible because of insufficient parameter
> > settings
> > DETAIL: max_connections = 100 is a lower setting than on the primary
> > server, where its value was 200.
> > CONTEXT: WAL redo at 2/E10000D8 for XLOG/PARAMETER_CHANGE:
> > max_connections=200 max_worker_processes=8 max_wal_senders=10
> > max_prepared_xacts=0 max_locks_per_xact=64 wal_level=logical
> > wal_log_hints=off track_commit_timestamp=off
> > LOG: recovery has paused
> > DETAIL: If recovery is unpaused, the server will shut down.
> > HINT: You can then restart the server after making the necessary
> > configuration changes.
> > CONTEXT: WAL redo at 2/E10000D8 for XLOG/PARAMETER_CHANGE:
> > max_connections=200 max_worker_processes=8 max_wal_senders=10
> > max_prepared_xacts=0 max_locks_per_xact=64 wal_level=logical
> > wal_log_hints=off track_commit_timestamp=off
> >
> > Both of these are rather unpleasant behavior.
> >
> > I thought I could get rid of that limitation with my CSN snapshot patch
> > [1], because it gets rid of the fixed-size known-assigned XIDs array,
> > but there's a second reason for these limitations. It's also used to
> > ensure that the standby has enough space in the lock manager to hold
> > possible AccessExclusiveLocks taken by transactions in the primary.
> >
> > So firstly, I think that's a bad tradeoff. In vast majority of cases,
> > you would not run out of lock space anyway, if you just started up the
> > system. Secondly, that cross-check of settings doesn't fully prevent the
> > problem. It ensures that the lock tables are large enough to accommodate
> > all the locks you could possibly hold in the primary, but that doesn't
> > take into account any additional locks held by read-only queries in the
> > hot standby. So if you have queries running in the standby that take a
> > lot of locks, this can happen anyway:
> >
> > 2024-08-29 21:44:32.634 EEST [668327] FATAL: out of shared memory
> > 2024-08-29 21:44:32.634 EEST [668327] HINT: You might need to increase
> > "max_locks_per_transaction".
> > 2024-08-29 21:44:32.634 EEST [668327] CONTEXT: WAL redo at 2/FD40FCC8
> > for Standby/LOCK: xid 996 db 5 rel 154045
> > 2024-08-29 21:44:32.634 EEST [668327] WARNING: you don't own a lock of
> > type AccessExclusiveLock
> > 2024-08-29 21:44:32.634 EEST [668327] LOG: RecoveryLockHash contains
> > entry for lock no longer recorded by lock manager: xid 996 database 5
> > relation 154045
> > TRAP: failed Assert("false"), File:
> > "../src/backend/storage/ipc/standby.c", Line: 1053, PID: 668327
> > postgres: startup recovering
> > 0000000100000002000000FD(ExceptionalCondition+0x6e)[0x556a4588396e]
> > postgres: startup recovering
> > 0000000100000002000000FD(+0x44156e)[0x556a4571356e]
> > postgres: startup recovering
> > 0000000100000002000000FD(StandbyReleaseAllLocks+0x78)[0x556a45712738]
> > postgres: startup recovering
> >
> 0000000100000002000000FD(ShutdownRecoveryTransactionEnvironment+0x15)[0x556a45712685]
> > postgres: startup recovering
> > 0000000100000002000000FD(shmem_exit+0x111)[0x556a457062e1]
> > postgres: startup recovering
> > 0000000100000002000000FD(+0x434132)[0x556a45706132]
> > postgres: startup recovering
> > 0000000100000002000000FD(proc_exit+0x59)[0x556a45706079]
> > postgres: startup recovering
> > 0000000100000002000000FD(errfinish+0x278)[0x556a45884708]
> > postgres: startup recovering
> > 0000000100000002000000FD(LockAcquireExtended+0xa46)[0x556a45719386]
> > postgres: startup recovering
> >
> 0000000100000002000000FD(StandbyAcquireAccessExclusiveLock+0x11d)[0x556a4571330d]
> > postgres: startup recovering
> > 0000000100000002000000FD(standby_redo+0x70)[0x556a45713690]
> > postgres: startup recovering
> > 0000000100000002000000FD(PerformWalRecovery+0x7b3)[0x556a4547d313]
> > postgres: startup recovering
> > 0000000100000002000000FD(StartupXLOG+0xac3)[0x556a4546dae3]
> > postgres: startup recovering
> > 0000000100000002000000FD(StartupProcessMain+0xe8)[0x556a45693558]
> > postgres: startup recovering
> > 0000000100000002000000FD(+0x3ba95d)[0x556a4568c95d]
> > postgres: startup recovering
> > 0000000100000002000000FD(+0x3bce41)[0x556a4568ee41]
> > postgres: startup recovering
> > 0000000100000002000000FD(PostmasterMain+0x116e)[0x556a4568eaae]
> > postgres: startup recovering
> > 0000000100000002000000FD(+0x2f960e)[0x556a455cb60e]
> > /lib/x86_64-linux-gnu/libc.so.6(+0x27c8a)[0x7f10ef042c8a]
> > /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0x85)[0x7f10ef042d45]
> > postgres: startup recovering
> > 0000000100000002000000FD(_start+0x21)[0x556a453af011]
> > 2024-08-29 21:44:32.641 EEST [668324] LOG: startup process (PID 668327)
> > was terminated by signal 6: Aborted
> > 2024-08-29 21:44:32.641 EEST [668324] LOG: terminating any other active
> > server processes
> > 2024-08-29 21:44:32.654 EEST [668324] LOG: shutting down due to startup
> > process failure
> > 2024-08-29 21:44:32.729 EEST [668324] LOG: database system is shut down
> >
> > Granted, if you restart the server, it will probably succeed because
> > restarting the server will kill all the other queries that were holding
> > locks. But yuck. With assertions disabled, it looks a little less scary,
> > but not nice anyway.
> >
> > So how to improve this? I see a few options:
> >
> > a) Downgrade the error at startup to a warning, and allow starting the
> > standby with smaller settings in standby. At least with a smaller
> > max_locks_per_transactions. The other settings also affect the size of
> > known-assigned XIDs array, but if the CSN snapshots get committed, that
> > will get fixed. In most cases there is enough lock memory anyway, and it
> > will be fine. Just fix the assertion failure so that the error message
> > is a little nicer.
> >
> > b) If you run out of lock space, kill running queries, and prevent new
> > ones from starting. Track the locks in startup process' private memory
> > until there is enough space in the lock manager, and then re-open for
> > queries. In essence, go from hot standby mode to warm standby, until
> > it's possible to go back to hot standby mode again.
> >
> > Thoughts, better ideas?
> >
> > [1] https://commitfest.postgresql.org/49/4912/
> >
> > --
> > Heikki Linnakangas
> > Neon (https://neon.tech)
> >
> >
>
> Hello! Do you intend to pursue this further?
>
> --
> Best regards,
> Kirill Reshke
>
>
>
^ permalink raw reply [nested|flat] 10+ messages in thread
end of thread, other threads:[~2026-02-28 08:26 UTC | newest]
Thread overview: 10+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2021-03-08 22:59 [PATCH v3 05/17] meson: prereq: move snowball_create.sql creation into perl file. Andres Freund <[email protected]>
2021-03-08 22:59 [PATCH v5 05/16] meson: prereq: move snowball_create.sql creation into perl file. Andres Freund <[email protected]>
2022-01-20 07:36 [PATCH v10 04/16] meson: prereq: Move snowball_create.sql creation into perl file Andres Freund <[email protected]>
2024-08-29 18:52 Primary and standby setting cross-checks Heikki Linnakangas <[email protected]>
2024-09-25 03:03 ` Re: Primary and standby setting cross-checks Noah Misch <[email protected]>
2025-03-12 18:09 ` Re: Primary and standby setting cross-checks Kirill Reshke <[email protected]>
2026-02-28 08:26 ` Re: Primary and standby setting cross-checks James Pang <[email protected]>
2025-07-25 15:26 [PATCH] Avoid unnecessary code execution in Instrument.c when TIMING is FALSE Hironobu SUZUKI <[email protected]>
2025-07-26 00:29 ` Re: [PATCH] Avoid unnecessary code execution in Instrument.c when TIMING is FALSE Michael Paquier <[email protected]>
2025-07-26 08:16 ` Re: [PATCH] Avoid unnecessary code execution in Instrument.c when TIMING is FALSE Hironobu SUZUKI <[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