On Thu, Jul 18, 2024 at 7:52 AM Zhijie Hou (Fujitsu) <houzj.f...@fujitsu.com> wrote: > > On Thursday, July 11, 2024 1:03 PM shveta malik <shveta.ma...@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.
Agree, it looks better with detect_conflict link. > > > > 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. I still feel the wording is bit unclear/incomplete Also to be consistent with previous fields (see sync_error_count:Number of times an error occurred during the initial table synchronization), we should at-least have it in past tense. Another way of writing could be: 'Number of times inserting a row violated a NOT DEFERRABLE unique constraint while applying changes.' and likewise for each conflict field. > > > > 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. Thanks for making the changes. thanks Shveta
