HyukjinKwon commented on code in PR #57580:
URL: https://github.com/apache/spark/pull/57580#discussion_r3710507495
##########
sql/catalyst/src/main/java/org/apache/spark/sql/connector/read/streaming/SupportsRealTimeRead.java:
##########
@@ -78,12 +78,40 @@ public Optional<Long> recArrivalTime() {
* Alternative function to be called than next(), that proceed to the next
record. The different
* from next() is that, if there is no more records, the call needs to
keep waiting until
* the timeout.
- * @param startTimeMs the base time (milliseconds) the was used to
calculate the timeout.
- * Sources should use it as the reference time to start
waiting for the next
- * record instead of getting the latest time from
LowLatencyClock.
+ * <p>
+ * A source must override exactly one of {@link #nextWithTimeout(Long)} and
+ * {@link #nextWithTimeout(Long, Long)}. This single-argument variant is
enough for sources that
+ * do not need the engine's reference start time. The engine always invokes
+ * {@link #nextWithTimeout(Long, Long)}, whose default implementation
delegates here, so
+ * overriding only this method is sufficient. The default implementation
of this method throws,
+ * since the two-argument variant must have been overridden if this one
was not.
+ * @param timeoutMs if no result is available after this timeout
(milliseconds), return
+ * @return {@link RecordStatus} describing whether a record is available
and its arrival time
+ * @throws IOException
+ */
+ default RecordStatus nextWithTimeout(Long timeoutMs) throws IOException {
Review Comment:
A class-level note would help third-party source authors: implement
`nextWithTimeout(Long timeoutMs)` for the common case, and only override the
two-arg `nextWithTimeout(Long startTimeMs, Long timeoutMs)` if you need the
engine's reference start time (`LowLatencyClock` is engine-internal, not part
of the third-party contract). The per-method javadoc already says "override
exactly one", but since both are now `default` the "must implement one" check
only fails at runtime, so the high-level "which one and why" up front is worth
it. (Raised by @HeartSaVioR as well.)
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
