HeartSaVioR commented on a change in pull request #30336:
URL: https://github.com/apache/spark/pull/30336#discussion_r526572871
##########
File path:
sql/core/src/main/scala/org/apache/spark/sql/execution/streaming/state/StateStore.scala
##########
@@ -178,11 +178,15 @@ object StateStoreMetrics {
trait StateStoreCustomMetric {
def name: String
def desc: String
+ def unit: String
}
-case class StateStoreCustomSumMetric(name: String, desc: String) extends
StateStoreCustomMetric
-case class StateStoreCustomSizeMetric(name: String, desc: String) extends
StateStoreCustomMetric
-case class StateStoreCustomTimingMetric(name: String, desc: String) extends
StateStoreCustomMetric
+case class StateStoreCustomSumMetric(name: String, desc: String, unit: String)
Review comment:
Nice finding! Glad I wait for 2 days to figure out any missing like this.
Btw, I have a deeper question. Why the state store (+provider) interfaces
are in sql/execution which is considered as "private"? We know there're several
3rd party implementations, and placing these APIs in sql/execution package
means that we don't guarantee the compatibility as public API. e.g. MiMa didn't
complain this.
What would be the way to go? Could we move the interface with aliasing the
old one, and deprecate the old one? Otherwise, should we make a breaking change
to get better result on future?
----------------------------------------------------------------
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:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
