public inbox for [email protected]help / color / mirror / Atom feed
[PATCH] Fix timeline-tracking failure while sending a historic timeline 7+ messages / 4 participants [nested] [flat]
* [PATCH] Fix timeline-tracking failure while sending a historic timeline @ 2021-01-05 04:34 Kyotaro Horiguchi <[email protected]> 0 siblings, 0 replies; 7+ messages in thread From: Kyotaro Horiguchi @ 2021-01-05 04:34 UTC (permalink / raw) Walsender should track timeline switches while sending a historic timeline. Regain that behavior, which was broken in PG13, by a thinko of 709d003fbd. Backpatch to PG13. --- src/backend/replication/walsender.c | 2 +- src/test/perl/PostgresNode.pm | 33 ++++++++++++++++++ src/test/perl/TestLib.pm | 16 ++++++--- src/test/recovery/t/001_stream_rep.pl | 41 ++++++++++++++++++++++- src/test/recovery/t/019_replslot_limit.pl | 37 ++++---------------- 5 files changed, 92 insertions(+), 37 deletions(-) diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c index 7f87eb7f19..04f6c3ebb4 100644 --- a/src/backend/replication/walsender.c +++ b/src/backend/replication/walsender.c @@ -2478,7 +2478,7 @@ WalSndSegmentOpen(XLogReaderState *state, XLogSegNo nextSegNo, XLogSegNo endSegNo; XLByteToSeg(sendTimeLineValidUpto, endSegNo, state->segcxt.ws_segsize); - if (state->seg.ws_segno == endSegNo) + if (nextSegNo == endSegNo) *tli_p = sendTimeLineNextTLI; } diff --git a/src/test/perl/PostgresNode.pm b/src/test/perl/PostgresNode.pm index 980f1f1533..687aa3ac88 100644 --- a/src/test/perl/PostgresNode.pm +++ b/src/test/perl/PostgresNode.pm @@ -2138,6 +2138,39 @@ sub pg_recvlogical_upto =pod +=item $node->current_log_position() + +Return the current position of server log. + +=cut + +sub current_log_position +{ + my $self = shift; + + return (stat $self->logfile)[7]; +} + +=pod + +=item $node->find_in_log($pattern, $startpos) + +Returns whether the $pattern occurs after $startpos in the server log. + +=cut + +sub find_in_log +{ + my ($self, $pattern, $startpos) = @_; + + $startpos = 0 unless defined $startpos; + my $log = TestLib::slurp_file($self->logfile, $startpos); + + return $log =~ m/$pattern/; +} + +=pod + =back =cut diff --git a/src/test/perl/TestLib.pm b/src/test/perl/TestLib.pm index a7490d2ce7..a0ce9521e2 100644 --- a/src/test/perl/TestLib.pm +++ b/src/test/perl/TestLib.pm @@ -402,30 +402,38 @@ sub slurp_dir =pod -=item slurp_file(filename) +=item slurp_file(filename, pos) -Return the full contents of the specified file. +Return the contents after pos of the specified file. +Reutrns the full contents if pos is omitted. =cut sub slurp_file { - my ($filename) = @_; + my ($filename, $from) = @_; local $/; my $contents; + + $from = 0 unless defined $from; + if ($Config{osname} ne 'MSWin32') { open(my $in, '<', $filename) or die "could not read \"$filename\": $!"; + seek($in, $from, 0) + or die "could not seek \"$filename\" to $from: $!"; $contents = <$in>; close $in; } else { my $fHandle = createFile($filename, "r", "rwd") - or die "could not open \"$filename\": $^E"; + or die "could not open \"$filename\": $^E\n"; OsFHandleOpen(my $fh = IO::Handle->new(), $fHandle, 'r') or die "could not read \"$filename\": $^E\n"; + seek($fh, $from, 0) + or die "could not seek \"$filename\" to $from: $^E\n"; $contents = <$fh>; CloseHandle($fHandle) or die "could not close \"$filename\": $^E\n"; diff --git a/src/test/recovery/t/001_stream_rep.pl b/src/test/recovery/t/001_stream_rep.pl index 778f11b28b..8d2b24fe55 100644 --- a/src/test/recovery/t/001_stream_rep.pl +++ b/src/test/recovery/t/001_stream_rep.pl @@ -2,8 +2,9 @@ use strict; use warnings; use PostgresNode; +use Time::HiRes qw(usleep); use TestLib; -use Test::More tests => 36; +use Test::More tests => 37; # Initialize master node my $node_master = get_new_node('master'); @@ -409,3 +410,41 @@ ok( ($phys_restart_lsn_pre cmp $phys_restart_lsn_post) == 0, my $master_data = $node_master->data_dir; ok(!-f "$master_data/pg_wal/$segment_removed", "WAL segment $segment_removed recycled after physical slot advancing"); + +# +# Check if timeline-increment works while reading a historic timeline. +my $node_primary_2 = get_new_node('primary_2'); +# archiving is needed to create .paritial segment +$node_primary_2->init(allows_streaming => 1, has_archiving => 1); +$node_primary_2->start; +$node_primary_2->backup($backup_name); +my $node_standby_3 = get_new_node('standby_3'); +$node_standby_3->init_from_backup($node_primary_2, $backup_name, + has_streaming => 1); +$node_primary_2->stop; +$node_primary_2->set_standby_mode; # increment primary timeline +$node_primary_2->start; +$node_primary_2->promote; +my $logstart = $node_standby_3->current_log_position(); +$node_standby_3->start; + +my $success = 0; +for (my $i = 0 ; $i < 1000; $i++) +{ + if ($node_standby_3->find_in_log( + "requested WAL segment [0-9A-F]+ has already been removed", + $logstart)) + { + last; + } + elsif ($node_standby_3->find_in_log( + "End of WAL reached on timeline", + $logstart)) + { + $success = 1; + last; + } + usleep(100_000); +} + +ok($success, 'Timeline increment while reading a historic timeline'); diff --git a/src/test/recovery/t/019_replslot_limit.pl b/src/test/recovery/t/019_replslot_limit.pl index a7231dcd47..8b3c5de057 100644 --- a/src/test/recovery/t/019_replslot_limit.pl +++ b/src/test/recovery/t/019_replslot_limit.pl @@ -165,19 +165,17 @@ $node_master->wait_for_catchup($node_standby, 'replay', $start_lsn); $node_standby->stop; -ok( !find_in_log( - $node_standby, - "requested WAL segment [0-9A-F]+ has already been removed"), +ok( !$node_standby->find_in_log( + "requested WAL segment [0-9A-F]+ has already been removed"), 'check that required WAL segments are still available'); # Advance WAL again, the slot loses the oldest segment. -my $logstart = get_log_size($node_master); +my $logstart = $node_master->current_log_position(); advance_wal($node_master, 7); $node_master->safe_psql('postgres', "CHECKPOINT;"); # WARNING should be issued -ok( find_in_log( - $node_master, +ok( $node_master->find_in_log( "invalidating slot \"rep1\" because its restart_lsn [0-9A-F/]+ exceeds max_slot_wal_keep_size", $logstart), 'check that the warning is logged'); @@ -190,14 +188,13 @@ is($result, "rep1|f|t|lost|", 'check that the slot became inactive and the state "lost" persists'); # The standby no longer can connect to the master -$logstart = get_log_size($node_standby); +$logstart = $node_standby->current_log_position(); $node_standby->start; my $failed = 0; for (my $i = 0; $i < 10000; $i++) { - if (find_in_log( - $node_standby, + if ($node_standby->find_in_log( "requested WAL segment [0-9A-F]+ has already been removed", $logstart)) { @@ -264,25 +261,3 @@ sub advance_wal } return; } - -# return the size of logfile of $node in bytes -sub get_log_size -{ - my ($node) = @_; - - return (stat $node->logfile)[7]; -} - -# find $pat in logfile of $node after $off-th byte -sub find_in_log -{ - my ($node, $pat, $off) = @_; - - $off = 0 unless defined $off; - my $log = TestLib::slurp_file($node->logfile); - return 0 if (length($log) <= $off); - - $log = substr($log, $off); - - return $log =~ m/$pat/; -} -- 2.27.0 ----Next_Part(Wed_Jan__6_10_48_04_2021_104)---- ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: run pgindent on a regular basis / scripted manner @ 2023-01-20 17:09 Tom Lane <[email protected]> 0 siblings, 1 reply; 7+ messages in thread From: Tom Lane @ 2023-01-20 17:09 UTC (permalink / raw) To: Jelte Fennema <[email protected]>; +Cc: Magnus Hagander <[email protected]>; Alvaro Herrera <[email protected]>; Stephen Frost <[email protected]>; Noah Misch <[email protected]>; Jesse Zhang <[email protected]>; Andres Freund <[email protected]>; pgsql-hackers Jelte Fennema <[email protected]> writes: > To me a master branch that pgindent never complains about sounds > amazing! And I personally think rejection of unindented pushes and > cfbot complaining about unindented patches would be a very good thing, > because that seems to be the only solution that could achieve that. The core problem here is that requiring that would translate to requiring every code contributor to have a working copy of pg_bsd_indent. Maybe that's not a huge lift, but it'd be YA obstacle to new contributors, and we don't need any more of those. Yeah, if we switched to some other tool maybe we could reduce the size of that problem. But as Bruce replied, we've not found another one that (a) can be coaxed to make output comparable to what we're accustomed to and (b) seems decently well maintained. regards, tom lane ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: run pgindent on a regular basis / scripted manner @ 2023-01-20 17:58 Andres Freund <[email protected]> parent: Tom Lane <[email protected]> 0 siblings, 1 reply; 7+ messages in thread From: Andres Freund @ 2023-01-20 17:58 UTC (permalink / raw) To: Tom Lane <[email protected]>; +Cc: Jelte Fennema <[email protected]>; Magnus Hagander <[email protected]>; Alvaro Herrera <[email protected]>; Stephen Frost <[email protected]>; Noah Misch <[email protected]>; Jesse Zhang <[email protected]>; pgsql-hackers Hi, On 2023-01-20 12:09:05 -0500, Tom Lane wrote: > Jelte Fennema <[email protected]> writes: > > To me a master branch that pgindent never complains about sounds > > amazing! And I personally think rejection of unindented pushes and > > cfbot complaining about unindented patches would be a very good thing, > > because that seems to be the only solution that could achieve that. > > The core problem here is that requiring that would translate to > requiring every code contributor to have a working copy of pg_bsd_indent. Wouldn't just every committer suffice? > Maybe that's not a huge lift, but it'd be YA obstacle to new contributors, > and we don't need any more of those. > > Yeah, if we switched to some other tool maybe we could reduce the size > of that problem. But as Bruce replied, we've not found another one that > (a) can be coaxed to make output comparable to what we're accustomed to > and (b) seems decently well maintained. One question around this is how much change we'd accept. clang-format for example is well maintained and can get somewhat close to our style - but there are things that can't easily be approximated. Greetings, Andres Freund ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: run pgindent on a regular basis / scripted manner @ 2023-01-20 18:19 Tom Lane <[email protected]> parent: Andres Freund <[email protected]> 0 siblings, 1 reply; 7+ messages in thread From: Tom Lane @ 2023-01-20 18:19 UTC (permalink / raw) To: Andres Freund <[email protected]>; +Cc: Jelte Fennema <[email protected]>; Magnus Hagander <[email protected]>; Alvaro Herrera <[email protected]>; Stephen Frost <[email protected]>; Noah Misch <[email protected]>; Jesse Zhang <[email protected]>; pgsql-hackers Andres Freund <[email protected]> writes: > On 2023-01-20 12:09:05 -0500, Tom Lane wrote: >> The core problem here is that requiring that would translate to >> requiring every code contributor to have a working copy of pg_bsd_indent. > Wouldn't just every committer suffice? Not if we have cfbot complaining about it. (Another problem here is that there's a sizable subset of committers who clearly just don't care, and I'm not sure we can convince them to.) >> Yeah, if we switched to some other tool maybe we could reduce the size >> of that problem. But as Bruce replied, we've not found another one that >> (a) can be coaxed to make output comparable to what we're accustomed to >> and (b) seems decently well maintained. > One question around this is how much change we'd accept. clang-format for > example is well maintained and can get somewhat close to our style - but > there are things that can't easily be approximated. If somebody wants to invest the effort in seeing how close we can get and what the remaining delta would look like, we could have a discussion about whether it's an acceptable change. I don't think anyone has tried with clang-format. regards, tom lane ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: run pgindent on a regular basis / scripted manner @ 2023-01-21 13:26 Andrew Dunstan <[email protected]> parent: Tom Lane <[email protected]> 0 siblings, 1 reply; 7+ messages in thread From: Andrew Dunstan @ 2023-01-21 13:26 UTC (permalink / raw) To: [email protected] On 2023-01-20 Fr 13:19, Tom Lane wrote: > Andres Freund <[email protected]> writes: >> On 2023-01-20 12:09:05 -0500, Tom Lane wrote: >>> The core problem here is that requiring that would translate to >>> requiring every code contributor to have a working copy of pg_bsd_indent. >> Wouldn't just every committer suffice? > Not if we have cfbot complaining about it. > > (Another problem here is that there's a sizable subset of committers > who clearly just don't care, and I'm not sure we can convince them to.) I think we could do better with some automation tooling for committers here. One low-risk and simple change would be to provide a non-destructive mode for pgindent that would show you the changes if any it would make. That could be worked into a git pre-commit hook that committers could deploy. I can testify to the usefulness of such hooks - I have one that while not perfect has saved me on at least two occasions from forgetting to bump the catalog version. I'll take a look at fleshing this out, for my own if no-one else's use. cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: run pgindent on a regular basis / scripted manner @ 2023-01-21 15:00 Andrew Dunstan <[email protected]> parent: Andrew Dunstan <[email protected]> 0 siblings, 1 reply; 7+ messages in thread From: Andrew Dunstan @ 2023-01-21 15:00 UTC (permalink / raw) To: [email protected] On 2023-01-21 Sa 08:26, Andrew Dunstan wrote: > On 2023-01-20 Fr 13:19, Tom Lane wrote: >> Andres Freund <[email protected]> writes: >>> On 2023-01-20 12:09:05 -0500, Tom Lane wrote: >>>> The core problem here is that requiring that would translate to >>>> requiring every code contributor to have a working copy of pg_bsd_indent. >>> Wouldn't just every committer suffice? >> Not if we have cfbot complaining about it. >> >> (Another problem here is that there's a sizable subset of committers >> who clearly just don't care, and I'm not sure we can convince them to.) > > I think we could do better with some automation tooling for committers > here. One low-risk and simple change would be to provide a > non-destructive mode for pgindent that would show you the changes if any > it would make. That could be worked into a git pre-commit hook that > committers could deploy. I can testify to the usefulness of such hooks - > I have one that while not perfect has saved me on at least two occasions > from forgetting to bump the catalog version. > > I'll take a look at fleshing this out, for my own if no-one else's use. > > Here's a quick patch for this. I have it in mind to use like this in a pre-commit hook: # only do this on master test `git rev-parse --abbrev-ref HEAD` = "master" || exit 0 src/tools/pgindent/pg_indent --silent `git diff --cached --name-only` || \ { echo "Need a pgindent run" >&2 ; exit 1; } The committer could then run src/tools/pgindent/pg_indent --show-diff `git diff --cached --name-only` to see what changes it thinks are needed. cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com ^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: run pgindent on a regular basis / scripted manner @ 2023-01-21 15:02 Andrew Dunstan <[email protected]> parent: Andrew Dunstan <[email protected]> 0 siblings, 0 replies; 7+ messages in thread From: Andrew Dunstan @ 2023-01-21 15:02 UTC (permalink / raw) To: [email protected] On 2023-01-21 Sa 10:00, Andrew Dunstan wrote: > On 2023-01-21 Sa 08:26, Andrew Dunstan wrote: >> On 2023-01-20 Fr 13:19, Tom Lane wrote: >>> Andres Freund <[email protected]> writes: >>>> On 2023-01-20 12:09:05 -0500, Tom Lane wrote: >>>>> The core problem here is that requiring that would translate to >>>>> requiring every code contributor to have a working copy of pg_bsd_indent. >>>> Wouldn't just every committer suffice? >>> Not if we have cfbot complaining about it. >>> >>> (Another problem here is that there's a sizable subset of committers >>> who clearly just don't care, and I'm not sure we can convince them to.) >> I think we could do better with some automation tooling for committers >> here. One low-risk and simple change would be to provide a >> non-destructive mode for pgindent that would show you the changes if any >> it would make. That could be worked into a git pre-commit hook that >> committers could deploy. I can testify to the usefulness of such hooks - >> I have one that while not perfect has saved me on at least two occasions >> from forgetting to bump the catalog version. >> >> I'll take a look at fleshing this out, for my own if no-one else's use. >> >> > Here's a quick patch for this. I have it in mind to use like this in a > pre-commit hook: > > # only do this on master > test `git rev-parse --abbrev-ref HEAD` = "master" || exit 0 > > src/tools/pgindent/pg_indent --silent `git diff --cached --name-only` || \ > > { echo "Need a pgindent run" >&2 ; exit 1; } > > > The committer could then run > > src/tools/pgindent/pg_indent --show-diff `git diff --cached --name-only` > > to see what changes it thinks are needed. > > This time with patch. cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com Attachments: [text/x-patch] pgindent_silent_plus_showdiff.patch (2.5K, ../../[email protected]/2-pgindent_silent_plus_showdiff.patch) download | inline diff: diff --git a/src/tools/pgindent/pgindent b/src/tools/pgindent/pgindent index 741b0ccb58..2834f5c9fc 100755 --- a/src/tools/pgindent/pgindent +++ b/src/tools/pgindent/pgindent @@ -21,7 +21,8 @@ my $indent_opts = my $devnull = File::Spec->devnull; -my ($typedefs_file, $typedef_str, $code_base, $excludes, $indent, $build); +my ($typedefs_file, $typedef_str, $code_base, $excludes, $indent, $build, + $show_diff, $silent_diff); my %options = ( "typedefs=s" => \$typedefs_file, @@ -29,9 +30,14 @@ my %options = ( "code-base=s" => \$code_base, "excludes=s" => \$excludes, "indent=s" => \$indent, - "build" => \$build,); + "build" => \$build, + "show-diff" => \$show_diff, + "silent_diff" => \$silent_diff,); GetOptions(%options) || die "bad command line argument\n"; +die "Cannot have both --silent-diff and --show-diff" + if $silent_diff && $show_diff; + run_build($code_base) if ($build); # command line option wins, then first non-option arg, @@ -283,30 +289,19 @@ sub run_indent } - -# for development diagnostics -sub diff +sub show_diff { - my $pre = shift; - my $post = shift; - my $flags = shift || ""; + my $indented = shift; + my $source_filename = shift; - print STDERR "running diff\n"; - - my $pre_fh = new File::Temp(TEMPLATE => "pgdiffbXXXXX"); my $post_fh = new File::Temp(TEMPLATE => "pgdiffaXXXXX"); - print $pre_fh $pre; - print $post_fh $post; + print $post_fh $indented; - $pre_fh->close(); $post_fh->close(); - system( "diff $flags " - . $pre_fh->filename . " " - . $post_fh->filename - . " >&2"); - return; + my $diff = `diff -u $source_filename $post_fh->filename 2>&1`); + return $diff; } @@ -404,6 +399,8 @@ push(@files, @ARGV); foreach my $source_filename (@files) { + # ignore anything that's not a .c or .h file + next unless $source_filename =~/\.[ch]$/; # Automatically ignore .c and .h files that correspond to a .y or .l # file. indent tends to get badly confused by Bison/flex output, @@ -429,7 +426,24 @@ foreach my $source_filename (@files) $source = post_indent($source, $source_filename); - write_source($source, $source_filename) if $source ne $orig_source; + if ($source ne $orig_source) + { + if ($silent_diff) + { + exit 2; + } + elsif ($show_diff) + { + print show_diff($source, $source_filename); + } + else + { + write_source($source, $source_filename); + } + } + } build_clean($code_base) if $build; + +exit 0; ^ permalink raw reply [nested|flat] 7+ messages in thread
end of thread, other threads:[~2023-01-21 15:02 UTC | newest] Thread overview: 7+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2021-01-05 04:34 [PATCH] Fix timeline-tracking failure while sending a historic timeline Kyotaro Horiguchi <[email protected]> 2023-01-20 17:09 Re: run pgindent on a regular basis / scripted manner Tom Lane <[email protected]> 2023-01-20 17:58 ` Re: run pgindent on a regular basis / scripted manner Andres Freund <[email protected]> 2023-01-20 18:19 ` Re: run pgindent on a regular basis / scripted manner Tom Lane <[email protected]> 2023-01-21 13:26 ` Re: run pgindent on a regular basis / scripted manner Andrew Dunstan <[email protected]> 2023-01-21 15:00 ` Re: run pgindent on a regular basis / scripted manner Andrew Dunstan <[email protected]> 2023-01-21 15:02 ` Re: run pgindent on a regular basis / scripted manner 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