Skip to content

Conversation

@divijvaidya
Copy link
Member

@divijvaidya divijvaidya commented May 23, 2022

File.createNewFile() returns a boolean signifying whether the file creation was successful or not. There are multiple places in the Kafka code base where we are not checking the value of the returned boolean.

Replacing it with the Files.createFile() API will decrease the chances of inadvertent bugs in the code base since Files.createFile() API thrown exceptions when the creation is not successful.

Note that this is one of many PRs which would help us leverage new features introduced in NIO.2 Java APIs. The parent task is listed at https://issues.apache.org/jira/browse/KAFKA-13928

@divijvaidya
Copy link
Member Author

@hachikuji @mimaison perhaps you folks would be interested to look into this code improvement?

@mimaison
Copy link
Member

@divijvaidya
Copy link
Member Author

Thanks @divijvaidya for the PR!

Should we also update this instance https://github.com/apache/kafka/blob/trunk/core/src/main/scala/kafka/log/AbstractIndex.scala#L110 ?

@mimaison I am going to address that code block in a separate PR along with replacement of RandomAccessFile with newer (SeekableByteChannel) APIs. This code block also requires some refactoring since we have code logic which relies on whether a new file was created or not. I will prefer to pick this file separately.

@divijvaidya
Copy link
Member Author

@mimaison please take another look at it! Thanks.

Copy link
Member

@mimaison mimaison left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for the PR

@mimaison mimaison merged commit 0a50005 into apache:trunk Jun 10, 2022
@divijvaidya divijvaidya deleted the KAFKA-13928-1 branch June 10, 2022 14:43
rajinisivaram added a commit to confluentinc/kafka that referenced this pull request Jun 12, 2022
…-2022

* apache/trunk: (52 commits)
  KAFKA-13967: Document guarantees for producer callbacks on transaction commit (apache#12264)
  [KAFKA-13848] Clients remain connected after SASL re-authentication f… (apache#12179)
  KAFKA-10000: Zombie fencing logic (apache#11779)
  KAFKA-13947: Use %d formatting for integers rather than %s (apache#12267)
  KAFKA-13929: Replace legacy File.createNewFile() with NIO.2 Files.createFile() (apache#12197)
  KAFKA-13780: Generate OpenAPI file for Connect REST API (apache#12067)
  KAFKA-13917: Avoid calling lookupCoordinator() in tight loop (apache#12180)
  KAFKA-10199: Implement removing active and standby tasks from the state updater (apache#12270)
  MINOR: Update Scala to 2.13.8 in gradle.properties (apache#12273)
  MINOR: add java 8/scala 2.12 deprecation info in doc (apache#12261)
  ...

 Conflicts:
	gradle.properties
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants