sunchao commented on code in PR #57219:
URL: https://github.com/apache/spark/pull/57219#discussion_r3606799468
##########
sql/connect/server/src/main/scala/org/apache/spark/sql/connect/ui/SparkConnectServerAppStatusStore.scala:
##########
@@ -74,11 +74,16 @@ class SparkConnectServerAppStatusStore(store: KVStore) {
}
private[connect] class SessionInfo(
- @KVIndexParam val sessionId: String,
+ val sessionId: String,
val startTimestamp: Long,
val userId: String,
val finishTimestamp: Long,
val totalExecution: Long) {
+ // Natural key. A session is identified by (userId, sessionId), since two
users may share the
+ // same session UUID; keying on sessionId alone would merge them into one
record.
+ @KVIndexParam
Review Comment:
[P2] Invalidate cached History Server stores when changing the natural key
This changes the physical `@KVIndexParam` from `sessionId` to
`userId/sessionId`, but `AppStatusStore.CURRENT_VERSION` remains `2`. A History
Server configured with `spark.history.store.path` will therefore reopen
pre-upgrade stores without replay: existing `SessionInfo` rows remain keyed by
`sessionId`. `getSessionList` can still enumerate those rows and generate the
new composite links, but `getSession(userId, sessionId)` cannot resolve them;
any same-UUID rows already collapsed in the cache also remain collapsed. The
added history path uses a fresh `InMemoryStore`, so it does not exercise this
upgrade. Please bump the status-store version to force replay, or add an
explicit migration or validated legacy fallback.
##########
sql/connect/server/src/main/scala/org/apache/spark/sql/connect/ui/SparkConnectServerSessionPage.scala:
##########
@@ -38,10 +38,12 @@ private[ui] class SparkConnectServerSessionPage(parent:
SparkConnectServerTab)
def render(request: HttpServletRequest): Seq[Node] = {
val sessionId = request.getParameter("id")
require(sessionId != null && sessionId.nonEmpty, "Missing id parameter")
+ val userId = request.getParameter("userId")
Review Comment:
[P2] Preserve the opaque user ID through the UI request path
`userId` is an opaque Connect identifier, but this exact-key lookup receives
the value after Spark UI's `XssSafeRequest` processing, which strips
apostrophes and HTML-escapes other characters. For example, valid user `a'b` is
looked up as `ab`; if both users share the UUID, this renders the wrong
session, otherwise it reports no information. The downstream `PagedTable`
navigation also reconstructs URLs from decoded parameters without encoding
them, so `alice+tag` becomes `alice tag` after a sort/page link. Since `userId`
is now part of the primary key, please carry a filter-safe token (for example
URL-safe Base64), encode every retained navigation parameter, and add
round-trip coverage through the real HTTP filter for values containing `'`,
`+`, and `=`.
--
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]
