squah-confluent commented on code in PR #22969:
URL: https://github.com/apache/kafka/pull/22969#discussion_r3730122021
##########
coordinator-common/src/test/java/org/apache/kafka/coordinator/common/runtime/CoordinatorRuntimeTest.java:
##########
@@ -5338,6 +5338,72 @@ public void
testCompressibleRecordTriggersFlushAndSucceeds() throws Exception {
assertEquals("response2", write2.get(5, TimeUnit.SECONDS));
}
+ @Test
+ public void testLargeCompressibleRecordSucceeds() throws Exception {
Review Comment:
I'm not 100% on board with this because that's a case where we have already
run into a bug. In normal operation there should never be an empty batch lying
around. I'm trying to keep KAFKA-20845 scoped to the happy path - the unhappy
path is broken for non-replaying operations in a few ways*
\* Off the top of my head, these include:
* A non-replaying operation that throws an exception midway through will
leave group coordinator state partially updated with no records written.
* Same for an exception midway through `append()`, except we may leave an
empty batch lying around. It's wrong to continue using that batch and commit
it, which we may currently do (even before this PR).
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
