From: | "Zhijie Hou (Fujitsu)" <houzj(dot)fnst(at)fujitsu(dot)com> |
---|---|
To: | Peter Smith <smithpb2250(at)gmail(dot)com> |
Cc: | pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Nisha Moond <nisha(dot)moond412(at)gmail(dot)com>, "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com> |
Subject: | RE: Collect statistics about conflicts in logical replication |
Date: | 2024-08-26 12:12:53 |
Message-ID: | OS3PR01MB5718B2910401BD8A4217DFEC948B2@OS3PR01MB5718.jpnprd01.prod.outlook.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Monday, August 26, 2024 3:30 PM Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
>
> ======
> src/include/replication/conflict.h
>
> nit - defined 'NUM_CONFLICT_TYPES' inside the enum (I think this way is
> often used in other PG source enums)
I think we have recently tended to avoid doing that, as it has been commented
that this style is somewhat deceptive and can cause confusion. See a previous
similar comment[1]. The current style follows the other existing examples like:
#define IOOBJECT_NUM_TYPES (IOOBJECT_TEMP_RELATION + 1)
#define IOCONTEXT_NUM_TYPES (IOCONTEXT_VACUUM + 1)
#define IOOP_NUM_TYPES (IOOP_WRITEBACK + 1)
#define BACKEND_NUM_TYPES (B_LOGGER + 1)
...
> ======
> src/test/subscription/t/026_stats.pl
>
> 1.
> + # Delete data from the test table on the publisher. This delete
> + operation # should be skipped on the subscriber since the table is already
> empty.
> + $node_publisher->safe_psql($db, qq(DELETE FROM $table_name;));
> +
> + # Wait for the subscriber to report tuple missing conflict.
> + $node_subscriber->poll_query_until(
> + $db,
> + qq[
> + SELECT update_missing_count > 0 AND delete_missing_count > 0 FROM
> + pg_stat_subscription_stats WHERE subname = '$sub_name'
> + ])
> + or die
> + qq(Timed out while waiting for tuple missing conflict for
> subscription '$sub_name');
>
> Can you write a comment to explain why the replicated DELETE is
> expected to increment both the 'update_missing_count' and the
> 'delete_missing_count'?
I think the comments several lines above the wait explained the reason[2]. I
slightly modified the comments to make it clear.
Other changes look good to me and have been merged, thanks!
Here is the V2 patch.
[1] https://www.postgresql.org/message-id/202201130922.izanq4hkkqnx%40alvherre.pgsql
[2]
..
# Truncate test table to ensure the upcoming update operation is skipped
# and the test can continue.
$node_subscriber->safe_psql($db, qq(TRUNCATE $table_name));
Best Regards,
Hou zj
Attachment | Content-Type | Size |
---|---|---|
v2-0001-Collect-statistics-about-conflicts-in-logical-rep.patch | application/octet-stream | 24.9 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Zhijie Hou (Fujitsu) | 2024-08-26 12:14:19 | RE: Conflict detection and logging in logical replication |
Previous Message | Alena Rybakina | 2024-08-26 11:55:13 | Re: Vacuum statistics |