bdr: decouple conflict logging and remote apply

Previously we always logged even applyed rows (which are not technically
conflicting) with optional check for apply_update. Now we log based on
what is actually happening and if the row was conflicting or not.

This also removes the bdr.log_applyed_conflicts config option as it's
not needed anymore.

diff --git a/bdr.c b/bdr.c
index 3cf4c12984deeb5a3036f7cf127761927b13c580..8facb86c357f14aab8cd39277c8a92270256e678 100644 (file)
--- a/bdr.c
+++ b/bdr.c
@@ -89,7 +89,6 @@ static char *connections = NULL;
 static bool bdr_synchronous_commit;
 int bdr_default_apply_delay;
 int bdr_max_workers;
-bool bdr_log_applied_conflicts;
 static bool bdr_skip_ddl_replication;
 
 /*
@@ -1580,18 +1579,6 @@ _PG_init(void)
                             0,
                             NULL, NULL, NULL);
 
-   DefineCustomBoolVariable("bdr.log_applied_conflicts",
-                            "BDR usually only logs conflicts resolved by user handlers or where "
-                            "the remote row is discarded. If enabled, log records will also be "
-                            "written for rows where the local row is overwritten by a newer remote "
-                            "row.",
-                            NULL,
-                            &bdr_log_applied_conflicts,
-                            false,
-                            PGC_SIGHUP,
-                            0,
-                            NULL, NULL, NULL);
-
    /* if nothing is configured, we're done */
    if (connections == NULL)
    {

diff --git a/bdr.h b/bdr.h
index 19d3d99c4cabb11bada29e664cf3c93ae3c10bf6..8cc95f404591f79a10fa00b6345130e9da3ebeb2 100644 (file)
--- a/bdr.h
+++ b/bdr.h
@@ -222,7 +222,6 @@ extern int  bdr_default_apply_delay;
 extern int bdr_max_workers;
 extern char *bdr_temp_dump_directory;
 extern bool bdr_init_from_basedump;
-extern bool bdr_log_applied_conflicts;
 extern bool bdr_log_conflicts_to_table;
 extern bool bdr_conflict_logging_include_tuples;
 extern bool bdr_permit_unsafe_commands;

diff --git a/bdr_apply.c b/bdr_apply.c
index ae49438cbf2455ee83a819cfd8c093895db27ff6..c06830e0e46e1292bcac1395a66b936a5c30d810 100644 (file)
--- a/bdr_apply.c
+++ b/bdr_apply.c
@@ -988,6 +988,8 @@ check_apply_update(RepNodeId local_node_id, TimestampTz local_ts,
    if (new_tuple)
        *new_tuple = NULL;
 
+   *log_update = false;
+
    if (local_node_id == replication_origin_id)
    {
        /*
@@ -997,7 +999,6 @@ check_apply_update(RepNodeId local_node_id, TimestampTz local_ts,
         * locking guarantees are met.
         */
        *perform_update = true;
-       *log_update = false;
        return;
    }
    else
@@ -1009,7 +1010,6 @@ check_apply_update(RepNodeId local_node_id, TimestampTz local_ts,
         * If the caller doesn't provide storage for the conflict handler to
         * store a new tuple in, don't fire any conflict triggers.
         */
-       *log_update = true;
 
        if (new_tuple)
        {
@@ -1021,6 +1021,7 @@ check_apply_update(RepNodeId local_node_id, TimestampTz local_ts,
             * - Take no action and fall through to the next handling option
             * --------------
             */
+
            TimestampDifference(replication_origin_timestamp, local_ts,
                                &secs, &microsecs);
 
@@ -1032,12 +1033,15 @@ check_apply_update(RepNodeId local_node_id, TimestampTz local_ts,
 
            if (skip)
            {
+               *log_update = true;
                *perform_update = false;
                *resolution = BdrConflictResolution_ConflictTriggerSkipChange;
                return;
            }
            else if (*new_tuple)
            {
+               /* Custom conflict handler returned tuple, log it. */
+               *log_update = true;
                *perform_update = true;
                *resolution = BdrConflictResolution_ConflictTriggerReturnedTuple;
                return;
@@ -1061,6 +1065,7 @@ check_apply_update(RepNodeId local_node_id, TimestampTz local_ts,
        else if (cmp < 0)
        {
            /* The most recent update is the local one; retain it */
+           *log_update = true;
            *perform_update = false;
            *resolution = BdrConflictResolution_LastUpdateWins_KeepLocal;
            return;
@@ -1114,9 +1119,14 @@ check_apply_update(RepNodeId local_node_id, TimestampTz local_ts,
             * later.
             */
            if (*perform_update)
+           {
                *resolution = BdrConflictResolution_LastUpdateWins_KeepRemote;
+           }
            else
+           {
                *resolution = BdrConflictResolution_LastUpdateWins_KeepLocal;
+               *log_update = true;
+           }
 
            return;
        }
@@ -1280,9 +1290,6 @@ do_log_conflict(BdrConflictType conflict_type,
     * local tuple is replaced by a newer remote tuple. Most of the time these
     * are just noise, but there are times it's useful for debugging and tracing.
     */
-   if (apply_remote && !bdr_log_applied_conflicts)
-       return;
-
 
    bdr_fetch_sysid_via_node_id(local_node_id,
                                &local_sysid, &local_tli,

diff --git a/bdr_isolationregress.conf b/bdr_isolationregress.conf
index 659ca3371383f97db713876a849f0fdd65198d4a..e5a6a29cc75c36e5cbc61ea9a79823a0cbf011c1 100644 (file)
--- a/bdr_isolationregress.conf
+++ b/bdr_isolationregress.conf
@@ -26,7 +26,6 @@ bdr.node3to2_local_dbname = 'node3'
 bdr.permit_unsafe_ddl_commands = false
 
 bdr.log_conflicts_to_table = True
-bdr.log_applied_conflicts = True
 bdr.default_apply_delay = 100
 
 track_commit_timestamp = on

diff --git a/expected/isolation/dmlconflict_ii.out b/expected/isolation/dmlconflict_ii.out
index 8b368535afc0d7c9caf423176274c3e94c7d5af0..cbd578af342d242a851bcd6af05e20de53d14c47 100644 (file)
--- a/expected/isolation/dmlconflict_ii.out
+++ b/expected/isolation/dmlconflict_ii.out
@@ -54,13 +54,10 @@ z              1              baz
 step s1h: SELECT object_schema, object_name, conflict_type, conflict_resolution, local_tuple, remote_tuple, error_sqlstate FROM bdr.bdr_conflict_history ORDER BY conflict_id;
 object_schema  object_name    conflict_type  conflict_resolutionlocal_tuple    remote_tuple   error_sqlstate 
 
-public         test_dmlconflictinsert_insert  last_update_wins_keep_remote{"a":"x","b":1,"c":"foo"}{"a":"y","b":1,"c":"bar"}               
-public         test_dmlconflictinsert_insert  last_update_wins_keep_remote{"a":"y","b":1,"c":"bar"}{"a":"z","b":1,"c":"baz"}               
 step s2h: SELECT object_schema, object_name, conflict_type, conflict_resolution, local_tuple, remote_tuple, error_sqlstate FROM bdr.bdr_conflict_history ORDER BY conflict_id;
 object_schema  object_name    conflict_type  conflict_resolutionlocal_tuple    remote_tuple   error_sqlstate 
 
 public         test_dmlconflictinsert_insert  last_update_wins_keep_local{"a":"y","b":1,"c":"bar"}{"a":"x","b":1,"c":"foo"}               
-public         test_dmlconflictinsert_insert  last_update_wins_keep_remote{"a":"y","b":1,"c":"bar"}{"a":"z","b":1,"c":"baz"}               
 step s3h: SELECT object_schema, object_name, conflict_type, conflict_resolution, local_tuple, remote_tuple, error_sqlstate FROM bdr.bdr_conflict_history ORDER BY conflict_id;
 object_schema  object_name    conflict_type  conflict_resolutionlocal_tuple    remote_tuple   error_sqlstate 
 

diff --git a/expected/isolation/dmlconflict_uu.out b/expected/isolation/dmlconflict_uu.out
index 6f72f5825b9d396b3f3d63ee3be6ddc300e3114b..cb140845b103b32712e09e32a1cfb83b7021749e 100644 (file)
--- a/expected/isolation/dmlconflict_uu.out
+++ b/expected/isolation/dmlconflict_uu.out
@@ -54,13 +54,10 @@ y              1              baz
 step s1h: SELECT object_schema, object_name, conflict_type, conflict_resolution, local_tuple, remote_tuple, error_sqlstate FROM bdr.bdr_conflict_history ORDER BY conflict_id;
 object_schema  object_name    conflict_type  conflict_resolutionlocal_tuple    remote_tuple   error_sqlstate 
 
-public         test_dmlconflictupdate_update  last_update_wins_keep_remote{"a":"x","b":1,"c":"foo"}{"a":"y","b":1,"c":"bar"}               
-public         test_dmlconflictupdate_update  last_update_wins_keep_remote{"a":"y","b":1,"c":"bar"}{"a":"y","b":1,"c":"baz"}               
 step s2h: SELECT object_schema, object_name, conflict_type, conflict_resolution, local_tuple, remote_tuple, error_sqlstate FROM bdr.bdr_conflict_history ORDER BY conflict_id;
 object_schema  object_name    conflict_type  conflict_resolutionlocal_tuple    remote_tuple   error_sqlstate 
 
 public         test_dmlconflictupdate_update  last_update_wins_keep_local{"a":"y","b":1,"c":"bar"}{"a":"x","b":1,"c":"foo"}               
-public         test_dmlconflictupdate_update  last_update_wins_keep_remote{"a":"y","b":1,"c":"bar"}{"a":"y","b":1,"c":"baz"}               
 step s3h: SELECT object_schema, object_name, conflict_type, conflict_resolution, local_tuple, remote_tuple, error_sqlstate FROM bdr.bdr_conflict_history ORDER BY conflict_id;
 object_schema  object_name    conflict_type  conflict_resolutionlocal_tuple    remote_tuple   error_sqlstate