tzulitai commented on code in PR #22303:
URL: https://github.com/apache/flink/pull/22303#discussion_r1152668428
##########
flink-connectors/flink-connector-kafka/src/main/java/org/apache/flink/connector/kafka/sink/KafkaWriter.java:
##########
@@ -420,6 +420,7 @@ private void checkAsyncException() throws IOException {
if (e != null) {
asyncProducerException = null;
+ numRecordsOutErrorsCounter.inc();
Review Comment:
On that matter, for leaner and more fine-grained test cases, additionally I
think we should:
1. Break up `testErrorPropagationAndErrorsCounter` test into 2 separate
tests that individually test error propagation and the error counter,
separately.
2. consider removing the `testNumRecordsOutErrorsCounterMetric` test as it
seems to have overlapping coverage with your new test (or improve that one).
The `testNumRecordsOutErrorsCounterMetric` is really bloated, though, as it's
using an awkward way just to trigger an async exception through the callback.
--
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: issues-unsubscr...@flink.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
