From 89cf5c3f76e214fbd06ce461470eb64b338c5144 Mon Sep 17 00:00:00 2001 From: Tim Beale Date: Thu, 20 Jul 2017 11:14:27 +1200 Subject: [PATCH] replmd: Don't fail cycle if we get link for deleted object with GET_TGT 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 Reviewed-by: Garming Sam Reviewed-by: Andrew Bartlett BUG: https://bugzilla.samba.org/show_bug.cgi?id=12972 --- source4/dsdb/repl/drepl_out_helpers.c | 4 ++ .../dsdb/samdb/ldb_modules/repl_meta_data.c | 51 +++++++++++-------- source4/dsdb/samdb/samdb.h | 1 + source4/libnet/libnet_vampire.c | 4 ++ 4 files changed, 39 insertions(+), 21 deletions(-) diff --git a/source4/dsdb/repl/drepl_out_helpers.c b/source4/dsdb/repl/drepl_out_helpers.c index 6e24e7f8de3..66308040377 100644 --- a/source4/dsdb/repl/drepl_out_helpers.c +++ b/source4/dsdb/repl/drepl_out_helpers.c @@ -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; } + 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); diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c index 8e2c64b10fd..228622cfcc0 100644 --- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c +++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c @@ -74,7 +74,7 @@ struct replmd_private { struct la_entry { struct la_entry *next, *prev; struct drsuapi_DsReplicaLinkedAttribute *la; - bool incomplete_replica; + uint32_t dsdb_repl_flags; }; 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; - 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); - incomplete_subset = (ar->objs->dsdb_repl_flags & DSDB_REPL_FLAG_OBJECT_SUBSET); 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]; - - /* - * 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 */ @@ -6793,17 +6785,19 @@ static int replmd_check_target_exists(struct ldb_module *module, 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 */ - 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__ @@ -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) { - 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; + } } } diff --git a/source4/dsdb/samdb/samdb.h b/source4/dsdb/samdb/samdb.h index 2abaf4a8f1b..01eb1f3c087 100644 --- a/source4/dsdb/samdb/samdb.h +++ b/source4/dsdb/samdb/samdb.h @@ -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_TARGETS_UPTODATE 32 #define DSDB_CONTROL_REPLICATED_UPDATE_OID "1.3.6.1.4.1.7165.4.3.3" diff --git a/source4/libnet/libnet_vampire.c b/source4/libnet/libnet_vampire.c index d89256b02a8..aa0ec91e590 100644 --- a/source4/libnet/libnet_vampire.c +++ b/source4/libnet/libnet_vampire.c @@ -647,6 +647,10 @@ WERROR libnet_vampire_cb_store_chunk(void *private_data, 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; -- 2.34.1