-
Notifications
You must be signed in to change notification settings - Fork 29.3k
[SPARK-24525][SS] Provide an option to limit number of rows in a MemorySink #21559
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 12 commits
ac7eb2f
8dc89cc
8ddf566
d82c7d5
7fefe87
58c5044
392f05f
9097dd5
a28fb38
f981cb8
74d5b6b
4ab9bda
b2ef59c
25d6de1
e5b6175
0402b60
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -33,6 +33,7 @@ import org.apache.spark.sql.catalyst.expressions.{Attribute, UnsafeRow} | |
| import org.apache.spark.sql.catalyst.plans.logical.{LeafNode, LogicalPlan, Statistics} | ||
| import org.apache.spark.sql.catalyst.plans.logical.statsEstimation.EstimationUtils | ||
| import org.apache.spark.sql.catalyst.streaming.InternalOutputModes._ | ||
| import org.apache.spark.sql.sources.v2.DataSourceOptions | ||
| import org.apache.spark.sql.sources.v2.reader.{InputPartition, InputPartitionReader, SupportsScanUnsafeRow} | ||
| import org.apache.spark.sql.sources.v2.reader.streaming.{MicroBatchReader, Offset => OffsetV2} | ||
| import org.apache.spark.sql.streaming.OutputMode | ||
|
|
@@ -228,19 +229,45 @@ trait MemorySinkBase extends BaseStreamingSink { | |
| def latestBatchId: Option[Long] | ||
| } | ||
|
|
||
| /** | ||
| * Companion object to MemorySinkBase. | ||
| */ | ||
| object MemorySinkBase { | ||
| val MAX_MEMORY_SINK_ROWS = "maxMemorySinkRows" | ||
| val MAX_MEMORY_SINK_ROWS_DEFAULT = -1 | ||
|
|
||
| /** | ||
| * Gets the max number of rows a MemorySink should store. This number is based on the memory | ||
| * sink row limit if it is set. If not, there is no limit. | ||
| * @param options Options for writing from which we get the max rows option | ||
| * @return The maximum number of rows a memorySink should store, or None for no limit. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. need to update docs |
||
| */ | ||
| def getMemorySinkCapacity(options: DataSourceOptions): Option[Int] = { | ||
| val maxRows = options.getInt(MAX_MEMORY_SINK_ROWS, MAX_MEMORY_SINK_ROWS_DEFAULT) | ||
| if (maxRows >= 0) Some(maxRows) else None | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you want to do |
||
| } | ||
| } | ||
|
|
||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: remove extra line |
||
| /** | ||
| * A sink that stores the results in memory. This [[Sink]] is primarily intended for use in unit | ||
| * tests and does not provide durability. | ||
| */ | ||
| class MemorySink(val schema: StructType, outputMode: OutputMode) extends Sink | ||
| with MemorySinkBase with Logging { | ||
| class MemorySink(val schema: StructType, outputMode: OutputMode, options: DataSourceOptions) | ||
| extends Sink with MemorySinkBase with Logging { | ||
|
|
||
| private case class AddedData(batchId: Long, data: Array[Row]) | ||
|
|
||
| /** An order list of batches that have been written to this [[Sink]]. */ | ||
| @GuardedBy("this") | ||
| private val batches = new ArrayBuffer[AddedData]() | ||
|
|
||
| /** The number of rows in this MemorySink. */ | ||
| private var numRows = 0 | ||
|
|
||
| /** The capacity in rows of this sink. */ | ||
| val sinkCapacity: Option[Int] = MemorySinkBase.getMemorySinkCapacity(options) | ||
|
|
||
| /** Returns all rows that are stored in this [[Sink]]. */ | ||
| def allData: Seq[Row] = synchronized { | ||
| batches.flatMap(_.data) | ||
|
|
@@ -273,14 +300,26 @@ class MemorySink(val schema: StructType, outputMode: OutputMode) extends Sink | |
| logDebug(s"Committing batch $batchId to $this") | ||
| outputMode match { | ||
| case Append | Update => | ||
| val rows = AddedData(batchId, data.collect()) | ||
| synchronized { batches += rows } | ||
| var rowsToAdd = data.collect() | ||
| synchronized { | ||
| if (sinkCapacity.isDefined) { | ||
| rowsToAdd = truncateRowsIfNeeded(rowsToAdd, sinkCapacity.get - numRows, batchId) | ||
| } | ||
| val rows = AddedData(batchId, rowsToAdd) | ||
| batches += rows | ||
| numRows += rowsToAdd.length | ||
| } | ||
|
|
||
| case Complete => | ||
| val rows = AddedData(batchId, data.collect()) | ||
| var rowsToAdd = data.collect() | ||
| synchronized { | ||
| if (sinkCapacity.isDefined) { | ||
| rowsToAdd = truncateRowsIfNeeded(rowsToAdd, sinkCapacity.get, batchId) | ||
| } | ||
| val rows = AddedData(batchId, rowsToAdd) | ||
| batches.clear() | ||
| batches += rows | ||
| numRows = rowsToAdd.length | ||
| } | ||
|
|
||
| case _ => | ||
|
|
@@ -294,6 +333,16 @@ class MemorySink(val schema: StructType, outputMode: OutputMode) extends Sink | |
|
|
||
| def clear(): Unit = synchronized { | ||
| batches.clear() | ||
| numRows = 0 | ||
| } | ||
|
|
||
| private def truncateRowsIfNeeded(rows: Array[Row], maxRows: Int, batchId: Long): Array[Row] = { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: I'd document that maxRows is the remaining row capacity, not the maximum row limit defined in the options. I got confused for a minute here. |
||
| if (rows.length > maxRows) { | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also adding a check here to make sure maxRows >= 0. It shouldn't ever be negative, but doesn't hurt to safeguard. |
||
| logWarning(s"Truncating batch $batchId to $maxRows rows") | ||
| rows.take(maxRows) | ||
| } else { | ||
| rows | ||
| } | ||
| } | ||
|
|
||
| override def toString(): String = "MemorySink" | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -46,7 +46,7 @@ class MemorySinkV2 extends DataSourceV2 with StreamWriteSupport with MemorySinkB | |
| schema: StructType, | ||
| mode: OutputMode, | ||
| options: DataSourceOptions): StreamWriter = { | ||
| new MemoryStreamWriter(this, mode) | ||
| new MemoryStreamWriter(this, mode, options) | ||
| } | ||
|
|
||
| private case class AddedData(batchId: Long, data: Array[Row]) | ||
|
|
@@ -55,6 +55,9 @@ class MemorySinkV2 extends DataSourceV2 with StreamWriteSupport with MemorySinkB | |
| @GuardedBy("this") | ||
| private val batches = new ArrayBuffer[AddedData]() | ||
|
|
||
| /** The number of rows in this MemorySink. */ | ||
| private var numRows = 0 | ||
|
|
||
| /** Returns all rows that are stored in this [[Sink]]. */ | ||
| def allData: Seq[Row] = synchronized { | ||
| batches.flatMap(_.data) | ||
|
|
@@ -81,22 +84,35 @@ class MemorySinkV2 extends DataSourceV2 with StreamWriteSupport with MemorySinkB | |
| }.mkString("\n") | ||
| } | ||
|
|
||
| def write(batchId: Long, outputMode: OutputMode, newRows: Array[Row]): Unit = { | ||
| def write(batchId: Long, outputMode: OutputMode, newRows: Array[Row], sinkCapacity: Option[Int]) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: our style is more like def write(
batchId: Long,
outputMode: OutputMode,
newRows: Array[Row],
sinkCapacity: Option[Int]): Unit = { |
||
| : Unit = { | ||
| val notCommitted = synchronized { | ||
| latestBatchId.isEmpty || batchId > latestBatchId.get | ||
| } | ||
| if (notCommitted) { | ||
| logDebug(s"Committing batch $batchId to $this") | ||
| outputMode match { | ||
| case Append | Update => | ||
| val rows = AddedData(batchId, newRows) | ||
| synchronized { batches += rows } | ||
| synchronized { | ||
| var rowsToAdd = newRows | ||
| if (sinkCapacity.isDefined) { | ||
| rowsToAdd = truncateRowsIfNeeded(rowsToAdd, sinkCapacity.get - numRows, batchId) | ||
| } | ||
| val rows = AddedData(batchId, rowsToAdd) | ||
| batches += rows | ||
| numRows += rowsToAdd.length | ||
| } | ||
|
|
||
| case Complete => | ||
| val rows = AddedData(batchId, newRows) | ||
| synchronized { | ||
| var rowsToAdd = newRows | ||
| if (sinkCapacity.isDefined) { | ||
| rowsToAdd = truncateRowsIfNeeded(rowsToAdd, sinkCapacity.get, batchId) | ||
| } | ||
| val rows = AddedData(batchId, rowsToAdd) | ||
| batches.clear() | ||
| batches += rows | ||
| numRows = rowsToAdd.length | ||
| } | ||
|
|
||
| case _ => | ||
|
|
@@ -110,40 +126,61 @@ class MemorySinkV2 extends DataSourceV2 with StreamWriteSupport with MemorySinkB | |
|
|
||
| def clear(): Unit = synchronized { | ||
| batches.clear() | ||
| numRows = 0 | ||
| } | ||
|
|
||
| private def truncateRowsIfNeeded(rows: Array[Row], maxRows: Int, batchId: Long): Array[Row] = { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can this go in MemorySinkBase? |
||
| if (rows.length > maxRows) { | ||
| logWarning(s"Truncating batch $batchId to $maxRows rows") | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How does take behave with negative rows? Printing a warning message with negative values may be weird. I would also include the sink limit in the warning. |
||
| rows.take(maxRows) | ||
| } else { | ||
| rows | ||
| } | ||
| } | ||
|
|
||
| override def toString(): String = "MemorySinkV2" | ||
| } | ||
|
|
||
| case class MemoryWriterCommitMessage(partition: Int, data: Seq[Row]) extends WriterCommitMessage {} | ||
|
|
||
| class MemoryWriter(sink: MemorySinkV2, batchId: Long, outputMode: OutputMode) | ||
| class MemoryWriter( | ||
| sink: MemorySinkV2, | ||
| batchId: Long, | ||
| outputMode: OutputMode, | ||
| options: DataSourceOptions) | ||
| extends DataSourceWriter with Logging { | ||
|
|
||
| val sinkCapacity: Option[Int] = MemorySinkBase.getMemorySinkCapacity(options) | ||
|
|
||
| override def createWriterFactory: MemoryWriterFactory = MemoryWriterFactory(outputMode) | ||
|
|
||
| def commit(messages: Array[WriterCommitMessage]): Unit = { | ||
| val newRows = messages.flatMap { | ||
| case message: MemoryWriterCommitMessage => message.data | ||
| } | ||
| sink.write(batchId, outputMode, newRows) | ||
| sink.write(batchId, outputMode, newRows, sinkCapacity) | ||
| } | ||
|
|
||
| override def abort(messages: Array[WriterCommitMessage]): Unit = { | ||
| // Don't accept any of the new input. | ||
| } | ||
| } | ||
|
|
||
| class MemoryStreamWriter(val sink: MemorySinkV2, outputMode: OutputMode) | ||
| class MemoryStreamWriter( | ||
| val sink: MemorySinkV2, | ||
| outputMode: OutputMode, | ||
| options: DataSourceOptions) | ||
| extends StreamWriter { | ||
|
|
||
| val sinkCapacity: Option[Int] = MemorySinkBase.getMemorySinkCapacity(options) | ||
|
|
||
| override def createWriterFactory: MemoryWriterFactory = MemoryWriterFactory(outputMode) | ||
|
|
||
| override def commit(epochId: Long, messages: Array[WriterCommitMessage]): Unit = { | ||
| val newRows = messages.flatMap { | ||
| case message: MemoryWriterCommitMessage => message.data | ||
| } | ||
| sink.write(epochId, outputMode, newRows) | ||
| sink.write(epochId, outputMode, newRows, sinkCapacity) | ||
| } | ||
|
|
||
| override def abort(epochId: Long, messages: Array[WriterCommitMessage]): Unit = { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maxRowsis sufficient