Skip to content

Conversation

@tdas
Copy link
Contributor

@tdas tdas commented Mar 11, 2016

What changes were proposed in this pull request?

In this PR, I am implementing a new abstraction for management of streaming state data - State Store. It is a key-value store for persisting running aggregates for aggregate operations in streaming dataframes. The motivation and design is discussed here.

https://docs.google.com/document/d/1-ncawFx8JS5Zyfq1HAEGBx56RDet9wfVp_hDM8ZL254/edit#

How was this patch tested?

  • Unit tests
  • Cluster tests

Coverage from unit tests

screen shot 2016-03-21 at 3 09 40 pm

## TODO - [x] Fix updates() iterator to avoid duplicate updates for same key - [x] Use Coordinator in ContinuousQueryManager - [x] Plugging in hadoop conf and other confs - [x] Unit tests - [x] StateStore object lifecycle and methods - [x] StateStoreCoordinator communication and logic - [x] StateStoreRDD fault-tolerance - [x] StateStoreRDD preferred location using StateStoreCoordinator - [ ] Cluster tests - [ ] Whether preferred locations are set correctly - [ ] Whether recovery works correctly with distributed storage - [x] Basic performance tests - [x] Docs

@tdas
Copy link
Contributor Author

tdas commented Mar 11, 2016

@marmbrus @rxin @zsxwing

@SparkQA
Copy link

SparkQA commented Mar 11, 2016

Test build #52897 has finished for PR 11645 at commit f417bde.

  • This patch fails Scala style tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • implicit class StateStoreOps[INPUT: ClassTag](dataRDD: RDD[INPUT])

@SparkQA
Copy link

SparkQA commented Mar 11, 2016

Test build #52898 has finished for PR 11645 at commit d8cee54.

  • This patch fails Scala style tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

}
}

private def remove(storeId: StateStoreId): Unit = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we also need synchronized for remove()?

@SparkQA
Copy link

SparkQA commented Mar 12, 2016

Test build #52988 has finished for PR 11645 at commit 7d74c67.

  • This patch fails Scala style tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Mar 14, 2016

Test build #53064 has finished for PR 11645 at commit 7adca70.

  • This patch fails Scala style tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Mar 14, 2016

Test build #53070 has finished for PR 11645 at commit a0ba498.

  • This patch fails MiMa tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • case class ValueUpdated(key: InternalRow, value: InternalRow) extends StoreUpdate
    • case class KeyRemoved(key: InternalRow) extends StoreUpdate

@SparkQA
Copy link

SparkQA commented Mar 14, 2016

Test build #53085 has finished for PR 11645 at commit 48afbe6.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Mar 15, 2016

Test build #53146 has finished for PR 11645 at commit 22d7e66.

  • This patch fails Scala style tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Mar 15, 2016

Test build #53147 has finished for PR 11645 at commit 34ae7ff.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.


override protected def getPartitions: Array[Partition] = dataRDD.partitions
override def getPreferredLocations(partition: Partition): Seq[String] = {
Seq.empty

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should not you be using preffered location here ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. Its still WIP. I need to add enable StateStoreCoordinator for this, which is what I am working on now.

@SparkQA
Copy link

SparkQA commented Mar 15, 2016

Test build #53178 has finished for PR 11645 at commit 13c29a2.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • case class ValueAdded(key: InternalRow, value: InternalRow) extends StoreUpdate
    • case class ValueUpdated(key: InternalRow, value: InternalRow) extends StoreUpdate
    • case class KeyRemoved(key: InternalRow) extends StoreUpdate

* to ensure re-executed RDD operations re-apply updates on the correct past version of the
* store.
*/
class HDFSBackedStateStoreProvider(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note to self: private[state]

@SparkQA
Copy link

SparkQA commented Mar 22, 2016

Test build #53805 has finished for PR 11645 at commit 63fad92.

  • This patch fails Scala style tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Mar 22, 2016

Test build #53794 has finished for PR 11645 at commit 4752d73.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Mar 22, 2016

Test build #53793 has finished for PR 11645 at commit 24fb325.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

maintenanceTask.cancel(false)
maintenanceTask = null
}
logInfo("StateStore stopped")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to call maintenanceTaskExecutor.shutdown before this line. To allow this companion object to be reused, it should not be shutdown.

@SparkQA
Copy link

SparkQA commented Mar 22, 2016

Test build #53804 has finished for PR 11645 at commit 756762a.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.


/**
* Create a reference to a [[StateStoreCoordinator]], This can be called from driver as well as
* executors.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can not be called from executors as creating StateStoreCoordinator will succeed even if there is one StateStoreCoordinator in driver.

@SparkQA
Copy link

SparkQA commented Mar 22, 2016

Test build #53791 has finished for PR 11645 at commit 2f29d9a.

  • This patch fails from timeout after a configured wait of 250m.
  • This patch merges cleanly.
  • This patch adds no public classes.

}
}

private[state] object HDFSBackedStateStoreProvider {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Remove this

@SparkQA
Copy link

SparkQA commented Mar 23, 2016

Test build #53810 has finished for PR 11645 at commit b147f59.

  • This patch fails from timeout after a configured wait of 250m.
  • This patch merges cleanly.
  • This patch adds no public classes.


test("distributed test") {
quietly {
withSpark(new SparkContext(sparkConf.setMaster("local-cluster[2, 1, 1024]"))) { sc =>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: should clone sparkConf

if (keySize == -1) {
eof = true
} else if (keySize < 0) {
throw new Exception(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: IOException

@zsxwing
Copy link
Member

zsxwing commented Mar 23, 2016

Looks good overall. Just some bits

On Tue, Mar 22, 2016 at 5:53 PM Apache Spark QA [email protected]
wrote:

Test build #53857 has started
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/53857/consoleFull

for PR 11645 at commit 70cc7b1
70cc7b1
.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#11645 (comment)

@SparkQA
Copy link

SparkQA commented Mar 23, 2016

Test build #53845 has finished for PR 11645 at commit b147f59.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Mar 23, 2016

Test build #53851 has finished for PR 11645 at commit 819ca17.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Mar 23, 2016

Test build #53850 has finished for PR 11645 at commit 313d8be.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Mar 23, 2016

Test build #53857 has finished for PR 11645 at commit 70cc7b1.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@tdas
Copy link
Contributor Author

tdas commented Mar 23, 2016

I am merging this. Thanks @zsxwing and @marmbrus for the comments.

@asfgit asfgit closed this in 8c82688 Mar 23, 2016
asfgit pushed a commit that referenced this pull request Apr 1, 2016
This PR adds the ability to perform aggregations inside of a `ContinuousQuery`.  In order to implement this feature, the planning of aggregation has augmented with a new `StatefulAggregationStrategy`.  Unlike batch aggregation, stateful-aggregation uses the `StateStore` (introduced in #11645) to persist the results of partial aggregation across different invocations.  The resulting physical plan performs the aggregation using the following progression:
   - Partial Aggregation
   - Shuffle
   - Partial Merge (now there is at most 1 tuple per group)
   - StateStoreRestore (now there is 1 tuple from this batch + optionally one from the previous)
   - Partial Merge (now there is at most 1 tuple per group)
   - StateStoreSave (saves the tuple for the next batch)
   - Complete (output the current result of the aggregation)

The following refactoring was also performed to allow us to plug into existing code:
 - The get/put implementation is taken from #12013
 - The logic for breaking down and de-duping the physical execution of aggregation has been move into a new pattern `PhysicalAggregation`
 - The `AttributeReference` used to identify the result of an `AggregateFunction` as been moved into the `AggregateExpression` container.  This change moves the reference into the same object as the other intermediate references used in aggregation and eliminates the need to pass around a `Map[(AggregateFunction, Boolean), Attribute]`.  Further clean up (using a different aggregation container for logical/physical plans) is deferred to a followup.
 - Some planning logic is moved from the `SessionState` into the `QueryExecution` to make it easier to override in the streaming case.
 - The ability to write a `StreamTest` that checks only the output of the last batch has been added to simulate the future addition of output modes.

Author: Michael Armbrust <[email protected]>

Closes #12048 from marmbrus/statefulAgg.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants