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 2021/05/25 13:42:07 UTC
[GitHub] [kafka] ijuma opened a new pull request #10761: MINOR: Don't ignore deletion of partition metadata file and log topic id clean-ups
ijuma opened a new pull request #10761:
URL: https://github.com/apache/kafka/pull/10761
Log if deletion fails and don't expose log topic id for mutability outside of `assignTopicId()`.
### 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] jolshan commented on a change in pull request #10761: MINOR: Don't ignore deletion of partition metadata file and log topic id clean-ups
Posted by GitBox <gi...@apache.org>.
jolshan commented on a change in pull request #10761:
URL: https://github.com/apache/kafka/pull/10761#discussion_r639833755
##########
File path: core/src/main/scala/kafka/log/Log.scala
##########
@@ -325,19 +325,25 @@ class Log(@volatile private var _dir: File,
// Ensure we do not try to assign a provided topicId that is inconsistent with the ID on file.
if (partitionMetadataFile.exists()) {
if (!keepPartitionMetadataFile)
- partitionMetadataFile.delete()
+ try partitionMetadataFile.delete()
+ catch {
+ case e: IOException =>
+ error(s"Error while trying to delete partition metadata file ${partitionMetadataFile}", e)
Review comment:
Ah this is a good point. The toString method could include the file path.
--
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] ijuma commented on pull request #10761: MINOR: Don't ignore deletion of partition metadata file and log topic id clean-ups
Posted by GitBox <gi...@apache.org>.
ijuma commented on pull request #10761:
URL: https://github.com/apache/kafka/pull/10761#issuecomment-849931983
JDK 11 build passed, a few unrelated failures for the other builds:
> Build / JDK 8 and Scala 2.12 / testMetricsDuringTopicCreateDelete() – kafka.integration.MetricsDuringTopicCreationDeletionTest
> 16s
> Build / JDK 15 and Scala 2.13 / testMetricsDuringTopicCreateDelete() – kafka.integration.MetricsDuringTopicCreationDeletionTest
> 7s
> Build / JDK 15 and Scala 2.13 / testCreateClusterAndCreateAndManyTopicsWithManyPartitions() – kafka.server.RaftClusterTest
> 19s
> Build / JDK 15 and Scala 2.13 / testCreateClusterAndWaitForBrokerInRunningState() – kafka.server.RaftClusterTest
> 1m 11s
> Build / JDK 15 and Scala 2.13 / testCreateClusterAndCreateListDeleteTopic() – kafka.server.RaftClusterTest
> 16s
> Build / JDK 15 and Scala 2.13 / testCreateClusterAndCreateListDeleteTopic() – kafka.server.RaftClusterTest
--
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] ijuma commented on a change in pull request #10761: MINOR: Don't ignore deletion of partition metadata file and log topic id clean-ups
Posted by GitBox <gi...@apache.org>.
ijuma commented on a change in pull request #10761:
URL: https://github.com/apache/kafka/pull/10761#discussion_r640639035
##########
File path: core/src/main/scala/kafka/log/Log.scala
##########
@@ -325,19 +325,25 @@ class Log(@volatile private var _dir: File,
// Ensure we do not try to assign a provided topicId that is inconsistent with the ID on file.
if (partitionMetadataFile.exists()) {
if (!keepPartitionMetadataFile)
- partitionMetadataFile.delete()
+ try partitionMetadataFile.delete()
+ catch {
+ case e: IOException =>
+ error(s"Error while trying to delete partition metadata file ${partitionMetadataFile}", e)
Review comment:
Good point, addressed.
--
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] ijuma merged pull request #10761: MINOR: Don't ignore deletion of partition metadata file and log topic id clean-ups
Posted by GitBox <gi...@apache.org>.
ijuma merged pull request #10761:
URL: https://github.com/apache/kafka/pull/10761
--
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] chia7712 commented on a change in pull request #10761: MINOR: Don't ignore deletion of partition metadata file and log topic id clean-ups
Posted by GitBox <gi...@apache.org>.
chia7712 commented on a change in pull request #10761:
URL: https://github.com/apache/kafka/pull/10761#discussion_r639477851
##########
File path: core/src/main/scala/kafka/log/Log.scala
##########
@@ -325,19 +325,25 @@ class Log(@volatile private var _dir: File,
// Ensure we do not try to assign a provided topicId that is inconsistent with the ID on file.
if (partitionMetadataFile.exists()) {
if (!keepPartitionMetadataFile)
- partitionMetadataFile.delete()
+ try partitionMetadataFile.delete()
+ catch {
+ case e: IOException =>
+ error(s"Error while trying to delete partition metadata file ${partitionMetadataFile}", e)
Review comment:
Should `partitionMetadataFile` override `toString` to offer readable message?
--
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] ijuma commented on pull request #10761: MINOR: Don't ignore deletion of partition metadata file and log topic id clean-ups
Posted by GitBox <gi...@apache.org>.
ijuma commented on pull request #10761:
URL: https://github.com/apache/kafka/pull/10761#issuecomment-847881296
cc @jolshan
--
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