micheal-o commented on code in PR #51484:
URL: https://github.com/apache/spark/pull/51484#discussion_r2208971837
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/streaming/state/RocksDBStateStoreProvider.scala:
##########
@@ -446,6 +449,8 @@ private[sql] class RocksDBStateStoreProvider
override def stateStoreId: StateStoreId = stateStoreId_
+ override protected def logName: String = s"${super.logName}
${stateStoreProviderId}"
Review Comment:
The provider id includes store id, which includes checkpoint location. Do we
really need the chkpt location in every log line? That would cause noise in
each log line, since the location path/length depends on what the user sets. I
think just opid, partid, storeName, runId should be fine.
@anishshri-db what do you think?
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/streaming/state/RocksDBStateStoreProvider.scala:
##########
@@ -547,9 +553,10 @@ private[sql] class RocksDBStateStoreProvider
val dfsRootDir = stateStoreId.storeCheckpointLocation().toString
val storeIdStr = s"StateStoreId(opId=${stateStoreId.operatorId}," +
Review Comment:
nit: remove unused `storeIdStr`?
--
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]
