de is expected"
because all the other prerequisites are now satisfied.
~~~
21.
+$new_publisher->start;
+my $result = $new_publisher->safe_psql('postgres',
+ "SELECT slot_name, two_phase FROM pg_replication_slots");
+is($result, qq(test_slot1|t), 'check the slot exists on new node');
Should there be a matching new_pulisher->stop;?
--
[1]
https://www.postgresql.org/message-id/CAA4eK1%2BdT2g8gmerguNd_TA%3DXMnm00nLzuEJ_Sddw6Pj-bvKVQ%40mail.gmail.com
[2]
https://www.postgresql.org/message-id/TYAPR01MB586604802ABE42E11866762FF51BA%40TYAPR01MB5866.jpnprd01.prod.outlook.com
Kind Regards,
Peter Smith.
Fujitsu Australia
On Fri, Aug 18, 2023 at 6:16 PM Zhijie Hou (Fujitsu)
wrote:
>
> On Friday, August 18, 2023 11:20 AM Amit Kapila
> wrote:
> >
> > On Mon, Aug 14, 2023 at 12:08 PM Peter Smith
> > wrote:
> > >
> > > The main patch for adding the worker ty
On Fri, Aug 18, 2023 at 12:47 PM Amit Kapila wrote:
>
> On Thu, Aug 17, 2023 at 2:10 PM Peter Smith wrote:
> >
> > Here are some review comments for the first 2 patches.
> >
> > /*
> > - * Fetch all libraries containing non-built-in C functions in
lit_walfile_name('00010001');
SELECT segment_number > 0 AS ok_segment_number, timeline_id
- FROM pg_split_walfile_name('ffFF0000000100af');
+ FROM pg_split_walfile_name('ffFF000100af');
\ No newline at end of file
~
What is this change for? It looks like maybe some accidental
whitespace change happened.
--
Kind Regards,
Peter Smith.
Fujitsu Australia
On Fri, Aug 18, 2023 at 10:47 AM Michael Paquier wrote:
>
> On Thu, Aug 17, 2023 at 06:09:31PM +1000, Peter Smith wrote:
> > Ping.
>
> Funnily enough, I was looking at this entry yesterday, before you
> replied, and was wondering what's happening here.
>
> > I tho
ations for the subsequent test. max_replication_slots is set to
# appropriate value
$new_node->append_conf('postgresql.conf', "max_replication_slots = 10");
# Remove an unnecessary slot and consume WALs
$old_node->start;
$old_node->safe_psql(
'postgres', qq[
SELECT pg_drop_replication_slot('test_slot1');
SELECT count(*) FROM pg_logical_slot_get_changes('test_slot2', NULL, NULL)
]);
$old_node->stop;
~
Some of that preparation seems unnecessary. I think the new node
max_replication_slots is 1 already, so if you are going to remove one
of test_slot1 here then there is only ONE slot left, right? So the
max_replication_slots on the new node should be OK now. Not only will
there be less test code needed here, but you will be testing the
boundary condition of max_replication_slots (which is probably a good
thing to do).
--
Kind Regards,
Peter Smith.
Fujitsu Australia
On Thu, May 11, 2023 at 7:09 PM Peter Smith wrote:
>
> On Thu, May 11, 2023 at 6:30 PM Daniel Gustafsson wrote:
> >
> > > On 11 May 2023, at 07:41, Peter Smith wrote:
> >
> > > While reviewing another patch for the file info.c, I noticed some
> > &
bscription proposal.
PSA patch v1.
Thoughts?
--
[1]
https://www.postgresql.org/message-id/CAA4eK1JO54%3D3s0KM9iZGSrQmmfzk9PEOKkW8TXjo2OKaKrSGCA%40mail.gmail.com
[2]
https://github.com/postgres/postgres/commit/2a8b40e3681921943a2989fd4ec6cdbf8766566c
Kind Regards,
Peter Smith.
Fujitsu Australia
v
after rebuilding again with HEAD+v26-0001
--
Kind Regards,
Peter Smith.
Fujitsu Australia
#!/bin/bash
tables=( 100 )
#tables=( 10 \
#100 \
#1000 \
#2000
#)
workers=( 2 4 8 16 )
#workers=( 2 \
#4 \
#8 \
#16
#)
prefix="0816headbusy"
# For now, either of "0" or "10
blesync process is just taking a very long
time handling the original 'relid'.
Won't the stale process name cause confusion to the users?
--
Kind Regards,
Peter Smith.
Fujitsu Australia
A rebase was needed due to a recent push [1].
PSA v3.
--
[1]
https://github.com/postgres/postgres/commit/2a8b40e3681921943a2989fd4ec6cdbf8766566c
Kind Regards,
Peter Smith.
Fujitsu Australia
v3-0001-logicalrep_worker_launch-limit-checks.patch
Description: Binary data
/2a8b40e3681921943a2989fd4ec6cdbf8766566c
Kind Regards,
Peter Smith.
Fujitsu Australia
v8-0001-Switch-on-worker-type.patch
Description: Binary data
The previous patch was accidentally not resetting the boolean limit
flags to false for retries.
Fixed in v2.
--
Kind Regards,
Peter Smith.
Fujitsu Australia
v2-0001-logicalrep_worker_launch-limit-checks.patch
Description: Binary data
On Fri, Aug 11, 2023 at 9:13 PM Amit Kapila wrote:
>
> On Fri, Aug 11, 2023 at 3:41 PM Peter Smith wrote:
> >
> > On Fri, Aug 11, 2023 at 7:33 PM Amit Kapila wrote:
> > >
> > > On Thu, Aug 10, 2023 at 7:50 AM Peter Smith wrote:
> > > >
> >
On Fri, Aug 11, 2023 at 7:33 PM Amit Kapila wrote:
>
> On Thu, Aug 10, 2023 at 7:50 AM Peter Smith wrote:
> >
> > >
> > > * If you do the above then there won't be a need to change the
> > > variable name is_parallel_apply_worker in logicalrep_worker_laun
ant countings should also be more efficient in
theory, but in practice, I think the unnecessary worker loops are too
short (max_logical_replication_workers) for any performance
improvements to be noticeable.
Thoughts?
--
Kind Regards,
Peter Smith.
Fujitsu Australia
v1-0001-logicalrep_worker_lau
On Fri, Aug 11, 2023 at 12:54 AM Melih Mutlu wrote:
>
> Hi Peter and Vignesh,
>
> Peter Smith , 7 Ağu 2023 Pzt, 09:25 tarihinde şunu
> yazdı:
>>
>> Hi Melih.
>>
>> Now that the design#1 ERRORs have been fixed, we returned to doing
>> performanc
}'
4014.266
3892.089
4195.318
3571.862
4312.183
HEAD+v26-0001
[peter@localhost res_0809_vignesh_timing_sync_v260001]$ cat *.dat_SUB
| grep RESULT | grep -v duration | awk '{print $3}'
3326.627
3213.028
3433.611
3299.803
3258.821
--
Kind Regards,
Peter Smith.
Fujitsu Australia
On Wed, Aug 9, 2023 at 4:18 PM Amit Kapila wrote:
>
> On Tue, Aug 8, 2023 at 1:39 PM Peter Smith wrote:
> >
> > On Fri, Aug 4, 2023 at 5:45 PM Peter Smith wrote:
> >
> > v4-0001 uses only 3 simple inline functions. Callers always pass
> >
On Wed, Aug 9, 2023 at 2:44 PM Amit Kapila wrote:
>
> On Wed, Aug 9, 2023 at 8:20 AM Peter Smith wrote:
> >
> > On Tue, Aug 8, 2023 at 6:52 PM Amit Kapila wrote:
> > >
> > > On Tue, Aug 8, 2023 at 1:50 PM Peter Eisentraut
> > > wrote:
> &g
h -- i.e. the list might easily be very long with
100s or 1000s of tables it, and so inappropriate to substitute in the
message. OTOH, showing only problematic publications is a short list
but it is still more useful than showing nothing (e.g. there other
publications of the subscription might be OK so those ones are not
listed)
--
Kind Regards,
Peter Smith.
Fujitsu Australia
v1-0001-Improved-message-for-create-subscription.patch
Description: Binary data
PSA v4 patches.
On Fri, Aug 4, 2023 at 5:45 PM Peter Smith wrote:
>
> Thank you for the feedback received so far. Sorry, I have become busy
> lately with other work.
>
> IIUC there is a general +1 for the idea, but I still have some
> juggling necessary to make the functions/ma
groups of #defines are very nearly, but not quite, in that order.
--
Kind Regards,
Peter Smith.
Fujitsu Australia
calproto.h#L57C31-L57C31
Kind Regards,
Peter Smith
Fujitsu Australia
ment 3% 2% 6% 25%
--
TEST SCRIPTS
The "busy apply" test scripts are basically the same as already posted
[1], but I have reattached the latest ones again anyway.
--
[1]
https://www.postgresql.org/message-id/CAHut%2BPuNVNK2%2BA%2BR6eV8rKPNBHemCFE4NDtEYfpXbYr6SsvvBg%40mail.gmail
rdpdO0vwuDivD99TW%2B_9vvfFsErVNzq1ehYV9Q%40mail.gmail.com
[2] Melih -
https://www.postgresql.org/message-id/CAGPVpCRZ-zEOa2Lkvw%2BiTU3NhJzfbGwH1dU7Htreup--8e5nxg%40mail.gmail.com
[3] Bharath -
https://www.postgresql.org/message-id/CALj2ACVro6oCsTg_gpYu%2BV_LPMSgk4wjmSPDk8d5thArWNRoRQ%40mail.gmail
FWIW, I confirmed that my review comments for v22* have all been
addressed in the latest v26* patches.
Thanks!
--
Kind Regards,
Peter Smith.
Fujitsu Australia
Just to clarify my previous post, I meant we will need new v26* patches
v24-0001 -> not needed because v25-0001 pushed
v24-0002 -> v26-0001
v24-0003 -> v26-0002
On Thu, Aug 3, 2023 at 6:19 PM Peter Smith wrote:
>
> Hi Melih,
>
> Now that v25-0001 has been pushed,
Hi Melih,
Now that v25-0001 has been pushed, can you please rebase the remaining patches?
--
Kind Regards,
Peter Smith.
Fujitsu Australia
ter to explicitly tell which reviews are addressed but
> anyway, I have done some minor cleanup in the 0001 patch including
> removing includes which didn't seem necessary, modified a few
> comments, and ran pgindent. I also thought of modifying some variable
> names based on suggestions by Pe
On Wed, Aug 2, 2023 at 1:00 PM Amit Kapila wrote:
>
> On Wed, Aug 2, 2023 at 8:10 AM Peter Smith wrote:
> >
> >
> > The am_xxx functions are removed now in the v2-0001 patch. See [1].
> >
> > The replacement set of macros (the ones with no arg) are not s
On Wed, Aug 2, 2023 at 3:35 PM Masahiko Sawada wrote:
>
> On Mon, Jul 31, 2023 at 10:47 AM Peter Smith wrote:
> >
> > Hi hackers,
> >
> > BACKGROUND:
> >
> > The logical replication has different worker "types" (e.g. apply
> > leader,
;
The am_xxx functions are removed now in the v2-0001 patch. See [1].
The replacement set of macros (the ones with no arg) are not strictly
necessary, except I felt it would make the code unnecessarily verbose
if we insist to pass MyLogicalRepWorker everywhere from the callers in
worker.c / table
Thanks for your detailed code review.
Most comments are addressed in the attached v2 patches. Details inline below:
On Mon, Jul 31, 2023 at 7:55 PM Bharath Rupireddy
wrote:
>
> On Mon, Jul 31, 2023 at 7:17 AM Peter Smith wrote:
> >
> > PROBLEM:
> >
> > I
Hi Hou-san,
Thanks for your review!
Oops. Of course, you are right. My cut/paste typo was mostly confined
to the post, but there was one instance also in the patch.
Fixed in v2.
--
Kind Regards,
Peter Smith.
Fujitsu Australia
v2-0001-Simplify-worker-type-checks.patch
Description: Binary data
On Fri, Jul 28, 2023 at 5:22 PM Peter Smith wrote:
>
> Hi Melih,
>
> BACKGROUND
> --
>
> We wanted to compare performance for the 2 different reuse-worker
> designs, when the apply worker is already busy handling other
> replications, and then simultaneousl
hose above-suggested changes. The 'make
check' and TAP subscription tests are all passing OK.
---
Kind Regards,
Peter Smith.
Fujitsu Australia
v1-0001-Simplify-worker-type-checks.patch
Description: Binary data
se now we can switch on the worker enum type,
instead of using cascading if/else. (see Patch 0002).
Thoughts?
--
Kind Regards,
Peter Smith.
Fujitsu Australia
v1-0001-Add-LogicalRepWorkerType-enum.patch
Description: Binary data
v1-0002-Switch-on-worker-type.patch
Description: Binary data
On Thu, Jul 27, 2023 at 11:30 PM Melih Mutlu wrote:
>
> Hi Peter,
>
> Peter Smith , 26 Tem 2023 Çar, 07:40 tarihinde şunu
> yazdı:
>>
>> Here are some comments for patch v22-0001.
>>
>> ==
>> 1. General -- naming conventions
>>
>> Ther
.
Since this is a new function, should it be named according to the
convention for static functions?
e.g.
ApplicationNameForTablesync -> app_name_for_tablesync
--
Kind Regards,
Peter Smith.
Fujitsu Australia
ue, this means that the tablesync
+ * worker is done with synchronization. Streaming has already been
+ * ended by process_syncing_tables_for_sync. We should move to the
+ * next table if needed, or exit.
+ */
+ if (MyLogicalRepWorker->relsync_completed)
+ endofstream = true;
Here I think it is better to use bracketing { }
n;
-
- InitializingApplyWorker = true;
-
/* Attach to slot */
logicalrep_worker_attach(worker_slot);
+ Assert(am_tablesync_worker() || am_leader_apply_worker());
+
Why is the Assert not the very first statement of this function?
==
Kind Regards,
Peter Smith.
Fujitsu Australia
On Fri, Jul 21, 2023 at 5:24 PM Amit Kapila wrote:
>
> On Fri, Jul 21, 2023 at 12:05 PM Peter Smith wrote:
> >
> > On Fri, Jul 21, 2023 at 3:39 PM Amit Kapila wrote:
> > >
> > > The other thing I noticed is that we
> > > don't seem to be
On Fri, Jul 21, 2023 at 3:39 PM Amit Kapila wrote:
>
> On Fri, Jul 21, 2023 at 7:30 AM Peter Smith wrote:
> >
> > ~~~
> >
> > 2. StartLogRepWorker
> >
> > /* Common function to start the leader apply or tablesync worker. */
> > void
> >
Some review comments for v21-0002.
On Thu, Jul 20, 2023 at 11:41 PM Melih Mutlu wrote:
>
> Hi,
>
> Attached the updated patches with recent reviews addressed.
>
> See below for my comments:
>
> Peter Smith , 19 Tem 2023 Çar, 06:08 tarihinde şunu
> yazdı:
>>
>
p near the other "extern void
InitializeLogRepWorker(void)" might be less ambiguous.
--
[1] worker name in errmsg -
https://www.postgresql.org/message-id/CAA4eK1%2B%2BwkxxMjsPh-z2aKa9ZjNhKsjv0Tnw%2BTVX-hCBkDHusw%40mail.gmail.com
Kind Regards,
Peter Smith.
Fujitsu Australia
On Thu, Jul 20, 2023 at 11:41 PM Melih Mutlu wrote:
>
> Hi,
>
> Attached the updated patches with recent reviews addressed.
>
> See below for my comments:
>
> Peter Smith , 19 Tem 2023 Çar, 06:08 tarihinde şunu
> yazdı:
>>
>> Some review comments for v19
is just a reminder so the earlier review doesn't get
forgotten.
--
[1] v17-0003 review -
https://www.postgresql.org/message-id/CAHut%2BPuMAiO_X_Kw6ud-jr5WOm%2Brpkdu7CppDU6mu%3DgY7UVMzQ%40mail.gmail.com
Kind Regards,
Peter Smith.
Fujitsu Australia
n table synchronization worker" like the HEAD
code used to do.
It seems this mistake was introduced in patch v20-0001.
======
src/include/replication/worker_internal.h
8.
Refer to [1] for my reply to a previous review comment
--
[1] Replies to previous 0002 comments --
https://www.postgresql.org/message-id/CAHut%2BPtiAtGJC52SGNdobOah5ctYDDhWWKd%3DuP%3DrkRgXzg5rdg%40mail.gmail.com
Kind Regards,
Peter Smith.
Fujitsu Australia
On Tue, Jul 18, 2023 at 1:54 AM Melih Mutlu wrote:
>
> Hi,
>
> PFA updated patches. Rebased 0003 with minor changes. Addressed Peter's
> reviews for 0001 and 0002 with some small comments below.
>
> Peter Smith , 10 Tem 2023 Pzt, 10:09 tarihinde şunu
> yazdı:
>
externs to be separated from all the others, with those static
inline functions in-between.
--
Kind Regards,
Peter Smith.
Fujitsu Australia
On Tue, Jul 18, 2023 at 11:25 AM Peter Smith wrote:
>
> On Tue, Jul 18, 2023 at 1:54 AM Melih Mutlu wrote:
> >
> > Hi,
> >
> > PFA updated patches. Rebased 0003 with minor changes. Addressed Peter's
> > reviews for 0001 and 0002 with some small comment
-----
Kind Regards,
Peter Smith.
Fujitsu Australia
ots();
+ stop_postmaster(false);
+ }
IMO "the command" is a bit vague. It might be better to be explicit
and say "... because pg_resetwal would remove X..."
==
src/bin/pg_upgrade/pg_upgrade.h
13.
+typedef struct
+{
+ LogicalSlotInfo *slots;
+ int nslots;
+} LogicalSl
for me. How about attached?
> >
>
> The last line seems repetitive to me. So, I have removed it. Apart
> from that patch looks good to me. Sergie, Peter, and others, any
> thoughts?
The v5 patch LGTM.
--
Kind Regards,
Peter Smith.
Fujitsu Australia
t;slot_number" or
"slotnum" might be a better name.
And then the comment can also be simplified.
SUGGESTION
/* Slot number of this worker. */
int slotnum;
--
Kind Regards,
Peter Smith.
Fujitsu Australia
from either kind of worker. At least, when the function
was first introduced (around patch v43-0001? [1]) it was also
replacing some tablesync logs.
--
[1] v43-0001 -
https://www.postgresql.org/message-id/OS0PR01MB5716E366874B253B58FC0A23943C9%40OS0PR01MB5716.jpnprd01.prod.outlook.com
Kind Regards,
Peter Smith.
Fujitsu Australia
On Thu, Jul 13, 2023 at 12:22 PM Masahiko Sawada wrote:
>
> On Thu, Jul 13, 2023 at 11:12 AM Peter Smith wrote:
> >
> > On Thu, Jul 13, 2023 at 11:28 AM Masahiko Sawada
> > wrote:
> > >
> > > On Thu, Jul 13, 2023 at 8:03 AM Peter Smith wrote:
>
On Thu, Jul 13, 2023 at 11:28 AM Masahiko Sawada wrote:
>
> On Thu, Jul 13, 2023 at 8:03 AM Peter Smith wrote:
> >
> > On Wed, Jul 12, 2023 at 5:01 PM Masahiko Sawada
> > wrote:
> > >
> > > On Tue, Jul 11, 2023 at 5:31 PM Peter Smith wrote:
> &g
j%2BMEF0gM-TAV0%3Dfd3EfPoEsa%2BcMQLiWjyA%40mail.gmail.com
Kind Regards,
Peter Smith.
Fujitsu Australia
On Wed, Jul 12, 2023 at 5:01 PM Masahiko Sawada wrote:
>
> On Tue, Jul 11, 2023 at 5:31 PM Peter Smith wrote:
> >
> > Here are my comments for v4.
> >
> > ==
> >
> > Docs/Comments:
> >
> >
>
> Agreed. I've attached the
this source file.
SUGGESTION
Returns the OID of the index access method the opclass belongs to.
--
[1] https://www.postgresql.org/docs/devel/logical-replication-publication.html
Kind Regards,
Peter Smith.
Fujitsu Australia
. Yes, there are some tests for
*only* expressions like (expr, expr, expr), but IMO there should be
another test for an index like (expr, expr, column) which should also
fail because the column is not the leftmost field.
--
[1] Sawada-san doc for RI restriction -
https://www.postgresql.org/message-id/CAD21AoBzp9H2WV4kDagat2WUOsiYJLo10zJ1E5dZYnRTx1pc_g%40mail.gmail.com
Kind Regards,
Peter Smith.
Fujitsu Austalia
-Wk7uBiirAHF5quDY_1Z6WDoUKRZqkn%2Brg%40mail.gmail.com
Kind Regards,
Peter Smith.
Fujitsu Australia
e easier to compare the different PoC designs for
patch 0002 if there is no extra logic involved.
PoC design#1 -- each tablesync decides for itself what to do next
after it finishes
PoC design#2 -- reuse tablesync using a "pool" of available workers
--
Kind Regards,
Peter Smith.
Fujitsu Australia
e like this?
if (cmd->kind == REPLICATION_KIND_PHYSICAL)
{
Assert(!streamingDoneSending && !streamingDoneReceiving)
StartReplication(cmd);
}
else
{
/* Reset flags because reusing tablesync workers can mean this is the
second time here. */
streamingDoneSending = streamingDoneReceiving = false
On Mon, Jul 10, 2023 at 2:21 PM Amit Kapila wrote:
>
> On Mon, Jul 10, 2023 at 7:55 AM Peter Smith wrote:
> >
> > On Sat, Jul 8, 2023 at 1:49 PM Amit Kapila wrote:
> > >
> > > On Fri, Jul 7, 2023 at 1:36 PM Masahiko Sawada
> > > wrote:
> &
e it
work, a primary key should be defined on the subscriber table, or a
different appropriate replica identity must be specified.
2.
Maybe "REPLICA IDENTITY FULL" should have a link, like from this [1] page.
--
[1] 31.1 Publication =
https://www.postgresql.org/docs/current/logical-replication-
as
relXXX, so I wonder if is better for this to follow the same pattern.
e.g. 'relsync_completed'
~
10c.
"If true, no need to continue with that table.".
I am not sure if this sentence is adding anything useful.
--
Kind Regards,
Peter Smith.
Fujitsu Australia
ith that?
- Chose not to do it?
- Will do it raised in another thread?
--
[1] my review v2 -
https://www.postgresql.org/message-id/CAHut%2BPsFdTZJ7DG1jyu7BpA_1d4hwEd-Q%2BmQAPWcj1ZLD_X5Dw%40mail.gmail.com
[2] create index - https://www.postgresql.org/docs/current/sql-createindex.html
Kind Regards,
Peter Smith.
Fujitsu Australia
am_XXX
inline functions that follow it, so it doesn’t seem the best place to
jam this extern in the middle of that.
--
Kind Regards,
Peter Smith.
Fujitsu Australia
This comment text is similar to the docs change, so refer to the same
suggestions as #1 above.
--
Kind Regards,
Peter Smith.
Fujitsu Australia
On Wed, Jul 5, 2023 at 5:37 PM Daniel Gustafsson wrote:
>
> > On 4 Jun 2023, at 18:48, Amit Kapila wrote:
> >
> > On Fri, Jun 2, 2023 at 7:01 PM Peter Smith wrote:
> >>
> >> Hi, I noticed a feature description [1] referring to a command example:
> >
On Wed, Jul 5, 2023 at 4:32 PM Masahiko Sawada wrote:
>
> On Wed, Jul 5, 2023 at 2:46 PM Amit Kapila wrote:
> >
> > On Wed, Jul 5, 2023 at 9:01 AM Peter Smith wrote:
> > >
> > > Hi. Here are some review comments for this patch.
> > >
> > > +
the published table. These restrictions on the non-unique
index properties adhere to some of the restrictions that are enforced
for primary keys.
--
[1] Amit suggestions -
https://www.postgresql.org/message-id/CAA4eK1LZ_A-UmC_P%2B_hLi%2BPbwyqak%2BvRKemZ7imzk2puVTpHOA%40mail.gmail.com
Kind Regards,
Peter Smith.
Fujitsu Australia
Y FULL. IIUC it is not
> documented.
> Please see attched script to reproduce that. The final DELETE statement
> cannot be
> replicated at the subscriber on my env.
>
Missing attachment?
--
Kind Regards,
Peter Smith.
Fujitsu Australia
ut no plan
was declared and done_testing() was not seen.
t/034_hash.pl .. All 2 subtests passed
t/100_bugs.pl .. ok
Test Summary Report
---
t/034_hash.pl(Wstat: 0 Tests: 2 Failed: 0)
Parse errors: No plan found in TAP output
Files
). It sounds
sensible, but I guess you would need to first demonstrate the end
result will be much cleaner to get support for such a big refactor.
--
Kind Regards,
Peter Smith.
Fujitsu Australia
then you are currently hoping and relying that
there there are no differences in the wording of all the existing
messages. If something instead says "tablesync worker" you will
accidentally overlook it.
OTOH, this patch eliminates the guesswork and luck. In the example,
grepping for LR_WORKER_NAME_TABLESYNC will give you all the messages
you were looking for.
--
[1] Alvaro review comments -
https://www.postgresql.org/message-id/20230615103759.bkkv226czkcnuork%40alvherre.pgsql
[2] Horiguchi-san review comments -
https://www.postgresql.org/message-id/20230616.104327.1878440413098623268.horikyota.ntt%40gmail.com
Kind Regards,
Peter Smith.
Fujitsu Australia
FYI - I have created and tested back-patches for Amit's v5 patch,
going all the way to REL_10_STABLE.
(the patches needed tweaking several times due to minor code/docs
differences in the earlier versions)
PSA.
--
Kind Regards,
Peter Smith.
Fujitsu Australia
commit
message)
==
src/include/replication/walsender.h
34. CRSSnapshotAction
CRS_EXPORT_SNAPSHOT,
CRS_NOEXPORT_SNAPSHOT,
- CRS_USE_SNAPSHOT
+ CRS_USE_SNAPSHOT,
+ CRS_EXPORT_USE_SNAPSHOT
} CRSSnapshotAction;
~
Should the CRS_USE_SNAPSHOT be renamed to CRS_NOEXOPRT_USE_SNAPSHOT to
t to be another enum added (CRS_UNUSED?) and pass that instead.
~
Alternatively, maybe continue to pass 0, but ensure the existing enums
do not include any value of 0.
e.g.
typedef enum
{
CRS_EXPORT_SNAPSHOT = 1,
CRS_NOEXPORT_SNAPSHOT,
CRS_USE_SNAPSHOT
} CRSSnapshotAction;
--
Kind Regards,
Peter Smith.
Fujitsu Australia
". I think it would be better to use the corresponding
> enum
> value.
>
+1
--
Kind Regards,
Peter Smith.
Fujitsu Australia
On Thu, Jun 15, 2023 at 4:14 PM Peter Smith wrote:
>
> On Thu, Jun 8, 2023 at 1:24 PM Masahiko Sawada wrote:
> >
> ...
>
> > We also need to research how to integrate the initial schema
> > synchronization with tablesync workers. We have a PoC patch[2].
> >
ic.patch
error: patch failed: src/backend/replication/logical/tablesync.c:1245
error: src/backend/replication/logical/tablesync.c: patch does not apply
--
Kind Regards,
Peter Smith.
Fujitsu Australia
in.
~~
It is better to have a *single* point where these worker names are
defined, so then all output uses identical LR worker nomenclature.
PSA a small patch to modify the code accordingly. This is not intended
to be a functional change - just a code cleanup.
Thoughts?
--
Kind Regards,
Peter
_to_reuse;
+
IIUC this field has no meaning except for a tablesync worker, but the
fieldname give no indication of that at all.
To make this more obvious it might be better to put this with the
other tablesync fields:
/* Used for initial table synchronization. */
Oid relid;
char relstate;
XLogRecPtr relstate_lsn;
slock_t relmutex;
And maybe rename it according to that convention relXXX -- e.g.
'relworker_available' or something
--
Kind Regards,
Peter Smith.
Fujitsu Australia
On Wed, Jun 14, 2023 at 1:10 PM Amit Kapila wrote:
>
> On Wed, May 17, 2023 at 11:57 AM Peter Smith wrote:
> >
> > On Wed, May 17, 2023 at 2:53 PM Robert Sjöblom
> > wrote:
> > >
> > > Attached is the revised version.
> > >
> >
>
etail/391/
[2] https://www.postgresql.org/docs/16/sql-createpublication.html
Kind Regards,
Peter Smith.
Fujitsu Australia
On Thu, Jun 1, 2023 at 7:22 AM Melih Mutlu wrote:
>
> Hi Peter,
>
> Peter Smith , 26 May 2023 Cum, 10:30 tarihinde
> şunu yazdı:
> >
> > On Thu, May 25, 2023 at 6:59 PM Melih Mutlu wrote:
> > Yes, I was mostly referring to the same as point 1 below about pat
On Thu, May 25, 2023 at 6:59 PM Melih Mutlu wrote:
>
> Hi,
>
>
> Peter Smith , 24 May 2023 Çar, 05:59 tarihinde şunu
> yazdı:
>>
>> Hi, and thanks for the patch! It is an interesting idea.
>>
>> I have not yet fully read this thread, so below are
r->ready_to_reuse = true;
SpinLockRelease(>relmutex);
+ }
+
+ SpinLockRelease(>relmutex);
Isn't there a double release of that mutex happening there?
--
Kind Regards,
Peter Smith.
Fujitsu Australia
On Wed, May 17, 2023 at 2:53 PM Robert Sjöblom
wrote:
>
> Den ons 17 maj 2023 kl 03:18 skrev Peter Smith :
> >
> > + errhint("Use %s to disassociate the subscription from the slot after
> > disabling the subscription.",
> >
> > IMO it looked strange
low this thread because the subject
keeps changing.
--
Kind Regards,
Peter Smith.
Fujitsu Australia
Hi Kuroda-san,
I confirmed the patch changes from v13-0001 to v14-0001 have addressed
the comments from my previous post, and the cfbot is passing OK, so I
don't have any more review comments at this time.
--
Kind Regards,
Peter Smith.
Fujitsu Australia
errhint in a similar way. There was no reply to Amit's
idea, so it's not clear whether it's an accidental omission from your
v2 patch or not.
--
[1]
https://www.postgresql.org/message-id/CAA4eK1J11phiaoCOmsjNqPZ9BOWyLXYrfgrm5vU2uCFPF2kN1Q%40mail.gmail.com
Kind Regards,
Peter Smith.
Fujitsu Australia
patch.
e.g. see get_logical_slot_infos:
pg_log(PG_VERBOSE, "Database: %s", pDbInfo->db_name);
--
Kind Regards,
Peter Smith.
Fujitsu Australia
https://www.postgresql.org/message-id/TYAPR01MB5866BD618DEE62AF1836E612F5749%40TYAPR01MB5866.jpnprd01.prod.outlook.com
Kind Regards,
Peter Smith.
Fujitsu Australia
On Thu, May 11, 2023 at 6:30 PM Daniel Gustafsson wrote:
>
> > On 11 May 2023, at 07:41, Peter Smith wrote:
>
> > While reviewing another patch for the file info.c, I noticed some
> > misplaced colon (':') in the verbose logs for print_rel_infos().
>
>
Hi --
While reviewing another patch for the file info.c, I noticed some
misplaced colon (':') in the verbose logs for print_rel_infos().
PSA patch to fix it.
--
Kind Regards,
Peter Smith.
Fujitsu Australia
v1-0001-pg_upgrade-typo-in-verbose-log.patch
Description: Binary data
401 - 500 of 1527 matches
Mail list logo
