-
Notifications
You must be signed in to change notification settings - Fork 1.4k
docs(review): minor updates to mm2 config #12273
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: prmellor <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR refines the MirrorMaker 2 documentation to improve clarity, consistency, and technical precision following a review. The changes focus on better explaining configuration concepts, improving readability, and providing more accurate descriptions of MirrorMaker 2 behavior.
- Clarifies configuration paths and terminology (e.g., "connector configuration" vs "MirrorMaker 2 configuration")
- Expands explanations of Kafka Connect worker settings and internal topics
- Improves wording and formatting for better readability across multiple configuration sections
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
con-mm2-recovery.adoc |
Updates disaster recovery documentation, removes trailing spaces and clarifies that IdentityReplicationPolicy is set in connector configuration |
con-config-mm2-multiple-instances.adoc |
Expands explanation of Kafka Connect worker settings and internal topics, clarifies when unique configurations are needed |
con-config-mirrormaker2-sync.adoc |
Simplifies wording about offset sync topic location and Java utility usage |
con-config-mirrormaker2-sync-acls.adoc |
Improves clarity of ACL synchronization behavior description |
con-config-mirrormaker2-producers-consumers.adoc |
Refines producer/consumer descriptions and adjusts table column widths; simplifies configuration paths |
con-config-mirrormaker2-connect-workers.adoc |
Enhances explanation of Kafka Connect worker behavior and adds inline comments to example configuration |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| |Loads the `offset-syncs` topic. | ||
| |mirrors.checkpointConnector.config: consumer.* | ||
| |Consumes data from the `offset-syncs` topic. | ||
| |`consumer.*` |
Copilot
AI
Dec 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The configuration path is incomplete. This should include the full path context showing it goes under "mirrors.checkpointConnector.config" section.
| . Deploy a new MirrorMaker 2 instance to replicate data from the active recovery cluster to the rebuilt cluster. | ||
| Treat the rebuilt cluster as the passive cluster during this replication process. | ||
| To prevent automatic renaming of topics, configure MirrorMaker 2 to use the `IdentityReplicationPolicy` by setting the `replication.policy.class` property in the MirrorMaker 2 configuration. | ||
| To prevent automatic renaming of topics, configure MirrorMaker 2 to use the `IdentityReplicationPolicy` by setting the `replication.policy.class` property in the connector configuration. |
Copilot
AI
Dec 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The change from "MirrorMaker 2 configuration" to "connector configuration" is more precise, but the sentence should clarify which connector. The IdentityReplicationPolicy is set on the MirrorSourceConnector specifically. Consider specifying "MirrorSourceConnector configuration" for clarity, as MirrorMaker 2 has multiple connector types (MirrorSourceConnector, MirrorCheckpointConnector, MirrorHeartbeatConnector).
| To prevent automatic renaming of topics, configure MirrorMaker 2 to use the `IdentityReplicationPolicy` by setting the `replication.policy.class` property in the connector configuration. | |
| To prevent automatic renaming of topics, configure MirrorMaker 2 to use the `IdentityReplicationPolicy` by setting the `replication.policy.class` property in the MirrorSourceConnector configuration. |
|
|
||
| Multiple instances attempting to use the same internal topics will cause unexpected errors, so you must change the values of these properties for each instance. | ||
| * Multiple MirrorMaker 2 instances use the same source Kafka cluster but different target Kafka clusters | ||
| * Deploying active-active replication where each MirrorMaker 2 instance uses a different target Kafka cluster |
Copilot
AI
Dec 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The second bullet point should include a comma before "where" for better readability. The sentence is a complex clause where the "where" introduces additional descriptive information. It should read: "Deploying active-active replication, where each MirrorMaker 2 instance uses a different target Kafka cluster".
| * Deploying active-active replication where each MirrorMaker 2 instance uses a different target Kafka cluster | |
| * Deploying active-active replication, where each MirrorMaker 2 instance uses a different target Kafka cluster |
| |mirrors.sourceConnector.config: producer.override.* | ||
| |Sends replicated topic messages to the target Kafka cluster. | ||
| Tune this producer when handling large volumes of data. | ||
| |`producer.override.*` |
Copilot
AI
Dec 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The configuration path is incomplete. Based on line 18 which mentions "where you can add configuration" and the context that these are MirrorMaker 2 connector configurations, the path should include the full configuration location. The original text "mirrors.sourceConnector.config: producer.override." provided context showing where to place this configuration. The new format only shows "producer.override." without indicating this goes under "mirrors.sourceConnector.config" section.
| |Writes to the `offset-syncs` topic, which maps the source and target offsets for replicated topic partitions. | ||
| |mirrors.sourceConnector.config: producer.* | ||
| |Writes to the `offset-syncs` topic, which maps source and target offsets. | ||
| |`producer.*` |
Copilot
AI
Dec 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The configuration path is incomplete. Similar to the producer.override.* configuration above, this should include the full path context showing it goes under "mirrors.sourceConnector.config" section.
| |Retrieves topic messages from the source Kafka cluster. | ||
| |mirrors.sourceConnector.config: consumer.* | ||
| |Consumes topic messages from the source Kafka cluster. | ||
| |`consumer.*` |
Copilot
AI
Dec 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The configuration path is incomplete. This should include the full path context showing it goes under "mirrors.sourceConnector.config" section.
| |Emits consumer offset checkpoints. | ||
| |mirrors.checkpointConnector.config: producer.override.* | ||
| |Emits consumer group offset checkpoints. | ||
| |`producer.override.*` |
Copilot
AI
Dec 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The configuration path is incomplete. This should include the full path context showing it goes under "mirrors.checkpointConnector.config" section.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #12273 +/- ##
============================================
- Coverage 74.81% 74.80% -0.01%
Complexity 6627 6627
============================================
Files 376 376
Lines 25345 25345
Branches 3402 3402
============================================
- Hits 18962 18960 -2
- Misses 4995 4996 +1
- Partials 1388 1389 +1 🚀 New features to boost your workflow:
|
scholzj
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One nit. LGTM otherwise.
| <4> Kafka topic that stores connector and task status updates. | ||
|
|
||
| NOTE: Values for the three topics must be the same for all instances with the same `group.id`. | ||
| NOTE: All workers that belong to the same Kafka Connect cluster must use the same values for these properties. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure this is meaningful as there is now way user can configure it per-worker-node.
It is even more confusing with the two sentences below as users might assume that they can configure multiple MM2s to the same values which is not true.
I would remove this note.
|
|
||
| * ACL rules apply to both source and target topics | ||
| * Users with source topic access automatically get equivalent target topic access | ||
| * Users with access to source topics automatically receive equivalent access to replicated target topics |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not quite true. MirrorSourceConnector mirror solely topic ACLs and does not mirror the ALLOW WRITE ACL.
| You can change this behavior by setting the `offset-syncs.topic.location` connector configuration to `target`. | ||
|
|
||
| Changing the location of the topic allows MirrorMaker 2 to operate when the connector has read-only access to the source cluster. | ||
| In this case, the connector must have read and write access to the target cluster. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file seems to be about MirrorCheckpointConnector. If so regardless of the location of the offset-sycns topics, this connector should already have read and write access to the checkpoints topic.
Documentation
Minor updates to the MM2 config docs following review
Checklist
Please go through this checklist and make sure all applicable tasks have been done