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/11/19 11:17:04 UTC
[GitHub] [kafka] bristy opened a new pull request #9621: KAFKA-9892: Producer state snapshot needs to be forced to disk
bristy opened a new pull request #9621:
URL: https://github.com/apache/kafka/pull/9621
*More detailed description of your change,
if necessary. The PR title and PR message become
the squashed commit message, so use a separate
comment to ping reviewers.*
FileChannel.close() does not guarantee modified buffer would be written on the file system. We are changing it with force() semantics to enforce file buffer and metadata written to filesystem ( FileChannel.force(true) updates buffer and metadata).
*Summary of testing strategy (including rationale)
I have run unittests after making the changes.
### 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] hachikuji commented on pull request #9621: KAFKA-9892: Producer state snapshot needs to be forced to disk
Posted by GitBox <gi...@apache.org>.
hachikuji commented on pull request #9621:
URL: https://github.com/apache/kafka/pull/9621#issuecomment-741974252
@bristy Thanks for the patch. I'd suggest changing to this:
```java
try {
fileChannel.write(buffer)
fileChannel.force(true)
} finally {
fileChannel.close()
}
```
If you don't have time to get back to this, I can push a commit.
----------------------------------------------------------------
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] bristy commented on a change in pull request #9621: KAFKA-9892: Producer state snapshot needs to be forced to disk
Posted by GitBox <gi...@apache.org>.
bristy commented on a change in pull request #9621:
URL: https://github.com/apache/kafka/pull/9621#discussion_r539562607
##########
File path: core/src/main/scala/kafka/log/ProducerStateManager.scala
##########
@@ -437,7 +437,7 @@ object ProducerStateManager {
val fileChannel = FileChannel.open(file.toPath, StandardOpenOption.CREATE, StandardOpenOption.WRITE)
try fileChannel.write(buffer)
- finally fileChannel.close()
+ finally fileChannel.force(true)
Review comment:
Fixed. Please check
----------------------------------------------------------------
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] junrao commented on a change in pull request #9621: KAFKA-9892: Producer state snapshot needs to be forced to disk
Posted by GitBox <gi...@apache.org>.
junrao commented on a change in pull request #9621:
URL: https://github.com/apache/kafka/pull/9621#discussion_r527105252
##########
File path: core/src/main/scala/kafka/log/ProducerStateManager.scala
##########
@@ -437,7 +437,7 @@ object ProducerStateManager {
val fileChannel = FileChannel.open(file.toPath, StandardOpenOption.CREATE, StandardOpenOption.WRITE)
try fileChannel.write(buffer)
- finally fileChannel.close()
+ finally fileChannel.force(true)
Review comment:
I think we need to do force and then close.
----------------------------------------------------------------
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] hachikuji merged pull request #9621: KAFKA-9892: Producer state snapshot needs to be forced to disk
Posted by GitBox <gi...@apache.org>.
hachikuji merged pull request #9621:
URL: https://github.com/apache/kafka/pull/9621
----------------------------------------------------------------
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] bristy commented on a change in pull request #9621: KAFKA-9892: Producer state snapshot needs to be forced to disk
Posted by GitBox <gi...@apache.org>.
bristy commented on a change in pull request #9621:
URL: https://github.com/apache/kafka/pull/9621#discussion_r539562607
##########
File path: core/src/main/scala/kafka/log/ProducerStateManager.scala
##########
@@ -437,7 +437,7 @@ object ProducerStateManager {
val fileChannel = FileChannel.open(file.toPath, StandardOpenOption.CREATE, StandardOpenOption.WRITE)
try fileChannel.write(buffer)
- finally fileChannel.close()
+ finally fileChannel.force(true)
Review comment:
Raising fix.
----------------------------------------------------------------
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