replmd: Don't fail cycle if we get link for deleted object with GET_TGT
authorTim Beale <timbeale@catalyst.net.nz>
Wed, 19 Jul 2017 23:14:27 +0000 (11:14 +1200)
committerAndrew Bartlett <abartlet@samba.org>
Fri, 18 Aug 2017 04:07:12 +0000 (06:07 +0200)
We are going to end up supporting 2 different server schemes:
A. the old/default behaviour of sending all the linked attributes last,
   at the end of the replication cycle.
B. the new/Microsoft way of sending the linked attributes interleaved
   with the source/target objects.

Normally if we're talking to a server using the old scheme-A, we won't
ever use the GET_TGT flag. However, there are a couple of cases where
it can happen:
- A link to a new object was added during the replication cycle.
- An object was deleted while the replication was in progress (and
the linked attribute got queued before the object was deleted).

Talking to an Samba DC running the old scheme will just cause it to
start the replication cycle from scratch again, which is fairly
harmless. However, there is a chance that the same thing can happen
again, in which case the replication cycle will fail (because GET_TGT
was already set).

Even if we're using the new scheme (B), we could still potentially hit
this case, as we can still queue up linked attributes between requests
(group memberships can be larger than what can fit into a single
replication chunk).

If GET_TGT is set in the GetNcChanges request, then the local copy of
the target object should always be up-to-date when we process the linked
attribute. So if we still think the target object is deleted/recycled at
this point, then it's safe to ignore the linked attribute (because we
know our local copy is up-to-date). This logic matches the MS spec logic
in ProcessLinkValue().

Not failing the replication cycle may be beneficial if we're trying to
do a full-sync of a large database. Otherwise it might be time-consuming
and frustrating to repeat the sync unnecessarily.

Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Reviewed-by: Garming Sam <garming@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
BUG: https://bugzilla.samba.org/show_bug.cgi?id=12972

source4/dsdb/repl/drepl_out_helpers.c
source4/dsdb/samdb/ldb_modules/repl_meta_data.c
source4/dsdb/samdb/samdb.h
source4/libnet/libnet_vampire.c

index 6e24e7f8de32846f89bad3c7f2a4ce0a85babff5..66308040377a2dbd0998296117cc9ac65ca32dc9 100644 (file)
@@ -868,6 +868,10 @@ static void dreplsrv_op_pull_source_apply_changes_trigger(struct tevent_req *req
                dsdb_repl_flags |= DSDB_REPL_FLAG_OBJECT_SUBSET;
        }
 
                dsdb_repl_flags |= DSDB_REPL_FLAG_OBJECT_SUBSET;
        }
 
+       if (state->op->more_flags & DRSUAPI_DRS_GET_TGT) {
+               dsdb_repl_flags |= DSDB_REPL_FLAG_TARGETS_UPTODATE;
+       }
+
        if (state->op->extended_op != DRSUAPI_EXOP_NONE) {
                ret = dsdb_find_nc_root(service->samdb, partition,
                                        partition->dn, &nc_root);
        if (state->op->extended_op != DRSUAPI_EXOP_NONE) {
                ret = dsdb_find_nc_root(service->samdb, partition,
                                        partition->dn, &nc_root);
index 8e2c64b10fd2c866ff12a5222e37ed189ca96c94..228622cfcc0c445e77128c6509606c3410fbe9c6 100644 (file)
@@ -74,7 +74,7 @@ struct replmd_private {
 struct la_entry {
        struct la_entry *next, *prev;
        struct drsuapi_DsReplicaLinkedAttribute *la;
 struct la_entry {
        struct la_entry *next, *prev;
        struct drsuapi_DsReplicaLinkedAttribute *la;
-       bool incomplete_replica;
+       uint32_t dsdb_repl_flags;
 };
 
 struct replmd_replicated_request {
 };
 
 struct replmd_replicated_request {
@@ -6052,14 +6052,12 @@ static int replmd_store_linked_attributes(struct replmd_replicated_request *ar)
 {
        int ret = LDB_SUCCESS;
        uint32_t i;
 {
        int ret = LDB_SUCCESS;
        uint32_t i;
-       bool incomplete_subset;
        struct ldb_module *module = ar->module;
        struct replmd_private *replmd_private =
                talloc_get_type(ldb_module_get_private(module), struct replmd_private);
        struct ldb_context *ldb;
 
        ldb = ldb_module_get_ctx(module);
        struct ldb_module *module = ar->module;
        struct replmd_private *replmd_private =
                talloc_get_type(ldb_module_get_private(module), struct replmd_private);
        struct ldb_context *ldb;
 
        ldb = ldb_module_get_ctx(module);
-       incomplete_subset = (ar->objs->dsdb_repl_flags & DSDB_REPL_FLAG_OBJECT_SUBSET);
 
        DEBUG(4,("linked_attributes_count=%u\n", ar->objs->linked_attributes_count));
 
 
        DEBUG(4,("linked_attributes_count=%u\n", ar->objs->linked_attributes_count));
 
@@ -6082,13 +6080,7 @@ static int replmd_store_linked_attributes(struct replmd_replicated_request *ar)
                        return LDB_ERR_OPERATIONS_ERROR;
                }
                *la_entry->la = ar->objs->linked_attributes[i];
                        return LDB_ERR_OPERATIONS_ERROR;
                }
                *la_entry->la = ar->objs->linked_attributes[i];
-
-               /*
-                * we may not be able to resolve link targets properly when
-                * dealing with subsets of objects, e.g. the source is a
-                * critical object and the target isn't
-                */
-               la_entry->incomplete_replica = incomplete_subset;
+               la_entry->dsdb_repl_flags = ar->objs->dsdb_repl_flags;
 
                /* we need to steal the non-scalars so they stay
                   around until the end of the transaction */
 
                /* we need to steal the non-scalars so they stay
                   around until the end of the transaction */
@@ -6793,17 +6785,19 @@ static int replmd_check_target_exists(struct ldb_module *module,
        if (target_res->count == 0) {
 
                /*
        if (target_res->count == 0) {
 
                /*
+                * we may not be able to resolve link targets properly when
+                * dealing with subsets of objects, e.g. the source is a
+                * critical object and the target isn't
+                *
                 * TODO:
                 * When we implement Trusted Domains we need to consider
                 * whether they get treated as an incomplete replica here or not
                 */
                 * TODO:
                 * When we implement Trusted Domains we need to consider
                 * whether they get treated as an incomplete replica here or not
                 */
-               if (la_entry->incomplete_replica) {
+               if (la_entry->dsdb_repl_flags & DSDB_REPL_FLAG_OBJECT_SUBSET) {
 
                        /*
 
                        /*
-                        * If we're only replicating a subset of objects (e.g.
-                        * critical-only, single-object), then an unknown target
-                        * is probably not a critical problem. We don't increase
-                        * the highwater-mark so subsequent replications should
+                        * We don't increase the highwater-mark in the object
+                        * subset cases, so subsequent replications should
                         * resolve any missing links
                         */
                        DEBUG(2,(__location__
                         * resolve any missing links
                         */
                        DEBUG(2,(__location__
@@ -6853,12 +6847,27 @@ static int replmd_check_target_exists(struct ldb_module *module,
                 * copy of the target object isn't up to date.
                 */
                if (target_deletion_state >= OBJECT_RECYCLED) {
                 * copy of the target object isn't up to date.
                 */
                if (target_deletion_state >= OBJECT_RECYCLED) {
-                       ldb_asprintf_errstring(ldb,
-                                              "Deleted target %s GUID %s linked from %s\n",
-                                              ldb_dn_get_linearized(dsdb_dn->dn),
-                                              GUID_string(tmp_ctx, guid),
-                                              ldb_dn_get_linearized(source_dn));
-                       ret = LDB_ERR_NO_SUCH_OBJECT;
+
+                       if (la_entry->dsdb_repl_flags & DSDB_REPL_FLAG_TARGETS_UPTODATE) {
+
+                               /*
+                                * target should already be uptodate so there's no
+                                * point retrying - it's probably just bad timing
+                                */
+                               *ignore_link = true;
+                               DEBUG(0, ("%s is deleted but up to date. "
+                                         "Ignoring link from %s\n",
+                                         ldb_dn_get_linearized(dsdb_dn->dn),
+                                         ldb_dn_get_linearized(source_dn)));
+
+                       } else {
+                               ldb_asprintf_errstring(ldb,
+                                                      "Deleted target %s GUID %s linked from %s",
+                                                      ldb_dn_get_linearized(dsdb_dn->dn),
+                                                      GUID_string(tmp_ctx, guid),
+                                                      ldb_dn_get_linearized(source_dn));
+                               ret = LDB_ERR_NO_SUCH_OBJECT;
+                       }
                }
        }
 
                }
        }
 
index 2abaf4a8f1bab1e5fe48bdbd4fc2a0085de50a1f..01eb1f3c0879be80c634d02ec6ed8ff6e00db22e 100644 (file)
@@ -65,6 +65,7 @@ struct dsdb_control_current_partition {
 #define DSDB_REPL_FLAG_ADD_NCNAME         4
 #define DSDB_REPL_FLAG_EXPECT_NO_SECRETS   8
 #define DSDB_REPL_FLAG_OBJECT_SUBSET       16
 #define DSDB_REPL_FLAG_ADD_NCNAME         4
 #define DSDB_REPL_FLAG_EXPECT_NO_SECRETS   8
 #define DSDB_REPL_FLAG_OBJECT_SUBSET       16
+#define DSDB_REPL_FLAG_TARGETS_UPTODATE    32
 
 #define DSDB_CONTROL_REPLICATED_UPDATE_OID "1.3.6.1.4.1.7165.4.3.3"
 
 
 #define DSDB_CONTROL_REPLICATED_UPDATE_OID "1.3.6.1.4.1.7165.4.3.3"
 
index d89256b02a80d3ac7fe0d0c769f16b8312a95c13..aa0ec91e5908c401ccf83f5a328aed6efd0cc25a 100644 (file)
@@ -647,6 +647,10 @@ WERROR libnet_vampire_cb_store_chunk(void *private_data,
                        is_exop = true;
                }
                req_replica_flags = c->req10->replica_flags;
                        is_exop = true;
                }
                req_replica_flags = c->req10->replica_flags;
+
+               if (c->req10->more_flags & DRSUAPI_DRS_GET_TGT) {
+                       dsdb_repl_flags |= DSDB_REPL_FLAG_TARGETS_UPTODATE;
+               }
                break;
        default:
                return WERR_INVALID_PARAMETER;
                break;
        default:
                return WERR_INVALID_PARAMETER;