[ 
https://issues.apache.org/jira/browse/KAFKA-8735?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16896526#comment-16896526
 ] 

ASF GitHub Bot commented on KAFKA-8735:
---------------------------------------

qinghui-xu commented on pull request #7139: KAFKA-8735: Check properties file 
existence first
URL: https://github.com/apache/kafka/pull/7139
 
 
   To make BrokerMetadataCheckpoint more robust, and avoid a leak of 
abstraction.
   
   Details and rationales are in the jira ticket.
   
 
----------------------------------------------------------------
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:
us...@infra.apache.org


> BrokerMetadataCheckPoint should check metadata.properties existence itself 
> ---------------------------------------------------------------------------
>
>                 Key: KAFKA-8735
>                 URL: https://issues.apache.org/jira/browse/KAFKA-8735
>             Project: Kafka
>          Issue Type: Improvement
>            Reporter: Qinghui Xu
>            Priority: Major
>
> BrokerMetadataCheckPoint tries to read metadata.properties from log directory 
> during server start up. And it relies on org.apache.kafka.common.util.Utils 
> (from org.apache.kafka:kafka-clients) to load the properties file in a given 
> directory.
> During the process, we need to handle the case in which the properties file 
> does not exist (not as an error). Currently, BrokerMetadataCheckPoint relies 
> on the behavior of  `org.apache.kafka.common.util.Utils#loadProps` to find 
> out if the file exists or not: if the properties file is absent, it is 
> expecting NoSuchFileException (for branch 2.1 and above), and it was 
> expecting FileNotFoundException (for branch 2.0 and before). Knowing that 
> `org.apache.kafka.common.util.Utils#loadProps` signature throws only 
> IOException, this exception pattern matching is thus sort of leak of 
> abstraction making BrokerMetadataCheckPoint relies on the implementation 
> details of `org.apache.kafka.common.util.Utils#loadProps`. 
> This makes BrokerMetadataCheckPoint very fragile, especially when 
> `org.apache.kafka.common.util.Utils` and 
> `kafka.server.BrokerMetadataCheckPoint` are from different artifacts, an 
> example that I just ran into:
>  * We have a project A that depends on project B, and project B has a compile 
> time dependency on `org.apache.kafka:kafka-clients`. A is relying 
> `org.apach.kafka:kafka_2.11` in its tests: it will spawn some kafka brokers 
> in the tests.
>  * At first A and B are both using kafka libraries 2.0.1, and everything is 
> working fine
>  * At some point a newer version of B upgrades 
> `org.apache.kafka:kafka-clients` to 2.3.0
>  * When A wants to use the newer version of B, its tests are broken because 
> kafka brokers fail to start: now `org.apache.kafka.common.util.Utils` (2.3.0) 
> throws NoSucheFileException while BrokerMetadataCheckPoint (2.0.1) expects to 
> catch FileNotFoundException
> It would be much more reliable for BrokerMetadataCheckPoint to check the file 
> existence before trying to load the properties from the file.



--
This message was sent by Atlassian JIRA
(v7.6.14#76016)

Reply via email to