for cross-bucket sync driven by a bucket replication policy, i agree that we shouldn't require full sync. skipping it by default would be compatible with aws, but that would change rgw's default behavior so we probably want a config option or zonegroup feature to allow admins to opt out of full sync. as a zonegroup feature, we could enable that aws behavior by default for new zones/zonegroups but existing deployments would have to enable manually

as discussed previously, i don't think we should allow skipping full syncs for normal bucket sync because that subverts disaster recovery and breaks symmetry within the zonegroup

i don't think we should allow skipping full syncs for normal bucket sync because that subverts disaster recovery and breaks symmetry within the zonegroup

That is the part I still struggle to distinguish a little bit :)
Can you please elaborate a little more? do you mean if the source was within the same zonegroup we should disregard the feature and go for full sync?

i'm referring to 'normal bucket sync' where the destination bucket is the same as the source bucket, just on another zone in the zonegroup. the kind you get by default in multisite without any bucket replication policy

i've been arguing that this replication should always be symmetric within the zonegroup, and for deprecation of features like sync policy and 'bucket sync disable' that break this symmetry. but this pr adds a new way to break the symmetry by skipping full sync

I moved this to the zonegroup feature. Also i've considered the normal bucket sync to disregard the feature and force the full-sync.

can you say more about the use case for bucket sync init --state incremental?

the purpose of bucket sync init is to restore consistency of a bucket that's either stuck behind or missing objects due to a bug or misconfiguration

if we skip the full sync, bucket sync init is just going to advance the incremental sync status markers to the end of the remote's bilogs. this means that it'll only replicate new changes that happen after the command runs. is that really the intent? that's very different from the existing command

if we really want to support this, i'd suggest naming it something completely different like bucket sync skip-existing. but again, this feature would subvert disaster recovery and break symmetry

right, I just wanted to re-use the existing code so I just added the state as an input here to extend. I guess we can name it something general like bucket sync reset-state --state {state} so we let admin switch from states as wished for now. what do you think?

but again, this feature would subvert disaster recovery and break symmetry

Is this something that again you think we should control by checking the source within the same zonegroup or are you referring to some other scenario? as we discussed the purpose of this PR is to subvert from DR and break symmetry but perhaps only when the source zone is in a different zonegroup?

right, I just wanted to re-use the existing code so I just added the state as an input here to extend. I guess we can name it something general like bucket sync reset-state --state {state} so we let admin switch from states as wished for now. what do you think?

i don't think --state stopped is a sensible thing to support. bucket sync disable is the correct way to disable logging and force all zones to transition to the stopped state. if we just set the sync status to stopped, i'm guessing that data sync will just transition back to full sync:

    if (syncstopped) {
      // transition to StateStopped in RGWSyncBucketShardCR. if sync is
      // still disabled, we'll delete the sync status object. otherwise we'll
      // restart full sync to catch any changes that happened while sync was
      // disabled
      sync_info.state = rgw_bucket_shard_sync_info::StateStopped;

that's right, thanks. I changed bucket sync init to bucket sync reset-state with only limited states init, full-sync, and incremental-sync. so this way we won't let stopped pass by that path. Also, made sure incremental-sync is not applied to a normal bucket sync scenario (within the zonegroup) to break the symmetry.

if we skip the full sync, bucket sync init is just going to advance the incremental sync status markers to the end of the remote's bilogs. this means that it'll only replicate new changes that happen after the command runs. is that really the intent?

I believe we need to. As the current implementation, full sync doesn't run in parallel to incremental, In case the state was set to full and that is taking so long or was a mistake and admin want to switch back to incremental, there should be a way to do so otherwise it might stuck forever.

otherwise it might stuck forever

as far as i know, the only way it would get stuck forever is if data sync loses track of the datalog entries that would trigger that bucket sync. a bucket sync reset-state wouldn't necessarily help with that either, because it doesn't write the corresponding datalog entries

instead, we have the bucket sync run command for this case. it manually processes the rest of bucket sync, including the transition from full to incremental. unlike the proposed bucket sync reset-state, this command preserves multisite's guarantee of eventual consistency of the bucket across zones

unlike the proposed bucket sync reset-state

right, but I didn't mean to replace bucket sync run with bucket sync reset-state (maybe the naming is misleading). my intent here is to open a door to the state machine of sync state for the admin to change it (to init, full-sync, and incremental) based on whatever factor he thinks is required and then eventually run the sync manually with bucket sync run or let the normal sync make it.
So the situations are:

• init: this is where we want to clear the state to restore consistency of a bucket.
• full-sync: here is where the feature is enabled but we want to simulate the Batch Replication. For this maybe we should only consider the current state is incremental before the transition.
• incremental: this is where the bucket is in full sync but this is taking so long or was a mistake (stuck was bad wording, I agree :) ) and the admin wants to revert it back to incremental (with respect to "normal bucket replication within zonegroup").

Can you share your thoughts on them? should we introduce each as a different command or you think we should not support some of them?

this behavior should apply to bucket replication policy in general, whether it's same- or cross-zonegroup

worth mentioning that this is the aws default for bucket replication policy after their deprecation of ExistingObjectReplication. that's why it's enabled by default on new deployments, but requires existing deployments to opt in

because aws uses the term ExistingObjectReplication, maybe we should call this feature something like skip-existing-object-replication-policy?

i'm referring to 'normal bucket sync' where the destination bucket is the same as the source bucket, just on another zone in the zonegroup. the kind you get by default in multisite without any bucket replication policy
i've been arguing that this replication should always be symmetric within the zonegroup, and for deprecation of features like sync policy and 'bucket sync disable' that break this symmetry.

this behavior should apply to bucket replication policy in general, whether it's same- or cross-zonegroup

ok so I guess that was my misunderstanding from the latter quote, you meant we should deprecate/block PutBucketReplication API calls or radosgw-admin bucket sync group commands followed by --bucket when it's about the same zonegroup (whether the bucket is the same or the zone) so we can always guarantee the symmetry between zones in a zonegroup. or any other way of deprecation?

can we find a way to make this specific to bucket replication policy instead of cross-zonegroup?

In that case, how are we going to tackle the existing policies pointing to the same zonegroup but with different zones or filters? are we going to release a breaking change in favor of the new cross-zonegroup feature or am I confused?

when i talk about symmetry between zones, i'm referring to the contents of a specific bucket. in the absense of sync policy or bucket replication policy, a given bucket should (eventually) have the same contents on each zone

bucket replication policy doesn't break this symmetry unless the source and destination bucket are the same. so i support your commit 888ddcb to reject such policies 👍

i've been slow to draft a design, but my general idea for cross-zonegroup replication is to add new primitives to enable/allow/forbid the sync relationships between zonegroups, similar to how existing sync policy treats zones

then we could deprecate the existing zone-level sync policy and other features that break per-bucket symmetry (like 'bucket sync disable' and our Zone extension to PutBucketReplication)

can we find a way to make this specific to bucket replication policy instead of cross-zonegroup?

RGWRunBucketSourcesSyncCR may spawn several RGWSyncBucketCRs. some of those may correspond to cross-bucket replication from policy, but one should be the same-bucket replication that we run by default for disaster recovery. i think we need to a way to control full sync differently for those two cases

so i don't think RGWDefaultSyncModuleInstance::should_full_sync() is the right place for this check, unless it can take should_full_sync(source_info, dest_info) and compare the buckets for equality. however, i think rgw supports bucket replication policy that names the same source bucket as the destination - do you know if aws allows that?

do you know if aws allows that?

No it doesn't, I had that check proposed here 888ddcb.

Basically, currently, the way the pipes are considered by RGWGetBucketPeersCR are two types:

• a sync policy (either on the bucket or zonegroup - bear in mind even without any sync policy on zonegroup level RGWBucketSyncPolicyHandler() will create a default sync policy

  ceph/src/rgw/driver/rados/rgw_bucket_sync.cc

  Lines 758 to 761 in 90a5d4e

  	if (sync_policy.empty()) {
  	RGWSyncPolicyCompat::convert_old_sync_config(zone_svc, sync_modules_svc, &sync_policy);
  	legacy_config = true;
  	}

  ) containing no dest bucket results in RGWBucketSyncFlowManager::reflect() to reflect a policy pointing to the same bucket in the current RGW's zone running sync.
• a hint created by RGWSI_Bucket_Sync_SObj::handle_bi_update() when a sync policy is created for a bucket pointing to that bucket.

So in all cases, we always deal with sync policies and distinguish between "bucket replication" or "within zonegroup - between zone replication) is kinda tricky now but it depends on what approach we want to take in the discussion above.

No it doesn't, I had that check proposed here 888ddcb.

okay, i think that helps. if we enforce that source != dest for new policy, then should_full_sync(source_info, dest_info) should be sufficient. we'll continue doing full sync for existing policy where they're equal, and the full sync of new policy will depend on the zonegroup feature. does that make sense?

given that we deprecated the only sync module that overrode should_full_sync(), i'm skeptical that we want to leave this behavior up to the sync module interface. we might just hard-code that bucket comparison in/under RGWRunBucketSourcesSyncCR

we'll continue doing full sync for existing policy where they're equal, and the full sync of new policy will depend on the zonegroup feature. does that make sense?

Do we see a sync policy with src_bucket == dest_bucket but carrying filters (prefixes and/or tags) symmetry or not?

Do we see a sync policy with src_bucket == dest_bucket but carrying filters (prefixes and/or tags) symmetry or not?

if it results in different contents on each zone, then it's not symmetric. but the reason to continue doing full sync in this case is for backward compatiblity, not symmetry

OK, let me break it down here:

1. We reject policy creation (through API and radosgw-admin) for a bucket that results in replicating to a bucket with the same name (here we don't care about zonegroup as cross-zonegroup logically we can't so both cases should be rejected).
2. we would change how the sync policy on the zonegroup level would work. basically, it should only control the cross-zonegroup replication not zones within the zonegroup - otherwise, that could break symmetry within the zonegroup (like having policies "enabling" replication of any source to a single bucket with a different name or stuff like that).
3. at this point we can say the sync policy that RGWGetBucketPeersCR() gives us is either pointing to the same bucket name with different zones in its zonegroup or a different bucket name (here doesn't matter if zonegroup is the same - although we don't support SRR yet so perhaps we need to return HTTP 501 on policy creation for that).
4. So we do if src_bucket == dest_bucket then it's symmetry and ignores the zonegroup feature and relies on should_full_sync() else consider zonegroup feature with should_full_sync().

Did I miss anything?

nice summary, i think we're on the same page!