LuciferYang commented on code in PR #56742:
URL: https://github.com/apache/spark/pull/56742#discussion_r3467952321
##########
sql/connect/server/src/main/scala/org/apache/spark/sql/connect/pipelines/PipelineEventSender.scala:
##########
@@ -174,3 +221,9 @@ class PipelineEventSender(
protoEventBuilder.build()
}
}
+
+object PipelineEventSender {
+ // Minimum interval between dropped-event warnings, to avoid flooding the
logs when the queue
+ // stays full. Mirrors AsyncEventQueue.LOGGING_INTERVAL.
+ private val DROPPED_EVENT_LOG_INTERVAL_MS = 60 * 1000
Review Comment:
Done, switched to `60L * 1000`.
##########
sql/connect/server/src/test/scala/org/apache/spark/sql/connect/pipelines/PipelineEventSenderSuite.scala:
##########
@@ -230,4 +235,107 @@ class PipelineEventSenderSuite extends
SparkDeclarativePipelinesServerTest with
eventSender.shutdown()
}
}
+
+ test("PipelineEventSender logs a warning when it drops events due to a full
queue") {
Review Comment:
Good idea for the symmetry, but `Long.MaxValue` doesn't work here. The
first-drop-always-logs behavior relies on `lastDropWarningTimestamp` starting
at 0, so the first check is `now - 0 >= interval`. `now` is epoch millis
(~1.7e12), which is itself smaller than `Long.MaxValue`, so that check would be
false and the first drop would be suppressed too, making the count 0 instead of
1. The re-log test can use `Long.MinValue` because the inverse (always re-log)
doesn't hit this.
Suppression here is already structural for any in-process run: the two drops
are submitted back-to-back, microseconds apart, far inside the 60s window (it
could only fail to suppress if 60s elapsed between two adjacent synchronous
sendEvent calls). I kept the default interval and clarified the comment to say
so.
--
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]
