KAFKA-9726 IdentityReplicationPolicy by ryannedolan · Pull Request #10652 · apache/kafka

ryannedolan · 2021-05-07T19:17:07Z

Enable active/passive, one-way replication without renaming topics, similar to MM1. This implementation is described in KIP-382 (adopted), originally as "LegacyReplicationPolicy".

This enables operators to migrate from MM1 to MM2 without re-architecting their replication flows, and enables some additional use-cases for MM2. For example, operators may wish to "upgrade" their Kafka clusters by mirroring everything to a completely new cluster. Such a migration would have been difficult with either MM1 or MM2 previously.

When using IdentityReplicationPolicy, operators should be aware that MM2 will not be able to detect cycles among replicated topics. A misconfigured topology may result in replicating the same records back-and-forth or in an infinite loop. However, we don't prevent this behavior, as some use-cases involve filtering records (via SMTs) to prevent cycles.

This PR includes major contributions from @mdedetrich and @ivanyu, so please include them in the commit log!

…PI changes

mdedetrich · 2021-05-27T10:07:19Z

@ryannedolan Since #10762 was merged maybe it makes sense to rebase against the current trunk? Some of the tests in this PR have assert statements without the failure messages which have been just been fixed (I believe you can just copy some of those assert failure messages to make sure its consistent).

…PI changes

…9726

mdedetrich · 2021-06-03T07:53:18Z

@mimaison As my PR at #10648 has been superseded by this one, would it be possible to do a quick review of it? Afaik its ready and @ryannedolan has done great work in avoiding the need for a KIP so there shouldn't be any blockers.

ryannedolan · 2021-06-03T22:04:04Z

None of the failing tests are related. Ready to merge.

mimaison · 2021-06-07T08:31:33Z

@ryannedolan @mdedetrich Thanks, I'll try to take a look this week

mimaison

Thanks @ryannedolan for the PR. It looks good overall, I left a few small comments

ryannedolan · 2021-06-17T02:08:48Z

@mimaison thanks for the feedback and suggestions. I've updated accordingly.

…9726

mdedetrich · 2021-06-24T08:50:12Z

@ryannedolan I actually just realized, does it make sense to add the IdentityReplicationPolicy to the release notes so that people are aware of it (talking about docs/streams/upgrade-guide.html)

mimaison · 2021-06-24T09:45:16Z

Yes, we can add a line in https://github.com/apache/kafka/blob/trunk/docs/upgrade.html#L85 to introduce this new ReplicationPolicy

mdedetrich · 2021-06-28T13:38:55Z

Is there anything left on this PR to be merged (apart from the changelog which is a nice to have)?

ryannedolan · 2021-06-28T16:21:49Z

I think changelog and documentation updates can come after this is merged.

mimaison · 2021-06-28T17:53:26Z

Let's put a line in the changelog now so we're sure it's included in the release notes. I'm happy to merge once this is done.

I agree the documentation can come later.

ryannedolan · 2021-06-29T22:34:15Z

        in <code>new ConsumerGroupMetadata(consumerGroupId)</code> to work with older brokers. See <a href="https://cwiki.apache.org/confluence/x/zJONCg">KIP-732</a> for more details.
    </li>
-
+    <li> The Connect-based MirrorMaker (MM2) includes changes to support <code>IdentityReplicationPolicy</code>, enabling replication without renaming topics.


@mimaison @mdedetrich wdyt?

I would make a quick amendment stating that it works like the original MM1 to make it more clear.

Yes I think it's enough, thanks

mimaison · 2021-06-30T17:42:15Z

@ryannedolan I was about to merge but I noticed IdentityReplicationIntegrationTest.testOffsetSyncsTopicsOnTarget() is failing. It probably needs some adjustements to work with this policy as the computed offset-syncs topic name looks odd:

org.opentest4j.AssertionFailedError: Condition not met within timeout 30000. Unable to find checkpoints for primarytest-topic-1 ==> expected: <true> but was: <false>

…ity replication

ryannedolan · 2021-06-30T20:30:25Z

testOffsetSyncsTopicsOnTarget

Yep good catch. Fixed!

mimaison · 2021-07-01T08:24:20Z

Thanks @ryannedolan, @mdedetrich and @ivanyu

This new policy enables active/passive, one-way replication without renaming topics, similar to MM1. This implementation is described in KIP-382 (adopted), originally as "LegacyReplicationPolicy". This enables operators to migrate from MM1 to MM2 without re-architecting their replication flows, and enables some additional use-cases for MM2. For example, operators may wish to "upgrade" their Kafka clusters by mirroring everything to a completely new cluster. Such a migration would have been difficult with either MM1 or MM2 previously. When using IdentityReplicationPolicy, operators should be aware that MM2 will not be able to detect cycles among replicated topics. A misconfigured topology may result in replicating the same records back-and-forth or in an infinite loop. However, we don't prevent this behavior, as some use-cases involve filtering records (via SMTs) to prevent cycles. Reviewers: Mickael Maison <mickael.maison@gmail.com> Co-authored-by: Ryanne Dolan <rdolan@twitter.com> Co-authored-by: Matthew de Detrich <mdedetrich@gmail.com> Co-authored-by: Ivan Yurchenko <ivanyu@aiven.io>

KAFKA-9726: Add IdentityReplicationPolicy to MirrorMaker2 (apache#10652)

This new policy enables active/passive, one-way replication without renaming topics, similar to MM1. This implementation is described in KIP-382 (adopted), originally as "LegacyReplicationPolicy". This enables operators to migrate from MM1 to MM2 without re-architecting their replication flows, and enables some additional use-cases for MM2. For example, operators may wish to "upgrade" their Kafka clusters by mirroring everything to a completely new cluster. Such a migration would have been difficult with either MM1 or MM2 previously. When using IdentityReplicationPolicy, operators should be aware that MM2 will not be able to detect cycles among replicated topics. A misconfigured topology may result in replicating the same records back-and-forth or in an infinite loop. However, we don't prevent this behavior, as some use-cases involve filtering records (via SMTs) to prevent cycles. Reviewers: Mickael Maison <mickael.maison@gmail.com> Co-authored-by: Ryanne Dolan <rdolan@twitter.com> Co-authored-by: Matthew de Detrich <mdedetrich@gmail.com> Co-authored-by: Ivan Yurchenko <ivanyu@aiven.io>

Justinwins · 2021-08-05T10:29:15Z

        props.putAll(stringsWithPrefix("header.converter"));
        props.putAll(stringsWithPrefix("task"));
        props.putAll(stringsWithPrefix("worker"));
+        props.putAll(stringsWithPrefix("replication.policy"));


In MirrorClientConfig ,it seems that it's not necessary to add replication.policy into the props.
As i see , org.apache.kafka.connect.mirror.MirrorClientConfig#replicationPolicy initialise the ReplicationPolicy instance,
and in MirrorClient this instance is used ,where ReplicationPolicy takes effect for real.

Justinwins · 2021-08-06T08:18:55Z

-
+    <li> The Connect-based MirrorMaker (MM2) includes changes to support <code>IdentityReplicationPolicy</code>, enabling replication without renaming topics.
+        The existing <code>DefaultReplicationPolicy</code> is still used by default, but identity replication can be enabled via the
+        <code>replication.policy</code> configuration property. This is especially useful for users migrating from the older MirrorMaker (MM1), or for


i think it's more clear to say "replication.policy.class " here ,you know , which means it's configured in that form in mm2.properties file.

Friendly to beginners .

This new policy enables active/passive, one-way replication without renaming topics, similar to MM1. This implementation is described in KIP-382 (adopted), originally as "LegacyReplicationPolicy". This enables operators to migrate from MM1 to MM2 without re-architecting their replication flows, and enables some additional use-cases for MM2. For example, operators may wish to "upgrade" their Kafka clusters by mirroring everything to a completely new cluster. Such a migration would have been difficult with either MM1 or MM2 previously. When using IdentityReplicationPolicy, operators should be aware that MM2 will not be able to detect cycles among replicated topics. A misconfigured topology may result in replicating the same records back-and-forth or in an infinite loop. However, we don't prevent this behavior, as some use-cases involve filtering records (via SMTs) to prevent cycles. Reviewers: Mickael Maison <mickael.maison@gmail.com> Co-authored-by: Ryanne Dolan <rdolan@twitter.com> Co-authored-by: Matthew de Detrich <mdedetrich@gmail.com> Co-authored-by: Ivan Yurchenko <ivanyu@aiven.io>

ryannedolan added 2 commits May 7, 2021 19:08

WIP LegacyReplicationPolicy and tests

447b38f

Merge branch 'trunk' of github.com:ryannedolan/kafka into trunk

8a1eec1

ryannedolan mentioned this pull request May 7, 2021

KAFKA-9726: Add IdentityReplicationPolicy for MM2 #10648

Closed

3 tasks

ryannedolan commented May 7, 2021

View reviewed changes

Comment thread ...st/java/org/apache/kafka/connect/mirror/integration/MirrorConnectorsIntegrationBaseTest.java

WIP fix integration tests -- two FIXMEs

697d212

ryannedolan force-pushed the KAFKA-9726 branch from 0a3d0df to 697d212 Compare May 18, 2021 22:02

steal mdedetrich's tests and IdentityReplicationPolicy, but without A…

9af883e

…PI changes

ryannedolan force-pushed the KAFKA-9726 branch from cd498ad to 9af883e Compare May 20, 2021 22:11

ryannedolan commented May 21, 2021

View reviewed changes

Comment thread connect/mirror-client/src/main/java/org/apache/kafka/connect/mirror/MirrorClient.java Outdated

dropped LegacyReplicationPolicy

53a2e2c

ryannedolan force-pushed the KAFKA-9726 branch from 6c9a5e5 to 53a2e2c Compare May 21, 2021 20:58

ryannedolan changed the title ~~WIP KAFKA-9726 LegacyReplicationPolicy (Ryanne's version)~~ KAFKA-9726 IdentityReplicationPolicy May 21, 2021

ryannedolan marked this pull request as ready for review May 21, 2021 21:05

ryannedolan and others added 5 commits May 28, 2021 21:25

WIP LegacyReplicationPolicy and tests

aaa10b2

WIP fix integration tests -- two FIXMEs

29a95e6

steal mdedetrich's tests and IdentityReplicationPolicy, but without A…

a3d7cc8

…PI changes

dropped LegacyReplicationPolicy

7f621cf

Merge branch 'KAFKA-9726' of github.com:ryannedolan/kafka into KAFKA-…

2a03f8c

…9726

Merge branch 'apache:trunk' into KAFKA-9726

5050370

mimaison requested changes Jun 10, 2021

View reviewed changes

mimaison mentioned this pull request Jun 10, 2021

KAFKA-12436 KIP-720 Deprecate MirrorMaker v1 #10805

Merged

ryannedolan and others added 3 commits June 16, 2021 12:03

Merge branch 'apache:trunk' into KAFKA-9726

10ebae2

drop unnecessary change to mirror-client api

a9cf4e4

improve tests for IdentityReplicationPolicy h/t @mimaison

bf85481

mimaison reviewed Jun 17, 2021

View reviewed changes

Comment thread ...t/mirror-client/src/main/java/org/apache/kafka/connect/mirror/IdentityReplicationPolicy.java Outdated

Ryanne Dolan and others added 3 commits June 21, 2021 16:16

drop unnecessary constructor from IdentityReplicationPolicy

627e099

Merge branch 'apache:trunk' into KAFKA-9726

b6628c4

Merge branch 'KAFKA-9726' of github.com:ryannedolan/kafka into KAFKA-…

5a5da9b

…9726

ryannedolan requested a review from mimaison June 21, 2021 21:21

mimaison approved these changes Jun 24, 2021

View reviewed changes

mention IdentityReplicationPolicy in upgrade notes

ada302c

ryannedolan commented Jun 29, 2021

View reviewed changes

ryannedolan and others added 2 commits June 30, 2021 14:40

Merge branch 'apache:trunk' into KAFKA-9726

5c99348

fix testOffsetSyncsTopicsOnTarget integration test to work with ident…

f9edd32

…ity replication

mimaison merged commit 93f5737 into apache:trunk Jul 1, 2021

ivanyu added a commit to aiven/kafka that referenced this pull request Jul 2, 2021

Merge pull request #1 from aiven/identity-replication-policy

dcae2d4

KAFKA-9726: Add IdentityReplicationPolicy to MirrorMaker2 (apache#10652)

Justinwins reviewed Aug 5, 2021

View reviewed changes

Justinwins reviewed Aug 18, 2021

View reviewed changes

Conversation

ryannedolan commented May 7, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mdedetrich commented May 27, 2021

Uh oh!

mdedetrich commented Jun 3, 2021

Uh oh!

ryannedolan commented Jun 3, 2021

Uh oh!

mimaison commented Jun 7, 2021

Uh oh!

mimaison left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ryannedolan commented Jun 17, 2021

Uh oh!

Uh oh!

mdedetrich commented Jun 24, 2021

Uh oh!

mimaison commented Jun 24, 2021

Uh oh!

mdedetrich commented Jun 28, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ryannedolan commented Jun 28, 2021

Uh oh!

mimaison commented Jun 28, 2021

Uh oh!

ryannedolan Jun 29, 2021

Choose a reason for hiding this comment

Uh oh!

mdedetrich Jun 30, 2021

Choose a reason for hiding this comment

Uh oh!

mimaison Jun 30, 2021

Choose a reason for hiding this comment

Uh oh!

mimaison commented Jun 30, 2021

Uh oh!

ryannedolan commented Jun 30, 2021

Uh oh!

mimaison commented Jul 1, 2021

Uh oh!

Justinwins Aug 5, 2021

Choose a reason for hiding this comment

Uh oh!

Justinwins Aug 6, 2021

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ryannedolan commented May 7, 2021 •

edited

Loading

mdedetrich commented Jun 28, 2021 •

edited

Loading