dongjinleekr commented on pull request #9414:
URL: https://github.com/apache/kafka/pull/9414#issuecomment-806739608
Add to this, I also updated `EOSUncleanShutdownIntegrationTest` also. I
inspected the history of this test suite precisely and found the following:
1. This suite was introduced in c2ec974 by @abbccdda. It asserts that the
Task's statestore directory (not root one in KafkaStreams's `state.dir`
property) is deleted after an unclean shutdown. However, IMHO this assertion is
a little bit inaccurate since the Task's statestore directory may not be
deleted, for example, with an empty checkpoint file.
2. A comment is added on L160 in d3c067f by @guozhangwang - 'the state
directory should still exist with the empty checkpoint file'. However, IMHO
this comment seems to be a mistake, since `assertFalse(stateDir.exists());` on
L161 asserts that the Task's statestore directory does not exist.
There are some other commits on this file later, but all of them are just
minor improvements, not changing the testing logic.
With this PR, Kafka Streams now deletes the empty application statestore
directory (i.e., `{state.dir}/{application.id}`). So, I improved this suite
like this:
1. After initialization, wait until the Task's statestore directory is
populated. (i.e., `{state.dir}/{application.id}/0_0/*`)
2. Assert that one of the following is satisfied:
- The Task's statestore directory is empty, so deleted.
- The Task's statestore directory is not empty but without a checkpoint
file.
- The Task's statestore directory is not empty but with an empty
checkpoint file.
(Wait, then should we separate the modifications on
`EOSUncleanShutdownIntegrationTest` into an independent PR?)
Please have a look when you are free. Thanks again for reviewing my PR!
:smiley:
--
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:
[email protected]
