ableegoldman commented on a change in pull request #8856:
URL: https://github.com/apache/kafka/pull/8856#discussion_r439549676
##########
File path:
streams/src/main/java/org/apache/kafka/streams/processor/internals/StreamTask.java
##########
@@ -528,7 +521,8 @@ private void maybeScheduleCheckpoint() {
private void writeCheckpointIfNeed() {
if (commitNeeded) {
- throw new IllegalStateException("A checkpoint should only be
written if no commit is needed.");
+ throw new IllegalStateException("A checkpoint should only be
written if the previous commit has completed"
+ + " and there is no new commit
needed.");
Review comment:
> and there is no new commit needed -> this seem to be miss leading
because the commitNeeded flag is not really a guard for this case.
Isn't that exactly what the `commitNeeded` flag is?
That said, looking at this again, I agree this new phrasing doesn't really
make sense. But the original comment took me a while to understand also
(shouldn't we _only_ write a checkpoint if there's a commit needed?) IIUC the
point is that we should not write a checkpoint before doing the actual commit,
ie there should not be pending uncommitted data when we write the checkpoint.
----------------------------------------------------------------
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:
us...@infra.apache.org
