viirya commented on a change in pull request #26935:
URL: https://github.com/apache/spark/pull/26935#discussion_r491744046



##########
File path: 
sql/core/src/main/scala/org/apache/spark/sql/execution/streaming/state/StateStore.scala
##########
@@ -81,6 +74,42 @@ trait StateStore {
     iterator()
   }
 
+  /**
+   * Return an iterator containing all the key-value pairs in the StateStore. 
Implementations must
+   * ensure that updates (puts, removes) can be made while iterating over this 
iterator.
+   */
+  def iterator(): Iterator[UnsafeRowPair]
+
+  /**
+   * Clean up the resource.
+   *
+   * The method name is to respect backward compatibility on [[StateStore]].
+   */
+  def abort(): Unit
+}
+
+/**
+ * Base trait for a versioned key-value store. Each instance of a `StateStore` 
represents a specific
+ * version of state data, and such instances are created through a 
[[StateStoreProvider]].
+ *
+ * Unlike [[ReadOnlyStateStore]], `abort` method may not be called if the 
`commit` method succeeds
+ * to commit the change. (`hasCommitted` returns `true`.) Otherwise, `abort` 
method will be called.
+ * Implementation should deal with resource cleanup in both methods, but also 
need to guard with
+ * double resource cleanup.
+ */
+trait StateStore extends ReadOnlyStateStore {

Review comment:
       Is this what we want? So all `StateStore` are `ReadOnlyStateStore`?

##########
File path: 
sql/core/src/main/scala/org/apache/spark/sql/execution/streaming/state/StateStore.scala
##########
@@ -81,6 +74,42 @@ trait StateStore {
     iterator()
   }
 
+  /**
+   * Return an iterator containing all the key-value pairs in the StateStore. 
Implementations must
+   * ensure that updates (puts, removes) can be made while iterating over this 
iterator.
+   */
+  def iterator(): Iterator[UnsafeRowPair]
+
+  /**
+   * Clean up the resource.
+   *
+   * The method name is to respect backward compatibility on [[StateStore]].
+   */
+  def abort(): Unit
+}
+
+/**
+ * Base trait for a versioned key-value store. Each instance of a `StateStore` 
represents a specific
+ * version of state data, and such instances are created through a 
[[StateStoreProvider]].
+ *
+ * Unlike [[ReadOnlyStateStore]], `abort` method may not be called if the 
`commit` method succeeds
+ * to commit the change. (`hasCommitted` returns `true`.) Otherwise, `abort` 
method will be called.

Review comment:
       It seems to me that `abort` has no clear semantics across `StateStore` 
and `ReadOnlyStateStore`.

##########
File path: 
sql/core/src/main/scala/org/apache/spark/sql/execution/streaming/state/StateStoreRDD.scala
##########
@@ -29,6 +29,57 @@ import org.apache.spark.sql.internal.SessionState
 import org.apache.spark.sql.types.StructType
 import org.apache.spark.util.SerializableConfiguration
 
+/**
+ * An RDD that allows computations to be executed against 
[[ReadOnlyStateStore]]s. It
+ * uses the [[StateStoreCoordinator]] to get the locations of loaded state 
stores
+ * and use that as the preferred locations.
+ */
+class ReadOnlyStateStoreRDD[T: ClassTag, U: ClassTag](
+    dataRDD: RDD[T],
+    storeReadFunction: (ReadOnlyStateStore, Iterator[T]) => Iterator[U],
+    checkpointLocation: String,
+    queryRunId: UUID,
+    operatorId: Long,
+    storeVersion: Long,
+    keySchema: StructType,
+    valueSchema: StructType,
+    indexOrdinal: Option[Int],
+    sessionState: SessionState,
+    @transient private val storeCoordinator: Option[StateStoreCoordinatorRef],
+    extraOptions: Map[String, String] = Map.empty) extends RDD[U](dataRDD) {
+
+  private val storeConf = new StateStoreConf(sessionState.conf, extraOptions)
+
+  // A Hadoop Configuration can be about 10 KB, which is pretty big, so 
broadcast it
+  private val hadoopConfBroadcast = dataRDD.context.broadcast(
+    new SerializableConfiguration(sessionState.newHadoopConf()))
+
+  override protected def getPartitions: Array[Partition] = dataRDD.partitions
+
+  /**
+   * Set the preferred location of each partition using the executor that has 
the related
+   * [[StateStoreProvider]] already loaded.
+   */
+  override def getPreferredLocations(partition: Partition): Seq[String] = {
+    val stateStoreProviderId = StateStoreProviderId(
+      StateStoreId(checkpointLocation, operatorId, partition.index),
+      queryRunId)
+    storeCoordinator.flatMap(_.getLocation(stateStoreProviderId)).toSeq
+  }
+
+  override def compute(partition: Partition, ctxt: TaskContext): Iterator[U] = 
{
+    val storeProviderId = StateStoreProviderId(
+      StateStoreId(checkpointLocation, operatorId, partition.index),
+      queryRunId)
+
+    val store = StateStore.getReadOnly(
+      storeProviderId, keySchema, valueSchema, indexOrdinal, storeVersion,

Review comment:
       Is this only difference from `StateStoreRDD`? Looks they are almost 
duplicate.




----------------------------------------------------------------
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]

Reply via email to