You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2020/05/04 04:58:15 UTC

[GitHub] [kafka] C0urante opened a new pull request #8608: KAFKA-9950: Construct new ConfigDef for MirrorTaskConfig before defining new properties

C0urante opened a new pull request #8608:
URL: https://github.com/apache/kafka/pull/8608


   [Jira](https://issues.apache.org/jira/browse/KAFKA-9950)
   
   MM2 is currently sharing the same `ConfigDef` object for all its connectors and tasks, which would be fine _if_ that object were used as-is. However, the `MirrorTaskConfig` class mutates the `ConfigDef` by defining additional properties, which leads to a potential `ConcurrentModificationException` during worker configuration validation and unintended inclusion of those new properties in the `ConfigDef` for the connectors which in turn is then visible via the REST API's `/connectors/{name}/config/validate` endpoint.
   
   The fix here is a one-liner that just creates a copy of the `ConfigDef` before defining new properties.
   
   A unit test is added that fails without this fix and passes with it.
   
   ### Committer Checklist (excluded from commit message)
   - [ ] Verify design and implementation 
   - [ ] Verify test coverage and CI build status
   - [ ] Verify documentation (including upgrade notes)
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [kafka] C0urante commented on pull request #8608: KAFKA-9950: Construct new ConfigDef for MirrorTaskConfig before defining new properties

Posted by GitBox <gi...@apache.org>.
C0urante commented on pull request #8608:
URL: https://github.com/apache/kafka/pull/8608#issuecomment-623571317


   Thanks Ryanne!
   
   @rhauch, @kkonstantine could one of you take a look at this when you have a chance?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [kafka] kkonstantine commented on pull request #8608: KAFKA-9950: Construct new ConfigDef for MirrorTaskConfig before defining new properties

Posted by GitBox <gi...@apache.org>.
kkonstantine commented on pull request #8608:
URL: https://github.com/apache/kafka/pull/8608#issuecomment-631254109


   ok to test


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [kafka] kkonstantine commented on pull request #8608: KAFKA-9950: Construct new ConfigDef for MirrorTaskConfig before defining new properties

Posted by GitBox <gi...@apache.org>.
kkonstantine commented on pull request #8608:
URL: https://github.com/apache/kafka/pull/8608#issuecomment-631867826


   jdk8: single failure on known flaky test: `org.apache.kafka.streams.integration.EosBetaUpgradeIntegrationTest.shouldUpgradeFromEosAlphaToEosBeta[true]`
   jdk11: single failure on known flaky test: `org.apache.kafka.streams.integration.EosBetaUpgradeIntegrationTest.shouldUpgradeFromEosAlphaToEosBeta[false]`
   jdk14: success
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [kafka] C0urante commented on pull request #8608: KAFKA-9950: Construct new ConfigDef for MirrorTaskConfig before defining new properties

Posted by GitBox <gi...@apache.org>.
C0urante commented on pull request #8608:
URL: https://github.com/apache/kafka/pull/8608#issuecomment-623257536


   @ryannedolan based on your experience MM2, would you be willing to take a look at this?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [kafka] kkonstantine merged pull request #8608: KAFKA-9950: Construct new ConfigDef for MirrorTaskConfig before defining new properties

Posted by GitBox <gi...@apache.org>.
kkonstantine merged pull request #8608:
URL: https://github.com/apache/kafka/pull/8608


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org