wcarlson5 commented on a change in pull request #9720:
URL: https://github.com/apache/kafka/pull/9720#discussion_r553613699
##########
File path:
streams/src/test/java/org/apache/kafka/streams/integration/EosBetaUpgradeIntegrationTest.java
##########
@@ -526,12 +521,10 @@ public void shouldUpgradeFromEosAlphaToEosBeta() throws
Exception {
assignmentListener.waitForNextStableAssignment(MAX_WAIT_TIME_MS);
- waitForStateTransition(stateTransitions2, CRASH);
-
commitErrorInjectedClient2.set(false);
stateTransitions2.clear();
streams2Alpha.close();
- waitForStateTransition(stateTransitions2, CLOSE_CRASHED);
Review comment:
the call to `close()` will have no effect. The `PENDING_ERROR -> ERROR`
transition is like the `PENDING_SHUTDOWN -> NOT_RUNNING ` where the transition
from the `PENDING...` state is automatic
##########
File path:
streams/src/test/java/org/apache/kafka/streams/integration/StandbyTaskEOSIntegrationTest.java
##########
@@ -174,8 +177,8 @@ private KafkaStreams buildStreamWithDirtyStateDir(final
String stateDirPath,
}
@Test
- @Deprecated
public void shouldWipeOutStandbyStateDirectoryIfCheckpointIsMissing()
throws Exception {
+ final long time = System.currentTimeMillis();
Review comment:
Maybe we will. It doesn't seems to be a problem for the other branches
but something with how the test changes in this PR exposed it. This happened in
the handler int tests too. I think as long as the other tests don't change it
won't be a problem and if those tests change we can fix it then.
But if you think it should be fixed in the other branches anyways I'll trust
your judgement.
##########
File path: streams/src/main/java/org/apache/kafka/streams/KafkaStreams.java
##########
@@ -201,32 +201,33 @@
* | | |
* | v |
* | +------+-------+ +----+-------+
- * +-----> | Pending |<--- | Error (5) |
- * | Shutdown (3) | +------------+
- * +------+-------+
- * |
- * v
- * +------+-------+
- * | Not |
- * | Running (4) |
+ * +-----> | Pending | | Pending |
+ * | Shutdown (3) | | Error (5) |
+ * +------+-------+ +-----+------+
+ * | |
+ * v v
+ * +------+-------+ +-----+--------+
+ * | Not | | Error (6) |
+ * | Running (4) | +--------------+
* +--------------+
*
*
* </pre>
* Note the following:
* - RUNNING state will transit to REBALANCING if any of its threads is in
PARTITION_REVOKED or PARTITIONS_ASSIGNED state
* - REBALANCING state will transit to RUNNING if all of its threads are
in RUNNING state
- * - Any state except NOT_RUNNING can go to PENDING_SHUTDOWN (whenever
close is called)
+ * - Any state except NOT_RUNNING, PENDING_ERROR or ERROR can go to
PENDING_SHUTDOWN (whenever close is called)
* - Of special importance: If the global stream thread dies, or all
stream threads die (or both) then
- * the instance will be in the ERROR state. The user will need to close
it.
+ * the instance will be in the ERROR state. The user will not need to
close it.
Review comment:
the handler will call close, but the user will not need to. The
`PENDING_ERROR` state is indicating the resources are closing before the
transition to `ERROR` after which no more work will be done. We made it so the
user can call close on `PENDING_ERROR` or `ERROR` but it will only log a warning
----------------------------------------------------------------
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]
