public inbox for [email protected]  

help / color / mirror / Atom feed

From: [email protected] <[email protected]>
To: Masahiko Sawada <[email protected]>
Cc: vignesh C <[email protected]>
Cc: Greg Nancarrow <[email protected]>
Cc: [email protected] <[email protected]>
Cc: [email protected] <[email protected]>
Cc: Alexey Lesovsky <[email protected]>
Cc: Peter Eisentraut <[email protected]>
Cc: PostgreSQL-development <[email protected]>
Cc: Amit Kapila <[email protected]>
Subject: RE: Skipping logical replication transactions on subscriber side
Date: Thu, 25 Nov 2021 13:05:43 +0000
Message-ID: <OS0PR01MB57169DBC045C7C5609FC7EED94629@OS0PR01MB5716.jpnprd01.prod.outlook.com> (raw)
In-Reply-To: <CAD21AoDwoJkf9_koeiLqCepTZxJgaF1_MB=Pav4qq1DNoPSciw@mail.gmail.com>
References: <CAA4eK1KzZp5YC34exW3Yb4Vk9v5iMW0YuJCQsWeTOeF7ejLdXw@mail.gmail.com>
	<CAD21AoDwoJkf9_koeiLqCepTZxJgaF1_MB=Pav4qq1DNoPSciw@mail.gmail.com>

On Thur, Nov 25, 2021 8:29 PM Masahiko Sawada <[email protected]> wrote:
> On Thu, Nov 25, 2021 at 1:57 PM Amit Kapila <[email protected]> wrote:
> >
> > On Wed, Nov 24, 2021 at 5:14 PM Masahiko Sawada
> <[email protected]> wrote:
> > >
> > > Changed. I've removed first_error_time as per discussion on the
> > > thread for adding xact stats.
> > >
> >
> > We also agreed to change the column names to start with last_error_*
> > [1]. Is there a reason to not make those changes? Do you think that we
> > can change it just before committing that patch? I thought it might be
> > better to do it that way now itself.
> 
> Oh, I thought that you think that we change the column names when adding xact
> stats to the view. But these names also make sense even without the xact stats.
> I've attached an updated patch. It also incorporated comments from Vignesh
> and Greg.
> 
Hi,

I only noticed some minor things in the testcases

1)
+$node_publisher->append_conf('postgresql.conf',
+			     qq[
+logical_decoding_work_mem = 64kB
+]);

It seems we don’t need set the decode_work_mem since we don't test streaming ?

2)
+$node_publisher->safe_psql('postgres',
+			   q[
+CREATE PUBLICATION tap_pub FOR TABLE test_tab1, test_tab2;
+]);

There are a few places where only one command exists in the 'q[' or 'qq[' like the above code.
To be consistent, I think it might be better to remove the wrap here, maybe we can write like:
$node_publisher->safe_psql('postgres',
	' CREATE PUBLICATION tap_pub FOR TABLE test_tab1, test_tab2;');

The others LGTM.

Best regards,
Hou zj

view thread (400+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: RE: Skipping logical replication transactions on subscriber side
  In-Reply-To: <OS0PR01MB57169DBC045C7C5609FC7EED94629@OS0PR01MB5716.jpnprd01.prod.outlook.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox