n the sub but it was reported as missing column. I think
we should somehow report like "generated column on publisher is replicated the
generated column on the subscriber".
Best regards,
Hayato Kuroda
FUJITSU LIMITED
ead tuples.
Best regards,
Hayato Kuroda
FUJITSU LIMITED
t it seemed that entries can be removed when it behinds
GetOldestNonRemovableTransactionId(NULL),
i.e., horizons.shared_oldest_nonremovable. The value is affected by the
replication
slots so that interesting commit_ts entries for us are not removed.
Best regards,
Hayato Kuroda
FUJITSU LIMITED
is topic introduces huge complexity and is not mandatory for
update_deleted
detection. We can work on it in later versions based on the needs.
Best regards,
Hayato Kuroda
FUJITSU LIMITED
s much as possible. This feature is related
with the replication so that changes should be closed within the replication
subdir.
[1]:
https://www.postgresql.org/message-id/TYAPR01MB5692541820BCC365C69442FFF54F2%40TYAPR01MB5692.jpnprd01.prod.outlook.com
Best regards,
Hayato Kuroda
FUJITSU LIMITED
I'm not sure it is correct. Why do you skip the generated column even when it
is in
the column list? Also, can you add comments what you want to do?
Best regards,
Hayato Kuroda
FUJITSU LIMITED
t.
```
Based on the comment, this case does not test the behavior of generated columns
anymore. So, I felt column 'd' could be removed from the case.
03. 031_column_list.pl
Can we test that generated columns won't be replaced if it does not included in
the column list?
[1]:
)"
```
I think no need to separate lines and add bracket. How about like below?
```
"SELECT DISTINCT gpt.attrs"
```
Best regards,
Hayato Kuroda
FUJITSU LIMITED
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 1
gresql.org/message-id/OS0PR01MB5716A78EE3D6F2F4754E1209946C2%40OS0PR01MB5716.jpnprd01.prod.outlook.com
Best regards,
Hayato Kuroda
FUJITSU LIMITED
test_1025.sh
Description: test_1025.sh
ns with streaming=off, it was chosen because it was a default.
I cannot find strong reasons to keep current behavior.
Best regards,
Hayato Kuroda
FUJITSU LIMITED
dXlogFiles() and
RemoveXlogFile().
I want to see the log when you can reproduce.
They are inspired by [1]. I doubt the thread and yours are the same issue or
not.
[1]: https://www.postgresql.org/message-id/flat/Yz2hivgyjS1RfMKs%40depesz.com
Best regards,
Hayato Kuroda
FUJITSU LIMITED
o pgoutput layer as
"relation". I.e., I think it is enough to invalidate relcache of leaf.
> I have also modified the tests in 0001 patch. These changes are only
> related to syntax of writing tests.
LGTM. I found small improvements, please find the attached.
Best regard
ould be avoided
in any case, and everyone knows it, I agree with your point.
One comment for your patch;
Shouldn't we add a streaming=off case for pg_dump test? You added lines but no
one
passes it.
Best regards,
Hayato Kuroda
FUJITSU LIMITED
think?
Best regards,
Hayato Kuroda
FUJITSU LIMITED
add_invalidations.diffs
Description: add_invalidations.diffs
ust to confirm - you do not push rb_mem_block_size patch and
just replace SLAB_LARGE_BLOCK_SIZE -> SLAB_DEFAULT_BLOCK_SIZE, right? It seems
that
only reorderbuffer.c uses the LARGE macro so that it can be removed.
Best regards,
Hayato Kuroda
FUJITSU LIMITED
//www.postgresql.org/message-id/OS0PR01MB5716E0A283D1B66954CDF5A694682%40OS0PR01MB5716.jpnprd01.prod.outlook.com
Best regards,
Hayato Kuroda
FUJITSU LIMITED
ww.postgresql.org/docs/devel/logical-replication-security.html
Best regards,
Hayato Kuroda
FUJITSU LIMITED
n and must
invalidate relcaches there.
2.
Similarly with above, the relcache won't be invalidated when ALTER PUBLICATION
OWNER TO is executed. This means that privilage checks may be ignored if the
entry
is still valid. Not sure, but is there a possibility this causes an
inconsistency?
Best rega
as shared in [1] and observed a significant
> decrease in the degradation with the new changes. With selective
> invalidation degradation is around ~5%. This results are an average of
> 3 runs.
IIUC, the executed workload did not contain ALTER SCHEMA command, so
third improvement did not contribute this improvement.
Best regards,
Hayato Kuroda
FUJITSU LIMITED
ration-checklist-for-oracle-grid-infrastructure.html
Best regards,
Hayato Kuroda
FUJITSU LIMITED
total: 721420288 bytes in 86 blocks; 1415344 free (0 chunks); 720004944
use
Best regards,
Hayato Kuroda
FUJITSU LIMITED
test.sh
Description: test.sh
Thanks everyone for many efforts!
[1]:
https://github.com/postgres/postgres/commit/4f08ab55457751308ffd8d33e82155758cd0e304
Best regards,
Hayato Kuroda
FUJITSU LIMITED
s NULL, invalid
>
> LGTM.
> Also I added the following to the example for clarity:
>
> postgres=# SELECT * FROM postgres_fdw_get_connections(true);
+1.
Best regards,
Hayato Kuroda
FUJITSU LIMITED
more
> than 4GB memory.
I appreciate your work on logical replication and am interested in the thread.
I've heard this issue from others, and this has been the barrier to using
logical
replication. Please let me know if I can help with benchmarking, other
measurements, etc.
Best regards,
Hayato Kuroda
FUJITSU LIMITED
name is quoted.
0002:
03.
```
-#include "replication/logicalrelation.h"
```
Just to confirm - this removal is not related with the feature but just the
improvement, right?
Best regards,
Hayato Kuroda
FUJITSU LIMITED
EANUP()/PG_END_ENSURE_ERROR_CLEANUP
macros here. According to codes, it assumes that any before-shmem callbacks are
not registered within the block because the cleanup function is registered and
canceled
within the macro. LogicalRepApplyLoop() can register the function when
it handles COMMIT PREPAR
``
@@ -3297,6 +3297,8 @@ apply_dispatch(StringInfo s)
saved_command = apply_error_callback_arg.command;
apply_error_callback_arg.command = action;
+elog(LOG, "XXX got message %d", action);
```
Best regards,
Hayato Kuroda
FUJITSU LIMITED
test_0809.sh
Description: test_0809.sh
same result as when the stream option is not "parallel."
- XactLastCommitEnd was replaced even ROLLBACK PREPARED case.
Since the COMMIT PREPARED record is flushed in
RecordTransactionAbortPrepared(),
there is no need to ensure the WAL must be sent.
Best regards,
Hayato Kuroda
FUJITS
nd;
>
Opps. Fixed version is posted in [1].
[1]:
https://www.postgresql.org/message-id/TYAPR01MB5692FAC23BE40C69DA8ED4AFF5B92%40TYAPR01MB5692.jpnprd01.prod.outlook.com
Best regards,
Hayato Kuroda
FUJITSU LIMITED
ql.org/message-id/CAA4eK1L-r8OKGdBwC6AeXSibrjr9xKsg8LjGpX_PDR5Go-A9TA%40mail.gmail.com
Best regards,
Hayato Kuroda
FUJITSU LIMITED
v2-0001-Prevent-origin-progress-advancement-if-failed-to-.patch
Description: v2-0001-Prevent-origin-progress-advancement-if-failed-to-.patch
test_2pc.sh
Description: test_2pc.sh
eign table with the given OID. A
> - ForeignTable object contains properties of
> the
> - foreign table (see foreign/foreign.h for details).
> -
> -
> -
> -
>
> Why did you remove these code? Just mistake?
Oh, my fault. I tried to remove GetUserMappingByOid() and the entry was also
Removed at that time. Restored.
Best regards,
Hayato Kuroda
FUJITSU LIMITED
v3-0001-Extend-postgres_fdw_get_connections-to-return-use.patch
Description: v3-0001-Extend-postgres_fdw_get_connections-to-return-use.patch
uce a global variable which is turned on when the
conflict
is found.
Thought?
Best regards,
Hayato Kuroda
FUJITSU LIMITED
ix patch. Feel free to include in the next version.
Best regards,
Hayato Kuroda
FUJITSU LIMITED
fixes_for_v11.patch
Description: fixes_for_v11.patch
hat we can keep the code consistent with the server case.
The part does not read data from the entry, and we can follow.
Best regards,
Hayato Kuroda
FUJITSU LIMITED
v2-0001-Extend-postgres_fdw_get_connections-to-return-use.patch
Description: v2-0001-Extend-postgres_fdw_get_connections-to-return-use.patch
ublic.tab" in transaction 740, finished at
0/1543940
```
=
Can we support the type of conflict?
[1]:
https://www.postgresql.org/docs/devel/sql-createtable.html#SQL-CREATETABLE-EXCLUDE
Best regards,
Hayato Kuroda
FUJITSU LIMITED
ments for parallel
apply.
Best regards,
Hayato Kuroda
FUJITSU LIMITED
120
Model name:Intel(R) Xeon(R) CPU E7-4890 v2 @ 2.80GHz
Core(s) per socket:15
Socket(s): 4
Best regards,
Hayato Kuroda
FUJITSU LIMITED
test.sh
Description: test.sh
nology and
> a similar operating mode, so it would make sense to keep this consistent.
+1. If so, should we say "default current dir." instead of "default current
directory" in usage()
because pg_upgrade says like that?
Best regards,
Hayato Kuroda
FUJITSU LIMITED
nfo;
+ targetrel = rel;
+
```
Here I can say the same thing as 1.
Best regards,
Hayato Kuroda
FUJITSU LIMITED
-e198dded0bac%40gmail.com
Best regards,
Hayato Kuroda
FUJITSU LIMITED
ct that
users will use.
Overall, I could agree to use pg_log_standby_snapshot(). This flushes the WAL
asynchronously but ensures the timing is not so delayed.
Best regards,
Hayato Kuroda
FUJITSU LIMITED
`GetUserMappingFromOid` to
`GetUserMappingByOid`
to keep the name consistent with others.
- Comments and docs were updated.
Best regards,
Hayato Kuroda
FUJITSU LIMITED
0001-Extend-postgres_fdw_get_connections-to-return-user-n.patch
Description: 0001-Extend-postgres_fdw_get_connections-to-return-user-n.patch
fd.revents &
> + (POLLRDHUP | POLLHUP | POLLERR |
> POLLNVAL)) ? 1 : 0;
I think you are right.
According to the man page, the input socket is invalid or disconnected if
revents
has such bits. So such cases should be also regarded as *failure*.
After the fix, th
://www.postgresql.org/message-id/768032ee-fb57-494b-b674-1ccb65b6f969%40oss.nttdata.com
Best regards,
Hayato Kuroda
FUJITSU LIMITED
v44-0001-doc-Enhance-documentation-for-postgres_fdw_get_c.patch
Description: v44-0001-doc-Enhance-documentation-for-postgres_fdw_get_c.patch
v44-0002-postgres_fdw-Add
ng, but can you clarify why (Datun) 0
is returned instead of PG_RETURN_VOID()?
[1]:
https://www.postgresql.org/message-id/3ee79f2c-e8b3-4342-857c-a31b87e1afda%40eisentraut.org
Best regards,
Hayato Kuroda
FUJITSU LIMITED
Dear Fujii-san,
>
> Thanks for updating the patches!
>
> I’ve created a new base patch and split the v42-0001 patch into two parts
> to implement the feature and improvements step by step. After pushing these
> patches, I’ll focus on the v42-0002 patch next.
Best regards,
Hay
PI for table scan as well?)
[1]:
```
vacuumlazy.c: In function ‘lazy_scan_prune’:
vacuumlazy.c:1666:34: error: ‘LVRelState’ {aka ‘struct LVRelState’} has no
member named ‘NewRelminMxid’
Assert(MultiXactIdIsValid(vacrel->NewRelminMxid));
^~
```
Best regards,
Hayato Kuroda
FUJITSU LIMITED
Dear Fujii-san,
> On 2024/07/18 19:49, Hayato Kuroda (Fujitsu) wrote:
> >> Shouldn't this test also check if the returned user_name is valid?
> >
> > You meant to say that we should print the user_name, right? Done.
>
> Yes, I think it's better to test if
R or 2) apply worker exits. 0002 patch fixes the issue.
How do you think?
[1]:
https://www.postgresql.org/message-id/flat/8fab8-65d74c80-1-2f28e880@39088166
Best regards,
Hayato Kuroda
FUJITSU LIMITED
test_2pc.sh
Description: test_2pc.sh
0001-Add-XactLastPrepareEnd-to-indicate-the-last-PREPARE
so, in create_logical_replication_slots(),
we may have to check the located database for every slot and connect to the
appropriate database. These changes make it difficult to parallelize the
operation.
[1]:
https://www.postgresql.org/message-id/flat/20240516211638.GA1688936@nathanxps13
Best regards,
Hayato Kuroda
FUJITSU LIMITED
blisher cannot be modified if the slot is currently acquired by the
* existing walsender.
+ *
+ * XXX: when toggling two_phase from "false" to "true", the slot parameter
+ * is not modified by the backend process so that the lock conflict won't
+ * occur. The restarted walsender will do the alternation. Therefore, we
+ * can allow to switch without the restriction. This can be changed in
+ * the future based on the requirement.
```
Best regards,
Hayato Kuroda
FUJITSU LIMITED
found an inconsistency of name between source and doc,
so I unified to postgres_fdw_can_verify_connection().
- Also, test patch (0002) was combined into this.
0002:
- user_name was added after the `valid` to preserve ordering of the old version.
- GetUserMappingFromOid() is allowed to miss a tuple.
[1
, I started to think patches can be merged in future versions because
they must be included at once and codes are not so long. Thought?
Best regards,
Hayato Kuroda
FUJITSU LIMITED
v20-0001-Allow-altering-of-two_phase-option-of-a-SUBSCRIP.patch
Description: v20-0001-Allow-altering-of-two_phase
erted just after the restart_lsn
but before the RUNNING_XACT record? In this case, yes, the streaming replication
finishes before replicating tuples but logical replication will skip them.
Euler's approach cannot be used as-is.
Best regards,
Hayato Kuroda
FUJITSU LIMITED
er.
How do you think?
[1]:
https://www.postgresql.org/message-id/osbpr01mb25521b15bf950d2523bbe143f5...@osbpr01mb2552.jpnprd01.prod.outlook.com
[2]:
https://www.postgresql.org/message-id/b1f0f8c7-8f01-4950-af77-339df3dc4684%40app.fastmail.com
Best regards,
Hayato Kuroda
FUJITSU LIMITED
s
> function simpler.
This style was needed to preserve error condition for failover option. Not
changed.
[1]:
https://www.postgresql.org/message-id/OS3PR01MB571834FBD3E6D3804484038F94A32%40OS3PR01MB5718.jpnprd01.prod.outlook.com
Best regards,
Hayato Kuroda
FUJITSU LIMITED
v19-0001-Allow-alter
gresql.org/message-id/OS3PR01MB57184E0995521300AC06CB4B94A72%40OS3PR01MB5718.jpnprd01.prod.outlook.com
Best regards,
Hayato Kuroda
FUJITSU LIMITED
v18-0001-Allow-altering-of-two_phase-option-of-a-SUBSCRIP.patch
Description: v18-0001-Allow-altering-of-two_phase-option-of-a-SUBSCRIP.patch
v18-0002-
quired in [2].
Previous patch set could not be accepted due to the initialization miss.
PSA new version.
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
https://www.fujitsu.com/
v17-0001-Allow-altering-of-two_phase-option-of-a-SUBSCRIP.patch
Description: v17-0001-Allow-altering-of-two_phase-option-of-a-SU
https://www.postgresql.org/message-id/CAA4eK1%2BFRrL_fLWLsWQGHZRESg39ixzDX_S9hU8D7aFtU%2Ba8uQ%40mail.gmail.com
[2]:
https://www.postgresql.org/message-id/CAA4eK1Khy_YWFoQ1HOF_tGtiixD8YoTg86coX1-ckxt8vK3U%3DQ%40mail.gmail.com
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
https://www.fujitsu.com/
v17
touched here. Let's make the scope
narrower.
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
https://www.fujitsu.com/
and see the reason? Based on the requirement I can
provide further information.
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
https://www.fujitsu.com/
parallel_test.sh
Description: parallel_test.sh
ql(
+ 'postgres', "
+ALTER SUBSCRIPTION tap_sub_copy SET (two_phase = false);
+ALTER SUBSCRIPTION tap_sub_copy ENABLE;");
```
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
https://www.fujitsu.com/
dded
in 0002 to clarify why we must wait. For 0003, a comment was added and
setting for standby was reverted.
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
https://www.fujitsu.com/
v3-0001-emit-dummy-message-while-setting-up-the-publisher.patch
Description: v3-0001-emit-dummy-message-while-s
his fixes a second failure.
[1]:
https://www.postgresql.org/message-id/0dffca12-bf17-4a7a-334d-225569de5e6e%40gmail.com
[2]:
https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=adder&dt=2024-07-02%2008%3A45%3A39
[3]:
https://www.postgresql.org/message-id/OSBPR01MB25521B15BF950D2523B
e
promotion. It can generate a WAL record so that standby can finish after the
application. But I'm not sure how do we do and it seems to lead an additional
timing issue. Also, this does not improve the behavior of the command - normal
user may have to wait some time by the command.
Do you hav
after the modification, the reported failure [1] could not be
resolved on my env.
How do you think?
[1]:
https://www.postgresql.org/message-id/0dffca12-bf17-4a7a-334d-225569de5e6e%40gmail.com
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
https://www.fujitsu.com/
emit_dummy_message.diff
Description: emit_dummy_message.diff
tion on node_p
> and do a test similar to what we are doing in "# Create failover slot
> to test its removal".
Your approach looks better than mine. I followed the approach.
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
https://www.fujitsu.com/
v4-0001-pg_createsubscriber-remove
nge, but I hoped
I checked your comments were not reproduced. Also, 0001 was created to remove an
unused attribute.
> Shouldn't this be an open item for PG17?
Added this thread to wikipage.
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
https://www.fujitsu.com/
v2-0001-pg_createsubscriber-rem
ked now because I don't think it is
helpful,
but it can be easily added. I did not add test for that, because current test
code
does not check outputs.
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
https://www.fujitsu.com/
v2-0001-pg_createsubscriber-Fix-cases-which-connection-pa.patch
Desc
eation of subscription.
+ */
```
I think this comment is not correct. After patching, all tablesync command
becomes
like COPY (SELECT ...) if include_genereted_columns is set to true. Is it right?
Can we restrict only when the table has generated ones?
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
https://www.fujitsu.com/
T oid FROM pg_database
WHERE datname = current_database())
```
Based on that I just added a comma in 0004 patch.
[1]: https://cirrus-ci.com/task/6710166165389312
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
https://www.fujitsu.com/
cess will wait for running XIDs. On the
> other hand, an INSERT/UPDATE/DELETE acquires its RowExclusiveLock and
> builds
> its relcache entry before assigning an XID, so perhaps the snapbuild.c process
> isn't enough to prevent that thread's race condition. What do you thi
jrRQ63ju8P41tBkdjkQb4X9uEdLK_Wkauxum1MVUdfA%40mail.gmail.com
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
https://www.fujitsu.com/
ch to remove the attribute.
How do you think?
[1]:
https://www.postgresql.org/message-id/CAA4eK1J22UEfrqx222h5j9DQ7nxGrTbAa_BC%2B%3DmQXdXs-RCsew%40mail.gmail.com
[2]:
https://www.postgresql.org/message-id/CANhcyEWvimA1-f6hSrA%3D9qkfR5SonFb56b36M%2B%2BvT%3DLiFj%3D76g%40mail.gmail.com
Best Regards,
Hayat
ction.
*/
XLogRecPtr
GetInsertRecPtr(void)
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
https://www.fujitsu.com/
ww.postgresql.org/message-id/CANhcyEWvimA1-f6hSrA%3D9qkfR5SonFb56b36M%2B%2BvT%3DLiFj%3D76g%40mail.gmail.com
[3]:
https://www.postgresql.org/message-id/5d5dd4cd-6359-4109-88e8-c8e13035ae16%40enterprisedb.com
[4]:
https://www.postgresql.org/message-id/CAA4eK1LZxYxcbeiOn3Q5hjXVtZKhJWj-fQtndAeTCvZrPev8BA%40mail.gmail.
the default is
set
to off, we can keep the current specification.
Thought?
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
https://www.fujitsu.com/
www.postgresql.org/message-id/CANhcyEV6q1Vhd37i1axUeScLi0UAGVxta1LDa0BV0Eh--TcPMg%40mail.gmail.com
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
https://www.fujitsu.com/
if (att->attgenerated && !publish_generated_column)
continue;
```
I think changes in v2 was reverted or wrongly merged.
08. test code
Can you add tests that generated columns are replicated by the logical
replication?
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
https://www.fujitsu.com/
gresql.org/message-id/OSBPR01MB25522052F9F3E3AAD3BA2A8CF5ED2%40OSBPR01MB2552.jpnprd01.prod.outlook.com
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
https://www.fujitsu.com/
a patch for confirmation purpose. This worked well on my environment.
Ian, how about you?
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
https://www.fujitsu.com/
fix_029.diff
Description: fix_029.diff
better to refer only to true|false
> everywhere for these boolean parameters, instead of sometimes using
> different values like on|off.
>
> What do you think?
It's OK for me to make message/code comments consistent. Not sure the
documentation,
but followed only my part.
[
rce_alter
> = true);");
> +$node_subscriber->safe_psql('postgres', "ALTER SUBSCRIPTION
> regress_sub ENABLE;");
> +
>
> What does "Apart from the above" mean? Be more explicit.
Clarified like "Apart from the last ALTER SUBSCRIPTION command...".
> 9.
> +# Verify the prepared transaction are aborted
> $result = $node_subscriber->safe_psql('postgres',
> "SELECT count(*) FROM pg_prepared_xacts;");
> is($result, q(0), "prepared transaction done by worker is aborted");
>
> /transaction are aborted/transaction was aborted/
Fixed.
[1]:
https://www.postgresql.org/message-id/OSBPR01MB2552FEA48D265EA278AA9F7AF5E22%40OSBPR01MB2552.jpnprd01.prod.outlook.com
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
https://www.fujitsu.com/
"off".
> +$result = $node_subscriber->safe_psql('postgres',
> +"SELECT count(*) FROM pg_prepared_xacts;");
> +is($result, q(0), "prepared transaction done by worker is aborted");
> +
>
> /the prepared transaction are aborted/any prepared transactions are aborted/
Fixed.
[1]:
https://www.postgresql.org/message-id/OSBPR01MB2552FEA48D265EA278AA9F7AF5E22%40OSBPR01MB2552.jpnprd01.prod.outlook.com
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
https://www.fujitsu.com/
h(),
and 2) subscription is created before attaching. In this case, the launcher will
become un-sleepable because the latch is set but won't be reset. It may waste
the
CPU time.
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
https://www.fujitsu.com/
off, force_alter = on);");
>
> "force" is a verb, so it is better to say 'force_alter = true' instead
> of 'force_alter = on'.
Fixed. Actually not sure it is better because I'm not a native.
> 8.
> $result = $node_subscriber->safe_psql('post
# verify the inserted rows got replicated ok
They were fixed based on your previous comments.
>
> 8. TAP test - subscription name
>
> It's better to rename the SUBSCRIPTION in this TAP test so you can
> avoid getting log warnings like:
>
> psql::4: WARNING: subscriptions created by regression test
> cases should have names starting with "regress_"
> psql::4: NOTICE: created replication slot "sub" on publisher
Modified, but it was included in 0001.
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
https://www.fujitsu.com/
re a transaction to insert some rows to the table
>
> # verify the prepared tx is not yet replicated to the subscriber
> (because 'two_phase = off')
>
> # toggle the two_phase to 'on' *before* the COMMIT PREPARED
>
> # verify the inserted rows got replicated ok
Modified like yours, but changed based on the suggestion by Grammarly.
> 8.
> IIUC this test will behave the same even if you DON'T do the toggle
> 'two_phase = on'. So I wonder is there something more you can do to
> test this scenario more convincingly?
I found an indicator. When the apply starts, it outputs the current status of
two_phase option. I added wait_for_log() to ensure below appeared. Thought?
```
ereport(DEBUG1,
(errmsg_internal("logical replication apply worker for
subscription \"%s\" two_phase is %s",
MySubscription->name,
MySubscription->twophasestate == LOGICALREP_TWOPHASE_STATE_DISABLED ?
"DISABLED" :
MySubscription->twophasestate == LOGICALREP_TWOPHASE_STATE_PENDING ? "PENDING" :
MySubscription->twophasestate == LOGICALREP_TWOPHASE_STATE_ENABLED ? "ENABLED" :
"?")));
```
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
https://www.fujitsu.com/
rror message */
> + ereport(ERROR,
> + (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
> + errmsg("cannot disable two_phase when uncommitted prepared
> transactions present"),
> + errhint("Resolve these transactions and try again")));
>
> The comment "/* Add error message */" seems unnecessary.
Yeah, this was an internal flag. Removed.
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
https://www.fujitsu.com/
change()
data->include_generated_columns is referred four times in the function.
Can you store the value to a varibable?
c. pg_decode_change()
```
-true);
+true, data->include_generated_columns );
```
Please remove th
ublisher->safe_psql(
> + 'postgres', qq{
> +BEGIN;
> +INSERT INTO tab_copy VALUES (100);
> +PREPARE TRANSACTION 'newgid';
> + });
> +
>
> 18a.
> /on publisher/on the publisher/
Fixed.
> 18b.
> What is that "DROP SUBS
Dear hackers,
Per recent commit (b29cbd3da), our patch needed to be rebased.
Here is an updated version.
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
https://www.fujitsu.com/
v6-0001-Allow-altering-of-two_phase-option-of-a-SUBSCRIPT.patch
Description: v6-0001-Allow-altering-of-two_phase
ns
> for now.
Attached patch set is a ported version for PG16, which breaks ABI. This can be
used
for testing purpose, but it won't be pushed to REL_16_STABLE.
At least, this patchset can pass my github CI.
Can you apply and check whether your issue is solved?
Best Regards,
Haya
aborted at that time.
```
subscriber=# ALTER SUBSCRIPTION sub SET (two_phase = off, force_alter = on);
ALTER SUBSCRIPTION
subscriber=# SELECT * FROM pg_prepared_xacts ;
transaction | gid | prepared | owner | database
-+-+--+---+------
(0 rows)
```
Best Regard
se.
IIUC, Adding a new feature (e.g., replication command) for minor updates is
generally
prohibited
We must consider another approach for backpatching, but we do not have solutions
for now.
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
https://www.fujitsu.com/global/
in the upgrade mode, but
your patch seems to remove the condition. Can you clarify the reason?
Other than that, the patch LGTM.
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
https://www.fujitsu.com/
_phase parameters
> specified in the subscription"
>
> [1]:
> https://www.postgresql.org/docs/devel/sql-altersubscription.html#SQL-ALTER
> SUBSCRIPTION-PARAMS-SET
I see, thanks for the clarification. Agreed that the description is not
conflict with
option 2.
Best Regards
ould not refer to the design of two_phase here, because
> two_phase can be considered as a streaming option, so it's fine to
> change the two_phase along with START_REPLICATION command. (the
> two_phase is not changed in subscription DDLs, but get changed in
> START_REPLICATION command). But the failover is closely related to a
> replication slot itself.
>
Sorry, I cannot find statements. Where did you refer?
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
https://www.fujitsu.com/
.
Also, should we add the restriction to the doc? I feel [1] can be updated.
[1]:https://www.postgresql.org/docs/devel/sql-altersubscription.html#SQL-ALTERSUBSCRIPTION-PARAMS-SET
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
https://www.fujitsu.com/
coded.
==
Attached zip file contains the PoC and used script. You can refer what I really
did.
Best Regards,
Hayato Kuroda
FUJITSU LIMITED
https://www.fujitsu.com/
<>
1 - 100 of 599 matches
Mail list logo
