Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DO NOT MERGE - KAFKA-1194 PARTIAL bug fix #5933

Closed
wants to merge 1 commit into from
Closed

DO NOT MERGE - KAFKA-1194 PARTIAL bug fix #5933

wants to merge 1 commit into from

Conversation

kobihikri
Copy link

This is a partial fix to the retention mechanism on Windows.
It makes sure to close a segment file handles prior to deletion trial.
Why is it partial? As it cannot work together will the log compaction policy.

Should work with a similar to the following configuration:

log.segment.bytes=1048576
log.retention.bytes= 5485760
log.retention.minutes = 1
log.retention.check.interval.minutes = 1
log.cleanup.policy = delete
log.cleaner.enable = false

log.roll.ms = 10000 <= Can be specified but will not cause any failure without the above two lines of configuration (log.cleanup.policy, log.cleaner.enable)

Committer Checklist (excluded from commit message)

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation (including upgrade notes)

It makes sure to close a segment file handles prior to deletion trial.
@simplesteph
Copy link
Contributor

Tested and not working for delete topics.

zookeeper-server-start ...
kafka-server-start
kafka-topics --zookeeper localhost:2181 --topic test --create --partitions 2 --replication-factor 1
kafka-console-producer --broker-list localhost:9092 --topic test
> foo
> bar
kafka-topics  --zookeeper localhost:2181 --topic test --delete

Fails with

[2018-11-20 09:18:43,525] ERROR Error while renaming dir for test-2 in log dir C:\tmp\kafka-logs (kafka.server.LogDirFailureChannel)
java.nio.file.AccessDeniedException: C:\tmp\kafka-logs\test-2 -> C:\tmp\kafka-logs\test-2.f221682bcfbf429db2f6cafe7256ef9a-delete
        at sun.nio.fs.WindowsException.translateToIOException(Unknown Source)
        at sun.nio.fs.WindowsException.rethrowAsIOException(Unknown Source)
        at sun.nio.fs.WindowsFileCopy.move(Unknown Source)
        at sun.nio.fs.WindowsFileSystemProvider.move(Unknown Source)
        at java.nio.file.Files.move(Unknown Source)
        at org.apache.kafka.common.utils.Utils.atomicMoveWithFallback(Utils.java:802)
        at kafka.log.Log.$anonfun$renameDir$2(Log.scala:730)
        at scala.runtime.java8.JFunction0$mcV$sp.apply(JFunction0$mcV$sp.java:12)
        at kafka.log.Log.maybeHandleIOException(Log.scala:1944)
        at kafka.log.Log.renameDir(Log.scala:728)
        at kafka.log.LogManager.asyncDelete(LogManager.scala:857)
        at kafka.cluster.Partition.$anonfun$delete$1(Partition.scala:354)
        at kafka.utils.CoreUtils$.inLock(CoreUtils.scala:251)
        at kafka.utils.CoreUtils$.inWriteLock(CoreUtils.scala:259)
        at kafka.cluster.Partition.delete(Partition.scala:348)
        at kafka.server.ReplicaManager.stopReplica(ReplicaManager.scala:351)
        at kafka.server.ReplicaManager.$anonfun$stopReplicas$2(ReplicaManager.scala:381)
        at scala.collection.Iterator.foreach(Iterator.scala:937)
        at scala.collection.Iterator.foreach$(Iterator.scala:937)
        at scala.collection.AbstractIterator.foreach(Iterator.scala:1425)
        at scala.collection.IterableLike.foreach(IterableLike.scala:70)
        at scala.collection.IterableLike.foreach$(IterableLike.scala:69)
        at scala.collection.AbstractIterable.foreach(Iterable.scala:54)
        at kafka.server.ReplicaManager.stopReplicas(ReplicaManager.scala:379)
        at kafka.server.KafkaApis.handleStopReplicaRequest(KafkaApis.scala:200)
        at kafka.server.KafkaApis.handle(KafkaApis.scala:111)
        at kafka.server.KafkaRequestHandler.run(KafkaRequestHandler.scala:69)
        at java.lang.Thread.run(Unknown Source)
        Suppressed: java.nio.file.AccessDeniedException: C:\tmp\kafka-logs\test-2 -> C:\tmp\kafka-logs\test-2.f221682bcfbf429db2f6cafe7256ef9a-delete
                at sun.nio.fs.WindowsException.translateToIOException(Unknown Source)
                at sun.nio.fs.WindowsException.rethrowAsIOException(Unknown Source)
                at sun.nio.fs.WindowsFileCopy.move(Unknown Source)
                at sun.nio.fs.WindowsFileSystemProvider.move(Unknown Source)
                at java.nio.file.Files.move(Unknown Source)
                at org.apache.kafka.common.utils.Utils.atomicMoveWithFallback(Utils.java:799)
                ... 22 more

Please test on your end for this test case too, and I think this PR should address this error

@kobihikri
Copy link
Author

Hi Stephane,

Thank you for testing.
I do think that topic deletion should work on windows as well, but do we really want to bind it with the log retention feature?
What I am trying to do here, is "attack" the lacking windows support in a "step by step" manner, according to features which are most requested by the community.

That been said, I will take a look at the directory renaming error as well (hopefully on Sunday), as I am traveling abroad shortly.

@simplesteph
Copy link
Contributor

To me, anything that has to do with log files being moved, renamed and deleted should be addressed as part of this PR.

@kobihikri
Copy link
Author

Thanks Stephane,

By binding the entire file handling (renaming, moving, deleting) as a single issue, we are preventing the community from having a better version.

I do agree that the topics you've mentioned should be addressed, but to be practical - I cannot estimate when I will have the time to solve all these issues.

Personally, I can only take it step by step.

I would advise looking at this PR as a file retention enabler.

It's the minimal non obtrusive change (I was able to come up with) required in order to make the mechanism work.

It seems (to me) that as these issues are being handled by people, based on free time and good will, it is worth considering the approach I've proposed.

But that's just me :-)

@kobihikri kobihikri closed this Nov 26, 2018
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