ATE_SYNCDONE 's' /* synchronization finished in front of
- * apply (sublsn set) */
+#define SUBREL_STATE_PRE_SYNCDONE 'p' /* synchronization finished in front of
+ * apply (sublsn set), but the final
+ * cleanup has not yet been performed */
+#define SUBREL_STATE_SYNCDONE 's' /* synchronization complete */
#define SUBREL_STATE_READY 'r' /* ready (sublsn set) */
Some adjustments to states and comments are needed according to my
"General Comment".
--
Kind Regards,
Peter Smith.
Fujitsu Australia
e why not write the
first error like:
errmsg("invalid value for parameter \"min_apply_delay\": \"%s\"",
input_string)
~
5b.
Since there are no translator considerations here why not write the
second error like:
errmsg("%d ms is outside the valid range for parameter
\"min_apply_delay\" (%d .. %d)",
result, 0, PG_INT32_MAX))
--
Kind Regards,
Peter Smith.
Fujitsu Australia
not applied");
"expectedly" ??
SUGGESTION (for comment)
# Confirm the record was not applied
--
Kind Regards,
Peter Smith.
Fujitsu Australia
t; --> "2023"
~~~
+extern char *deparse_utility_command(CollectedCommand *cmd, bool verbose_mode);
+extern char *deparse_ddl_json_to_string(char *jsonb);
+extern char *deparse_drop_command(const char *objidentity, const char
*objecttype,
+ DropBehavior behavior);
+
+
+#endif /* DDL_DEPARSE
On Fri, Feb 3, 2023 at 9:21 PM Alvaro Herrera wrote:
>
> On 2023-Feb-03, Peter Smith wrote:
>
...
> > 3. ExecuteGrantStmt
> >
> > + /* Copy the grantor id needed for DDL deparsing of Grant */
> > + istmt.grantor_uid = grantor;
> > +
> >
>
On Fri, Feb 3, 2023 at 4:21 PM Amit Kapila wrote:
>
> On Fri, Feb 3, 2023 at 6:41 AM Peter Smith wrote:
> >
> > On Thu, Feb 2, 2023 at 7:21 PM Takamichi Osumi (Fujitsu)
> > wrote:
> > >
> > ...
> > >
> > >
> >
Here are my review comments for patch v26-0001.
On Thu, Feb 2, 2023 at 7:18 PM Takamichi Osumi (Fujitsu)
wrote:
>
> Hi,
>
> On Wednesday, February 1, 2023 1:37 PM Peter Smith
> wrote:
> > Here are my review comments for the patch v25-0001.
> Thank you fo
log the remaining wait time even if that wait time
becomes negative. Then I think the test cases can be made
deterministic instead of relying on good luck. This seems like the
better option.
--
Kind Regards,
Peter Smith.
Fujitsu Australia
to say something like "Passes back", or maybe
just "Returns" is better.
==
src/include/replication/logicalrelation.h
12.
@@ -14,6 +14,7 @@
#include "access/attmap.h"
#include "replication/logicalproto.h"
+#include "storage/lockdefs.h"
What is this needed here for? I tried without this change and
everything still builds OK.
--
Kind Regards,
Peter Smith.
Fujitsu Australia
paragraph, and
also same wording as the GUC hint text).
--
Kind Regards,
Peter Smith.
Fujitsu Australia
criptionInfo
char*subdisableonerr;
char*suborigin;
char*subsynccommit;
+ int subminapplydelay;
char*subpublications;
} SubscriptionInfo;
Should this also be "int32" to match the other member type changes?
==
src/test/subscription/t/032_apply_delay.pl
16.
+# M
t;update_progress_txn(rb, txn, change->lsn);
--
Kind Regards,
Peter Smith.
Fujitsu Australia
Thanks for the updates to address all of my previous review comments.
Patch v90-0001 LGTM.
--
Kind Reagrds,
Peter Smith.
Fujitsu Australia
On Mon, Jan 30, 2023 at 5:23 PM houzj.f...@fujitsu.com
wrote:
>
> On Monday, January 30, 2023 12:13 PM Peter Smith
> wrote:
> >
> > Here are my review comments for v88-0002.
>
> Thanks for your comments.
>
> >
> > ==
> > General
> >
On Tue, Jan 31, 2023 at 4:00 AM Tom Lane wrote:
>
> Peter Smith writes:
> > The v3 patch LGTM (just for the logical replication commands).
>
> Pushed then.
>
Thanks for pushing the v3 patch.
I'd forgotten about the 'streaming' option -- AFAIK this was
previously a boolean
just refer to CREATE PUBLICATION.)
>
The v3 patch LGTM (just for the logical replication commands).
--
Kind Regards,
Peter Smith.
Fujitsu Australia
On Fri, Jan 27, 2023 at 10:30 PM Peter Eisentraut
wrote:
>
> On 19.01.23 00:45, Peter Smith wrote:
> > The original $SUBJECT requirements evolved to also try to make each
> > view appear on a separate page after that was suggested by DavidJ [2].
> > I was unable to achieve
rkers to read and apply them at the end of the transaction.
SUGGESTION
On the subscriber, if the streaming option is set to parallel, it
directs the leader apply worker to send changes to the shared memory
queue or to serialize changes and apply them at the end of the
transaction.
--
Kind Regards,
Peter Smith.
Fujitsu Australia
, Kuroda Hayato, Amit Kapila
"Pater" --> "Peter"
------
Kind Regards,
Peter Smith.
Fujitsu Australia
uot;
+ "the end of the transaction."),
GUC_NOT_IN_SAMPLE
},
6a. short description
User PoV behaviour should be the same. Instead, maybe say "controls
the internal behavior" or something like that?
~
6b. long description
IMO the long description shouldn’t mention ‘immediate’ mode first as it does.
BEFORE
If set to immediate, on the publisher side, ...
AFTER
On the publisher side, ...
--
Kind Regards,
Peter Smith.
Fujitsu Australia
tern PGDLLIMPORT int logical_decoding_mode;
+extern PGDLLIMPORT int logical_replication_mode;
Probably here should also be a comment to say "/* GUC variables */"
--
[1]
https://www.postgresql.org/message-id/OS0PR01MB5716AE9F095F9E7888987BC794C99%40OS0PR01MB5716.jpnprd01.prod.outlook.com
Kind Regards,
Peter Smith.
Fujitsu Australia
+ BEGIN;
+ INSERT INTO test_tab_2 values(1);
+ SAVEPOINT sp;
+ INSERT INTO test_tab_2 values(1);
+ ROLLBACK TO sp;
+ COMMIT;
+ });
Perhaps this should insert 2 different values so then the verification
code can check the correct value remains instead of just checking
COUNT(*)?
--
Kind Regards,
Peter Smith.
Fujitsu Australia
s a] delay"
(which is also correct, but it seemed a more awkward way to say it IMO)
~
Perhaps it's better to rename it more fully like
*maybe_delay_the_apply* to remove any ambiguous interpretations.
--
Kind Regards,
Peter Smith.
Fujitsu Australia
On Tue, Jan 24, 2023 at 1:45 PM wangw.f...@fujitsu.com
wrote:
>
> On Tues, Jan 24, 2023 at 8:28 AM Peter Smith wrote:
> > Hi Hou-san, Here are my review comments for v5-0001.
>
> Thanks for your comments.
...
>
> Changed as suggested.
>
> Attach the new
,
{NULL, 0, false}
};
I noticed this array is still called "logical_decoding_mode_options".
Was that deliberate?
--
Kind Regards,
Peter Smith.
Fujitsu Australia
}
--
Kind Regards,
Peter Smith.
Fujitsu Australia
On Mon, Jan 23, 2023 at 9:44 PM Amit Kapila wrote:
>
> On Mon, Jan 23, 2023 at 1:36 PM Peter Smith wrote:
> >
> > Here are my review comments for v19-0001.
> >
> ...
> >
> > 5. parse_subscription_options
> >
> > + /*
> > + * The c
ve nothing called a "substantial delay-time" case.
e.g2. the word "later" confused me. Originally, I thought you meant it
is not tested yet but that you will check it "later", but now IIUC you
are just referring to the "1 day 5 minutes" test that comes below in
this location TAP file (??)
--
Kind Regards,
Peter Smith.
Fujitsu Australia
ificant enough
that just leaving the curent code is neater?
LogicalDecodingContext *ctx = rb->private_data;
...
if (ctx->update_progress_txn && (++changes_count >= CHANGES_THRESHOLD))
{
rb->update_progress_txn(rb, txn, change);
changes_count = 0;
}
--
Kind Reagrds,
Peter Smith.
Fujitsu Australia
16 ms. Remaining wait time: 142828 ms
DEBUG: time-delayed replication for txid 1234, min_apply_delay =
16 ms. Remaining wait time: 129994 ms
DEBUG: time-delayed replication for txid 1234, min_apply_delay =
16 ms. Remaining wait time: 110001 ms
...
--
Kind Regards,
Peter Smith.
Fujitsu Australia
places here) seemed
strange. Maybe it's better to say like
SUGGESTION
# Confirm disabling the subscription by ALTER DISABLE did not cause
the delayed transaction to be applied.
$result = $node_subscriber->safe_psql('postgres',
"SELECT count(a) FROM test_tab WHERE a = 0;");
is($result, qq(0), "check the delayed transaction was not applied");
--
Kind Regards,
Peter Smith.
Fujitsu Australia
On Fri, Jan 20, 2023 at 3:35 PM Amit Kapila wrote:
>
> On Fri, Jan 20, 2023 at 7:40 AM Peter Smith wrote:
> >
> > Here are some review comments for patch v3-0001.
> >
> > ==
> > src/backend/replication/logical/logical.c
> >
> > 3. forward
BEFORE
ctx->update_progress(ctx, ctx->write_location, ctx->write_xid, false);
AFTER
OutputPluginUpdateProgress(ctx, false);
--
Kind Regards,
Peter Smith.
Fujitsu Australia
l is 0?
e.g.
if (indent)
{
appendStringInfoCharMacro(out, '\n');
if (level > 0)
appendStringInfoSpaces(out, level * 4);
}
V.
if (indent)
{
appendStringInfoCharMacro(out, '\n');
appendStringInfoSpaces(out, level * 4);
}
--
Kind Regards,
Peter Smith.
Fujitsu Australia
hat comment be modified to say
/* not strictly necessary for LOGICALREP_COLUMN_BINARY but per
StringInfo practice */
--
Kind Regards,
Peter Smith.
Fujitsu Australia
On Wed, Jan 18, 2023 at 6:06 PM Peter Smith wrote:
>
> Here are my review comments for the latest patch v16-0001. (excluding
> the test code)
>
And here are some review comments for the v16-0001 test code.
==
src/test/regress/sql/subscription.sql
1. General
For all com
On Wed, Jan 18, 2023 at 6:06 PM Peter Smith wrote:
>
> Here are my review comments for the latest patch v16-0001. (excluding
> the test code)
>
...
>
> 8. AlterSubscription (general)
>
> I observed during testing there are 3 different errors….
>
> At subscr
z1TLWOLSoFvoyC0mq%2Bs92yFSd534ctWSdjEFtKCw%40mail.gmail.com
[4] DJ how to do it using refentry -
https://www.postgresql.org/message-id/CAKFQuwYkM5UZT%2B6tG%2BNgZvDcd5VavS%2BxNHsGsWC8jS-KJsxh7w%40mail.gmail.com
Kind Regards,
Peter Smith.
Fujitsu Australia
minapplydelay; /* Replication apply delay */
+
NameData subname; /* Name of the subscription */
Oid subowner BKI_LOOKUP(pg_authid); /* Owner of the subscription */
SUGGESTION (for comment)
Replication apply delay (ms)
~~
24.
@@ -120,6 +122,7 @@ typedef struct Subscription
* in */
XLogRecPtr skiplsn; /* All changes finished at this LSN are
* skipped */
+ int64 minapplydelay; /* Replication apply delay */
SUGGESTION (for comment)
Replication apply delay (ms)
--
Kind Regards,
Peter Smith.
Fujitsu Australia
. -e "linkend='" | wc -l
12
(double-quotes)
$ grep --include=*.sgml -rn . -e 'linkend="' | wc -l
5915
~~
PSA patch that makes them all use double quotes.
--
Kind Regards,
Peter Smith.
Fujitsu Australia
v1-0001-Replace-linkend-single-quotes-with-double-quotes.patch
Description: Binary data
On Tue, Jan 17, 2023 at 2:37 PM houzj.f...@fujitsu.com
wrote:
>
> On Tuesday, January 17, 2023 11:32 AM Peter Smith
> wrote:
> >
> > On Tue, Jan 17, 2023 at 1:21 PM houzj.f...@fujitsu.com
> > wrote:
> > >
> > > On Tuesday, January 17, 2023 5:43 AM P
On Tue, Jan 17, 2023 at 1:21 PM houzj.f...@fujitsu.com
wrote:
>
> On Tuesday, January 17, 2023 5:43 AM Peter Smith
> wrote:
> >
> > On Mon, Jan 16, 2023 at 5:41 PM Amit Kapila
> > wrote:
> > >
> > > On Mon, Jan 16, 2023 at 10:24
On Mon, Jan 16, 2023 at 5:41 PM Amit Kapila wrote:
>
> On Mon, Jan 16, 2023 at 10:24 AM Peter Smith wrote:
> >
> > 2.
> >
> > /*
> > + * Return the pid of the leader apply worker if the given pid is the pid
> > of a
> > + * pa
etty much in the
correct numerical order except this one, so IMO this code ought to be
relocated to later in this same function.
--
Kind Regards,
Peter Smith.
Fujitsu Australia.
prod.outlook.com
[2] Amit -
https://www.postgresql.org/message-id/CAA4eK1J%3D9m-VNRMHCqeG8jpX0CTn3Ciad2o4H-ogrZMDJ3tn4w%40mail.gmail.com
Kind Regards,
Peter Smith.
Fujitsu Australia
not a "parallel apply" worker then it can only be
one of the other 2. So I think it is sufficient and less confusing to
say:
Process ID of the leader apply worker if this process is a parallel
apply worker; NULL if this process is a leader apply worker or a
synchronization worker.
--
Kind Regards,
Peter Smith.
Fujitsu Australia
%2BPvA10Bp9Jaw9OS2%2BpuKHr7ry_xB3Tf2-bbv5gyxD5E_gw%40mail.gmail.com
Kind Regards,
Peter Smith.
Fujitsu Australia
tat_get_subscription(NULL::oid) st(subid, relid,
pid, apply_leader_pid, received_lsn, last_msg_send_time,
last_msg_receipt_time, latest_end_lsn, latest_end_time) ON ((st.subid
= su.oid)));
pg_stat_subscription_stats| SELECT ss.subid,
s.subname,
ss.apply_error_count,
(Same comment as elsewhere)
"apply_leader_pid" --> "leader_apply_pid"
--
Kind Regards,
Peter Smith.
Fujitsu Australia
On Wed, Jan 4, 2023 at 6:08 PM vignesh C wrote:
>
> On Mon, 2 Jan 2023 at 13:47, Peter Eisentraut
> wrote:
> >
> > On 08.12.22 03:30, Peter Smith wrote:
> > > PSA patches for v9*
> > >
> > > v9-0001 - Now the table rows are ordered per Pe
On Wed, Dec 21, 2022 at 6:59 PM Alvaro Herrera wrote:
>
> On 2022-Dec-21, Peter Smith wrote:
>
> > By "searching" I also meant just scanning visually, although I was
> > thinking more about scanning the PDF.
> >
> > Right now, the intention of any
ers at all, rather brings risks.
>
> I prefer the idea Kuroda-san previously proposed; setting
> logical_decoding_mode = 'immediate' means setting
> logical_decoding_work_mem = 0. We might not need to have it as an enum
> parameter since it has only two values, though.
Did you mean one GUC (logical_decoding_mode) will cause a side-effect
implicit value change on another GUC value
(logical_decoding_work_mem)?
If so, then that seems a like potential source of confusion IMO.
- e.g. actual value is invisibly set differently from what the user
sees in the conf file
- e.g. will it depend on the order they get assigned
Are there any GUC precedents for something like that?
--
Kind Regards,
Peter Smith.
Fujitsu Australia
ny streaming changes to the output plugin immediately
without waiting until logical_decoding_work_mem is exceeded."),
--
[1] https://www.postgresql.org/docs/devel/logical-replication-config.html
[2] GUC declarations -
https://github.com/postgres/postgres/commit/d9d873bac67047cfacc9f5ef96ee488f2cb0f1c3
Kind Regards,
Peter Smith.
Fujitsu Australia
On Tue, Dec 20, 2022 at 5:22 PM Peter Smith wrote:
>
> On Tue, Dec 20, 2022 at 2:20 PM Amit Kapila wrote:
> >
> > On Tue, Dec 20, 2022 at 8:17 AM Peter Smith wrote:
> > >
> > > Summary
> > > ---
> > >
> > > In summary, everyt
Looks like a recent commit [1] to add copyrights broke the patch
--
[1]
https://github.com/postgres/postgres/commit/8284cf5f746f84303eda34d213e89c8439a83a42
Kind Regards,
Peter Smith.
Fujitsu Australia
On Tue, Dec 20, 2022 at 2:29 AM Alvaro Herrera wrote:
>
> On 2022-Oct-31, Peter Smith wrote:
>
> > 6. add_policy_clauses
> >
> > + else
> > + {
> > + append_bool_object(policyStmt, "present", false);
> > + }
> >
> > Somethin
On Tue, Dec 20, 2022 at 7:21 PM Alvaro Herrera wrote:
>
> On 2022-Dec-20, Peter Smith wrote:
>
> > If you change this warning title then it becomes the odd one out -
> > every other warning in all the pg docs just says "Warning". IMO
> > maintaining consi
On Tue, Dec 20, 2022 at 2:20 PM Amit Kapila wrote:
>
> On Tue, Dec 20, 2022 at 8:17 AM Peter Smith wrote:
> >
> > Summary
> > ---
> >
> > In summary, everything I have tested so far appeared to be working
> > properly. In other words, for overlapp
is best. e.g. I can imagine maybe
someone searching for "Warning" in the docs, and now they are not
going to find this one.
Maybe a safer way to fix this "silly otherwise-empty section title"
would be to just add some explanatory text so it is no longer empty.
--
Kind Regards,
Peter Smith.
Fujitsu Australia
On Mon, Dec 19, 2022 at 5:35 PM Peter Smith wrote:
>
> On Mon, Dec 19, 2022 at 5:04 PM Tom Lane wrote:
> >
> > Peter Smith writes:
> > > My patch/idea makes a small change to the isolationtester spec
> > > grammar. Now each session can optionally specify i
On Mon, Dec 19, 2022 at 5:04 PM Tom Lane wrote:
>
> Peter Smith writes:
> > My patch/idea makes a small change to the isolationtester spec
> > grammar. Now each session can optionally specify its own connection
> > string. When specified, this will overri
vjqm2KURZEAGw%40mail.gmail.com
[2] isolation tests -
https://github.com/postgres/postgres/blob/master/src/test/isolation/README
[3] test_decoding spec tests -
https://github.com/postgres/postgres/tree/master/contrib/test_decoding/specs
Kind Regards,
Peter Smith
Fujitsu Australia
#!/bin/bash
p
On Tue, Dec 13, 2022 at 2:33 PM Peter Smith wrote:
>
> On Tue, Dec 6, 2022 at 5:23 PM shiy.f...@fujitsu.com
> wrote:
> >
> > Hi hackers,
> >
> > In logical decoding, when logical_decoding_work_mem is exceeded, the
> > changes are
>
er is already
launched, then that will have no effect on the already running
walsender.
Is that understanding correct?
--
Kind Regards,
Peter Smith.
Fujitsu Australia.
e leader worker so it can update
the lsn_mappings." ??
--
Kind Regards,
Peter Smith.
Fujitsu Australia
/a8500750ca0acf6bb95cf9d1ac7f421749b22db7
Kind Regards,
Peter Smith.
Fujitsu Australia
On Tue, Dec 13, 2022 at 6:25 AM Alvaro Herrera wrote:
>
> On 2022-Dec-07, samay sharma wrote:
>
> > On Tue, Dec 6, 2022 at 11:12 PM Peter Smith wrote:
>
> > > OK. I copied the tablesync note back to config.sgml definition of
> > > 'max_replication_slot
es like those shown below might be more appropriate:
stream_checks_work_mem = true/false
stream_mode_checks_size = true/false
stream_for_large_tx_only = true/false
... etc.
The GUC name length could get a bit unwieldy but isn't it better for
it to have the correct meaning than to have a shorter but slightly
ambiguous name? Anyway, it is a developer option so I guess longer
names are less of a problem.
--
Kind Regards,
Peter Smith.
Fujitsu Australia
so I
assumed it would just carry forward to the next one.
Anyway, I've added it again to 2023-01 commitfest [2]. Thanks for telling me.
--
[1] 2022-11 CF - https://commitfest.postgresql.org/40/3959/
[2] 2023-01 CF - https://commitfest.postgresql.org/41/4061/
Kind Regards,
Peter Smith.
Fujitsu Australia.
- StaticAssertStmt(EEOP_LAST + 1 == lengthof(dispatch_table),
+ StaticAssertDecl(EEOP_LAST + 1 == lengthof(dispatch_table),
"dispatch_table out of whack with ExprEvalOp");
Ditto the previous comment.
--
Kind Regards,
Peter Smith.
Fujitsu Australia
llel apply feature? If
> sending the additional information is also controlled by an option
> like "streaming", we can decide what we send based on these options,
> no?
>
AFAIK the protocol version defines what protocol message bytes are
transmitted on the wire. So I thought the protocol version should
*always* be updated whenever the message format changes. In other
words, I don't think we ought to be transmitting different protocol
message formats unless it is a different protocol version.
Whether the pub/sub implementation actually needs to check that
protocol version or whether we happen to have some alternative knob we
can check doesn't change what the protocol version is supposed to
mean. And the PGDOCS [1] and [2] currently have clear field notes
about when those fields are present (e.g. "This field is available
since protocol version XXX"), but if hypothetically you don't change
the protocol version for some new fields then now the message format
becomes tied to the built-in implementation of pub/sub -- now what
field note will you say instead to explain that?
--
[1] https://www.postgresql.org/docs/current/protocol-logical-replication.html
[2]
https://www.postgresql.org/docs/current/protocol-logicalrep-message-formats.html
Kind Regards,
Peter Smith.
Fujitsu Australia.
parameter
> > + > linkend="guc-max-worker-processes">max_worker_processes
> > +may need to be adjusted to accommodate for replication workers, at
> > least (
> > + > linkend="guc-max-logical-replication-workers">max_logical_replication_workers
> > ++ 1). Note, some extensions and parallel queries
> > also
> > +take worker slots from max_worker_processes.
> > +
>
> Maybe do max_worker_processes in a new line like the rest.
OK. Done as suggested.
--
Kind Regards,
Peter Smith.
Fujitsu Australia
v7-0001-Logical-replication-GUCs-links-and-tidy.patch
Description: Binary data
-8d9bf855bc48%40enterprisedb.com
[2]
https://www.postgresql.org/message-id/CAKFQuwby7xWHek8%3D6UPaNXrhGA-i0B2zMOmBoGHgc4yaO8NH_w%40mail.gmail.com
Kind Regards,
Peter Smith.
Fujitsu Australia.
v9-0001-Re-order-Table-28.2-Collected-Statistics-Views.patch
Description: Binary data
v9-0003-Add
On Tue, Dec 6, 2022 at 5:57 AM samay sharma wrote:
>
> Hi,
>
> On Mon, Oct 24, 2022 at 12:45 AM Peter Smith wrote:
>>
>> Hi hackers.
>>
>> There is a docs Logical Replication section "31.10 Configuration
>> Settings" [1] which describes
I'd like to "fix" this but IIUC there is no consensus yet about what
order is best for patch 0001, right?
--
Kind Regards,
Peter Smith.
Fujitsu Australia
k the largest transaction (or subtransaction) and evict it from
IIUC this logic can be simplified quite a lot just by shifting that
"bail out" condition into the loop.
Something like:
while (true)
{
if (!(force_stream && rb->size > 0 || rb->size <
logical_decoding_work_mem * 1024L))
break;
...
}
--
Kind Regards,
Peter Smith.
Fujitsu Australia
hem all separated. Putting everything into a single
'logical_replication_mode' might cause difficulties later when/if you
want combinations of the different modes.
For example, instead of
logical_replication_mode = XXX/YYY/ZZZ
maybe something like below will give more flexibility.
logical_replication_dev_XXX = true/false
logical_replication_dev_YYY = true/false
logical_replication_dev_ZZZ = true/false
--
Kind Regards,
Peter Smith.
Fujitsu Australia
On Tue, Dec 6, 2022 at 2:51 PM Amit Kapila wrote:
>
> On Tue, Dec 6, 2022 at 5:27 AM Peter Smith wrote:
> >
> > Here are my review comments for patch v55-0002
> >
> ...
> > 4.
> >
> > /*
> > + * Replay the spooled messages in the parallel a
luding "logical
replication apply delay" is logged. I guess that is OK, but another
way might be to compare the actual timing values of the published and
replicated rows.
The publisher table can have a column with default now() and the
subscriber side table can have an *additional* column also with
default now(). After replication, those two timestamp values can be
compared to check if the difference exceeds the min_time_delay
parameter specified.
--
Kind Regards,
Peter Smith.
Fujitsu Australia
is
+ * ready to be read by a parallel apply worker.
+ */
+typedef enum PartialFileSetState
"ready to be read" sounded a bit strange.
SUGGESTION
... to the file so it is now OK for a parallel apply worker to read it.
--
[1] Houz reply to my review v51-0002 --
https://www.postgresql.org/message-id/OS0PR01MB5716350729D8C67AA8CE333194129%40OS0PR01MB5716.jpnprd01.prod.outlook.com
Kind Regards,
Peter Smith.
Fujitsu Australia
(Resending this because somehow my previous post did not appear in the
mail archives)
-- Forwarded message -
From: Peter Smith
Date: Fri, Dec 2, 2022 at 7:59 PM
Subject: Re: Perform streaming logical transactions by background
workers and parallel apply
To: houzj.f...@fujitsu.com
-- Forwarded message -
From: Peter Smith
Date: Sat, Dec 3, 2022 at 8:03 AM
Subject: Re: Perform streaming logical transactions by background
workers and parallel apply
To: Amit Kapila
On Fri, Dec 2, 2022 at 8:57 PM Amit Kapila wrote:
>
> On Fri, Dec 2, 2022 at 2:29 PM
On Sat, Nov 26, 2022 at 2:43 PM David G. Johnston
wrote:
>
> On Wed, Nov 23, 2022 at 1:36 AM Peter Smith wrote:
>>
>> On Thu, Nov 17, 2022 at 8:46 AM David G. Johnston
>> wrote:
>>
>> > Also, make it so each view ends up being its own separate page.
&g
tidies the Chapter 31.10 "Configuration Settings" section.
Meanwhile, the Subscriber GUC descriptions are left on the
config.sgml, where you said they are supposed to be.
--
Kind Regards,
Peter Smith.
Fujitsu Australia
v5-0001-Logical-replication-GUCs-links-and-tidy.patch
Description: Binary data
t be achieved using a simpler syntax than
what had been previously suggested. But in practice there may be some
problems with this approach -- e.g. how will the initial tablesync
COPY efficiently assign these subscriber name column values?
--
[1]
https://www.postgresql.org/message-id/CAHut%
leset for the next streaming transaction.
~~~
35. globals
/*
+ * Indicates whether the leader apply worker needs to serialize the
+ * remaining changes to disk due to timeout when sending data to the
+ * parallel apply worker.
+ */
+ bool serialize_changes;
35a.
I wonder if the comment would be better to also mention "via shared memory".
SUGGESTION
Indicates whether the leader apply worker needs to serialize the
remaining changes to disk due to timeout when attempting to send data
to the parallel apply worker via shared memory.
~
35b.
I wonder if a more informative variable name might be
serialize_remaining_changes?
--
[1]
https://stackoverflow.com/questions/17504316/what-happens-with-an-extern-inline-function
Kind Regards,
Peter Smith.
Fujitsu Australia
On Sat, Nov 26, 2022 at 2:43 PM David G. Johnston
wrote:
>
> On Wed, Nov 23, 2022 at 1:36 AM Peter Smith wrote:
>>
>> On Thu, Nov 17, 2022 at 8:46 AM David G. Johnston
>> wrote:
>>
>> > Also, make it so each view ends up being its own separate page.
&g
On Fri, Nov 25, 2022 at 11:09 PM Peter Eisentraut
wrote:
>
> On 23.11.22 09:36, Peter Smith wrote:
> > PSA new patches. Now there are 6 of them. If some of the earlier
> > patches are agreeable can those ones please be committed? (because I
> > think this patch may be susc
STION
if (am_parallel_apply_worker())
{
/* Notify the leader apply worker that we have exited cleanly. */
pq_putmessage('X', NULL, 0);
}
--
[1] Hou-san's reply to my v49-0001 review.
https://www.postgresql.org/message-id/OS0PR01MB5716339FF7CB759E751492CB940D9%40OS0PR01MB5716.jpnprd01.prod.outlook.com
Kind Regards,
Peter Smith.
Fujitsu Australia
On Fri, Nov 25, 2022 at 2:15 AM Takamichi Osumi (Fujitsu)
wrote:
>
> On Wednesday, October 5, 2022 6:42 PM Peter Smith
> wrote:
...
>
> > ==
> >
> > 5. src/backend/commands/subscriptioncmds.c - SubOpts
> >
> > @@ -89,6 +91,7 @@ typedef struct S
On Wed, Nov 23, 2022 at 9:16 AM Peter Smith wrote:
>
> On Wed, Nov 16, 2022 at 10:24 PM vignesh C wrote:
> >
> ...
>
> > One suggestion:
> > The format of subscribers includes the data type and default values,
> > the format of publishers does not include data
On Thu, Nov 17, 2022 at 8:46 AM David G. Johnston
wrote:
>
> On Tue, Nov 15, 2022 at 6:39 PM Peter Smith wrote:
>>
>>
>> I was also wondering (but have not yet done) if the content *outside*
>> the tables should be reordered to match the table 28.1/28.2 ord
missing functionality, but perhaps there is
some information that is harder to find than it ought to be, so I
would like to help first address that part.
--
[1] conflicts.
https://www.postgresql.org/docs/current/logical-replication-conflicts.html
[2] max_sync_workers_per_subscription.
https://www.postgresql.org/docs/current/runtime-config-replication.html
[3] srsubstate.
https://www.postgresql.org/docs/current/catalog-pg-subscription-rel.html
Kind Regards,
Peter Smith.
Fujitsu Australia
---+--
1 | one | sub_tenant1
2 | two | sub_tenant1
3 | three | sub_tenant1
(3 rows)
~~
Subscriptions to different tenants would be named differently.
And using other SQL you can map/filter those names however your
application wants.
--
Kind Regards,
Peter Smith.
Fujitsu Australia
same format, we could give a link to
> runtime-config-replication where data type and default is defined for
> publisher configurations max_replication_slots and max_wal_senders.
>
Thanks for your suggestions.
I have included xref links to the original definitions, rather than
defining the
ep_parallel_apply_worker_count(Oid subid);
Would it be better to call those new functions using similar shorter
names as done elsewhere?
logicalrep_parallel_apply_worker_stop -> logicalrep_pa_worker_stop
logicalrep_parallel_apply_worker_count -> logicalrep_pa_worker_count
--
[1] Hou-san's reply to my review v47-0001.
https://www.postgresql.org/message-id/OS0PR01MB571680391393F3CB63469F3E940A9%40OS0PR01MB5716.jpnprd01.prod.outlook.com
Kind Regards,
Peter Smith.
Fujitsu Australia
On Fri, Nov 18, 2022 at 6:03 PM Peter Smith wrote:
>
> Here are some review comments for v47-0001
>
> (This review is a WIP - I will post more comments for this patch next week)
>
Here are the rest of my comments for v47-0001
==
doc/src/sgml/monitoring.
1.
@@ -1851,6 +1851
quot; -> "directly by the worker" (??) 2x
~~~
13. get_worker_name
+/*
+ * Return the name of the logical replication worker.
+ */
+static const char *
+get_worker_name(void)
+{
+ if (am_tablesync_worker())
+ return _("logical replication table synchronization worker");
+ el
need to change anything at all to get the
benefits of parallel streaming.
> Another variant is to have a new subscription parameter for example
> "parallel_workers" parameter that specifies the number of parallel
> workers. That way, users can specify the number of parallel workers
> per subscription.
>
--
Kind Regards,
Peter Smith.
Fujitsu Australia.
iver
28.2.13. pg_stat_bgwriter
28.2.14. pg_stat_wal
28.2.15. pg_stat_database
28.2.16. pg_stat_database_conflicts
28.2.23. pg_stat_slru
28.2.5. pg_stat_replication_slots
28.2.17. pg_stat_all_tables
28.2.18. pg_stat_all_indexes
28.2.19. pg_statio_all_tables
28.2.20. pg_statio_all_indexes
2
On Sun, Nov 13, 2022 at 11:47 AM vignesh C wrote:
>
> On Mon, 24 Oct 2022 at 13:15, Peter Smith wrote:
> >
> > Hi hackers.
> >
> > There is a docs Logical Replication section "31.10 Configuration
> > Settings" [1] which describes some lo
601 - 700 of 1480 matches
Mail list logo
