Re: Stopping logical replication protocol

On 9 September 2016 at 03:56, Vladimir Gordiychuk <folyga(at)gmail(dot)com> wrote:
>>It'd helpful if you summarize the changes made when posting revisions.
>
> Can we use as summary about changes first message?

I meant "what did you change between the last patch I posted and the
one you just posted" ?

I'm looking at the revised patch now.

> Sounds good. Is it changes should be include in this PR?

Yes, probably as a 3rd patch on top of these two that adds the
pg_recvlogical tests.

I suggest:

1 and 2: same patches as currently
3: send CopyDone on SIGINT in pg_recvlogical, second SIGINT just
closes per current behaviour
4: add tests using the code in 1-3

When you're editing such a patch stack, sometimes you'll find you want
to change something in patch 1 or 2 or 3 while working on patch 4. The
trick to doing this is to make sure to commit your changes in small
pieces and avoid mixing up changes for different patches. So you'll
land up with lots of small commits like

* "fix typo in ReorderBufferIsActive declaration"
* "forgot to test streamingDoneReceiving in SomeFunction"

etc.

Then when you're ready to send a new patch series, you tag your
current working branch (optional, but makes cleaning up after mistakes
easier), rebase the changes, tag the result, git format-patch it and
push it, e.g.:

git checkout my-branch
git tag my-branch-v4-before-rebase
# non-interactive rebase on top of current Pg git master
git rebase master
# Interactive rebase of all patches on top of master.
git rebase -i master

This will produce an editor window. Here you choose patches to reorder
and squash into existing patches, e.g if you start with:

pick 81b3f61 Respect client-initiated CopyDone in walsender
pick 43de23d Second part of Vladimir Gordiychuk's patch
pick 1231134 fix a typo in part 1
pick 1231111 forgot to check something in part 2
pick a213111 another typo

you might edit it to:

pick 81b3f61 Respect client-initiated CopyDone in walsender
fixup 1231134 fix a typo in part 1
fixup a213111 another typo
pick 43de23d Second part of Vladimir Gordiychuk's patch
fixup 1231111 forgot to check something in part 2

When you save and quit, git will reorder the patches and squash them
for you. If there are merge conflicts you fix them at each stage then
"git rebase --continue".

It takes some getting used to but isn't really hard at all. If you run
into trouble, just push your current working branch to a public tree
(github or whatever) and ask for help here.

Once you're used to it, git has a few helper features like "fixup!"
commits and --auto-squash that make this a bit quicker, but it's worth
doing manually first since it's easier to figure out problems.

There's lots of good reference material on this out there so I won't
go on about it endlessly.

> I'm not ensure that
> I do this before complete this commitefest, I need some time to understand
> how this tests works, and how can i write new one.

No problem. I don't think it matters much if the patch bumps to the
next commitfest. It's not a big intrusive change that needs to get in
early in the development cycle.

I'm not a committer and can't commit this myself. If I were I don't
think I'd do so without a working test anyway.

A good starting point for the tests is src/test/perl/README . You can
add a test to src/test/recovery/t . Follow the model used in the other
tests there.

I wrote some tests using logical decoding in the logical decoding
timeline following patch; see
https://commitfest.postgresql.org/10/779/ for the patch, or
https://github.com/2ndQuadrant/postgres/tree/dev/logical-decoding-timeline-following-pg10-v1
. They use the SQL interface not pg_recvlogical though.

There's also https://github.com/2ndQuadrant/postgres/blob/dev/failover-slots/src/test/recovery/t/008_failover_slots.pl
which has another example of controlling an external process, in this
case pg_receivexlog, using IPC::Run. See start_pg_receivexlog(...) and
its callers.

When writing TAP tests for Perl you must ensure you use only Perl
5.8.8-compatible code and only the core modules plus IPC::Run . You'll
usually be fine if you just avoid importing things you don't see other
modules already using. Don't bother actually installing Perl 5.8.8, we
can run tests in an ancient-perl Docker container before committing
and fix any issues. I don't see any reason you'd need anything special
for this test.

If you think some of the code you add would be very reusable for other
test modules in future, feel free to propose new methods for
PostgresNode.pm . Ask if you're not sure.

Ping me if you're stuck or need a hand. I really want more people to
have a clue about how the TAP tests work. I don't use IRC (wrong
timezone and life is busy) so email is best.

--
Craig Ringer http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services