HeartSaVioR commented on a change in pull request #28904:
URL: https://github.com/apache/spark/pull/28904#discussion_r456729588
##########
File path:
sql/core/src/main/scala/org/apache/spark/sql/execution/streaming/CompactibleFileStreamLog.scala
##########
@@ -106,10 +106,8 @@ abstract class CompactibleFileStreamLog[T <: AnyRef :
ClassTag](
interval
}
- /**
- * Filter out the obsolete logs.
- */
- def compactLogs(logs: Seq[T]): Seq[T]
+ /** Determine whether the log should be retained or not. */
+ def shouldRetain(log: T): Boolean
Review comment:
Is there a specific reason to avoid changing the method which is private
API? I'd say it'd be less clear to provide the intention on downstream
(technically we shouldn't concern with downstream though) - if you don't change
the method signature but just change the semantic, no one would know about the
change and it will be no-op for downstream.
I'll consider it if you have strong reason to do so, otherwise let's hear
other's voice as well. cc. @zsxwing
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
