hudi-agent commented on code in PR #19202:
URL: https://github.com/apache/hudi/pull/19202#discussion_r3530038820
##########
hudi-flink-datasource/hudi-flink/src/main/java/org/apache/hudi/table/format/cdc/CdcIterators.java:
##########
@@ -135,14 +153,23 @@ public boolean hasNext() {
}
@Override
- public RowData next() {
+ public synchronized RowData next() {
+ if (closed || recordIterator == null) {
Review Comment:
🤖 nit: the combined `closed || recordIterator == null` condition with a
ternary message makes you mentally parse the same `closed` flag twice. Two
separate guards would be clearer and keep each error message self-contained:
```java
if (closed) {
throw new NoSuchElementException("CDC file-splits iterator was closed
during teardown");
}
if (recordIterator == null) {
throw new NoSuchElementException("No more CDC records");
}
```
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
##########
hudi-flink-datasource/hudi-flink/src/test/java/org/apache/hudi/table/ITTestHoodieDataSource.java:
##########
@@ -4108,6 +4130,32 @@ private static boolean
containsReadNextRowGroupFrame(Throwable t) {
return false;
}
+ /**
+ * True for a real {@link NoSuchElementException} as well as one wrapped in
Flink's
+ * {@code SerializedThrowable} when the failure is propagated back from the
cluster (its
+ * {@code toString()} preserves the original {@code
java.util.NoSuchElementException} prefix).
+ */
+ private static boolean isNoSuchElementException(Throwable t) {
+ return t instanceof NoSuchElementException
+ || t.toString().startsWith(NoSuchElementException.class.getName());
+ }
+
+ /**
+ * Whether {@code t}'s stack trace (preserved even through {@code
SerializedThrowable}) contains a
+ * frame from one of the CDC read iterators that a teardown {@code close()}
can null out mid-drain -
+ * {@code CdcIterators$CdcFileSplitsIterator} or {@code
CdcIterators$BaseImageIterator}.
+ */
+ private static boolean containsCdcIteratorTeardownFrame(Throwable t) {
+ for (StackTraceElement frame : t.getStackTrace()) {
+ String className = frame.getClassName();
+ if (className.contains("CdcIterators$CdcFileSplitsIterator")
Review Comment:
🤖 nit: `CdcFileSplitsIterator` is `public static`, so the hardcoded string
`"CdcIterators$CdcFileSplitsIterator"` could silently go stale on a rename —
could you use `CdcIterators.CdcFileSplitsIterator.class.getName()` there? (If
`BaseImageIterator` is also accessible the same applies to the second check.)
<sub><i>⚠️ AI-generated; verify before applying. React 👍/👎 to flag
quality.</i></sub>
--
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]
