RE: Conflict detection and logging in logical replication

Zhijie Hou (Fujitsu) <houzj.fnst@fujitsu.com>

From: "Zhijie Hou (Fujitsu)" <houzj.fnst@fujitsu.com>
To: shveta malik <shveta.malik@gmail.com>
Cc: Nisha Moond <nisha.moond412@gmail.com>, pgsql-hackers <pgsql-hackers@postgresql.org>, Dilip Kumar <dilipbalaut@gmail.com>, Jan Wieck <jan@wi3ck.info>, Tomas Vondra <tomas.vondra@enterprisedb.com>, Amit Kapila <amit.kapila16@gmail.com>, Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>
Date: 2024-07-18T02:22:16Z
Lists: pgsql-hackers

Attachments

On Thursday, July 11, 2024 1:03 PM shveta malik <shveta.malik@gmail.com> wrote:

Hi,

Thanks for the comments!

> 
> I have one concern about how we deal with conflicts. As for insert_exists, we
> keep on erroring out while raising conflict, until it is manually resolved:
> ERROR:  conflict insert_exists detected
> 
> But for other cases, we just log conflict and either skip or apply the operation. I
> LOG:  conflict update_differ detected
> DETAIL:  Updating a row that was modified by a different origin
> 
> I know that it is no different than HEAD. But now since we are logging conflicts
> explicitly, we should call out default behavior on each conflict. I see some
> incomplete and scattered info in '31.5.
> Conflicts' section saying that:
>  "When replicating UPDATE or DELETE operations, missing data will not
> produce a conflict and such operations will simply be skipped."
> (lets say it as pt a)
> 
> Also some more info in a later section saying (pt b):
> :A conflict will produce an error and will stop the replication; it must be resolved
> manually by the user."
> 
> My suggestions:
> 1) in point a above, shall we have:
> missing data or differing data (i.e. somehow reword to accommodate
> update_differ and delete_differ cases)

I am not sure if rewording existing words is better. I feel adding a link to
let user refer to the detect_conflict section for the all the
conflicts is sufficient, so did like that.

>
> 2)
> monitoring.sgml: Below are my suggestions, please change if you feel apt.
> 
> 2a) insert_exists_count : Number of times inserting a row that violates a NOT
> DEFERRABLE unique constraint while applying changes. Suggestion: Number of
> times a row insertion violated a NOT DEFERRABLE unique constraint while
> applying changes.
> 
> 2b) update_differ_count : Number of times updating a row that was previously
> modified by another source while applying changes. Suggestion: Number of times
> update was performed on a row that was previously modified by another source
> while applying changes.
> 
> 2c) delete_differ_count: Number of times deleting a row that was previously
> modified by another source while applying changes. Suggestion: Number of times
> delete was performed on a row that was previously modified by another source
> while applying changes.

I am a bit unsure which one is better, so I didn't change in this version.

> 
> 5)
> conflict.h:CONFLICT_NUM_TYPES
> 
> --Shall the macro be CONFLICT_TYPES_NUM  instead?

I think the current style followed existing ones(e.g. IOOP_NUM_TYPES,
BACKEND_NUM_TYPES, IOOBJECT_NUM_TYPES ...), so I didn't change this.

Attach the V5 patch set which changed the following:
1. addressed shveta's comments which are not mentioned above.
2. support update_exists conflict which indicates
that the updated value of a row violates the unique constraint.

Best Regards,
Hou zj

Commits

  1. Rename the conflict types for the origin differ cases.

  2. Doc: explain the log format of logical replication conflicts.

  3. Log the conflicts while applying changes in logical replication.

  4. Avoid duplicate table scans for cross-partition updates during logical replication.