Skip to content

WIP - [SPARK-10816][SS] Support session window natively #22482

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

Closed
wants to merge 60 commits into from

Conversation

HeartSaVioR
Copy link
Contributor

What changes were proposed in this pull request?

This patch proposes native support of session window, like Spark has been supporting for time window.

Please refer the attached doc in SPARK-10816 for more details on rationalization, concepts, and limitation, etc.

In point of end users' view, only the change is addition of "session" SQL function. End users could define query with session window as replacing "window" function to "session" function, and "window" column to "session" column. After then the patch will provide same experience with time window.

Internally, this patch will change the physical plan of aggregation a bit: if there's session function being used in query, it will sort the input rows as "grouping keys" + "session", and merge overlapped sessions into one with applying aggregations, so it's like a sort based aggregation but the unit of group is grouping keys + session.

Due to handle late event, there's a case multiple session windows co-exist per key which are not yet to evict. This patch handles the case via borrowing state implementation from streaming join which can handle multiple values for given key.

How was this patch tested?

Many UTs are added to verify session window queries for both batch and streaming.

Please review http://spark.apache.org/contributing.html before opening a pull request.

@SparkQA
Copy link

SparkQA commented Sep 20, 2018

Test build #96321 has finished for PR 22482 at commit fb19879.

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

@HeartSaVioR
Copy link
Contributor Author

HeartSaVioR commented Sep 20, 2018

The patch is a bit huge, so I'm not sure we would be better to squash commits into one before reviewing.

Two TODOs are left hence marking the patch as WIP, but it's closer to be a complete patch:

  1. Optimal implementation of state for session window.

It borrowed the state implementation from streaming join since it fits the necessary concept of state for session window, but it may not be optimal one so I'm going to see we can have better implementation.

  1. Javadoc (Maybe structured streaming guide doc too?)

I didn't add javadoc yet to speed up POC and actual development, but to complete the patch I guess I need to write javadoc for new classes as well as methods (maybe).

@SparkQA
Copy link

SparkQA commented Sep 20, 2018

Test build #96323 has finished for PR 22482 at commit 0072ebe.

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

@SparkQA
Copy link

SparkQA commented Sep 20, 2018

Test build #96324 has finished for PR 22482 at commit 7d8371c.

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

@SparkQA
Copy link

SparkQA commented Sep 20, 2018

Test build #96328 has finished for PR 22482 at commit ad0b746.

  • This patch fails due to an unknown error code, -9.
  • This patch merges cleanly.
  • This patch adds no public classes.

@HeartSaVioR
Copy link
Contributor Author

retest this, please

@SparkQA
Copy link

SparkQA commented Sep 20, 2018

Test build #96336 has finished for PR 22482 at commit ad0b746.

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

@HeartSaVioR
Copy link
Contributor Author

retest this, please

@HeartSaVioR
Copy link
Contributor Author

Please review the general approach and direction first. I'm planning to spend time to rewrite streaming part to tightly integrate logic with state so that updating state is going to be minimized.

@SparkQA
Copy link

SparkQA commented Sep 20, 2018

Test build #96348 has finished for PR 22482 at commit ad0b746.

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

@arunmahadevan
Copy link
Contributor

+1 for the idea to provide native session window support.

On the approach, it would be ideal if all windowing aggregations can be handled via single plan and state store (v/s the separate plan and state store the patch proposes for session window). Underlying steps are more or less the same for Fixed, Session and Sliding windows. The sort/merge operations have to be part of a window merge function rather than the plan itself.

K,Values -> AssignWindows (produces [k, v, timestamp, window]) -> GroupByKey (shuffle) -> MergeWindows (optional step) -> GroupWindows -> aggregate values.

Based on how we want to approach it, it could be handled now or as a follow up item (with major refactoring).

@HeartSaVioR
Copy link
Contributor Author

HeartSaVioR commented Sep 20, 2018

@arunmahadevan
We may want to be aware is that the requirement is pretty different from other streaming frameworks like Flink, which normally set a long period of checkpoint interval and do a full snapshot (though it supports incremental checkpoint, which deals with how it minimizes amount of storing data).

Here in Spark, we are expecting smaller batch interval, and Spark deals with the requirement as storing "delta" of state change. The behavior brings concern about the strategy of how we store and how we remove the state.

Let's say we have 3 rows in group in batch result and there're also 3 rows in same group in state, and we want to replace state with new batch result. For full snapshot removing 3 rows first and putting 3 rows may not matter much, but with delta approach, we should compare them side-by-side and bring less changes on state. (We also avoid having List[V] as value for state and have two different states because of that. If you make a change of any element on List, Spark's state store will treat it as whole change of List[V] and store whole elements to delta.)

The difference is not trivial one for session window, because arbitrary changes are required: for example, two different sessions in state can be merged later when late events come in between two sessions, then we ideally should have to overwrite one and remove others. Some new sessions can be created as well as existing session, and we want to overwrite session if the new output session is originated from old state, and append session if not. For other window, it is just a "put" because there's no group and we are just safe to put (overwrite if any, and without evict there's no need to remove). The different requirements between time window and session window are hard to be combined into one.

That's what I realized the difficulty of state part for session window, and that's why I feel I need to make change on streaming part. (Not sure I'm too worry about optimization of the state change, but minimizing delta was the core concept of #21733 and it really affects the performance.) For batch part current patch is doing OK.

Btw, we can assume AssignWindows as TimeWindowing and SessionWindowing as we are logically assign rows to individual window. So unless we would like to support custom window like dynamic gap session window, I think we can address it later whenever needed.

@HeartSaVioR
Copy link
Contributor Author

If we are fine with ignoring the optimal delta of state, or OK with addressing it in follow-up issue (it should be addressed in same release version to avoid having state V1, V2, etc...), I think the only TODO is writing javadoc as well as deduplicate some codes.

@HeartSaVioR
Copy link
Contributor Author

According to the discussion on SPARK-10816, I'm holding up effort to improve and plan to discuss further from JIRA issue. I guess someone interested for this patch can still review or try this out and share feedback.

@SparkQA
Copy link

SparkQA commented Oct 2, 2018

Test build #96838 has finished for PR 22482 at commit 9a60cf3.

  • This patch fails due to an unknown error code, -9.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Oct 8, 2018

Test build #97107 has finished for PR 22482 at commit 78fdd99.

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

@SparkQA
Copy link

SparkQA commented Oct 9, 2018

Test build #97135 has finished for PR 22482 at commit 94e9859.

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

* This will be also used from session window state as well
We can enable it but there're lots of approaches on aggregations in batch side...

* AggUtils.planAggregateWithoutDistinct
* AggUtils.planAggregateWithOneDistinct
* RewriteDistinctAggregates
* AggregateInPandasExec

So unless we are sure which things to support, just block them for now...
… node

* we will leverage such node for batch case if we want
@SparkQA
Copy link

SparkQA commented Oct 31, 2018

Test build #98289 has finished for PR 22482 at commit c03c946.

  • This patch fails due to an unknown error code, -9.
  • This patch merges cleanly.
  • This patch adds no public classes.

…w for group key

Also modify CodeGenerator to print out debug information when code generation takes too long
@SparkQA
Copy link

SparkQA commented Nov 1, 2018

Test build #98347 has finished for PR 22482 at commit 5de4075.

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

@SparkQA
Copy link

SparkQA commented Nov 1, 2018

Test build #98348 has finished for PR 22482 at commit d1536b4.

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

@HeartSaVioR
Copy link
Contributor Author

retest this, please

@SparkQA
Copy link

SparkQA commented Nov 2, 2018

Test build #98382 has finished for PR 22482 at commit 5a76383.

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

@SparkQA
Copy link

SparkQA commented Nov 2, 2018

Test build #98380 has finished for PR 22482 at commit ee67bca.

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

@SparkQA
Copy link

SparkQA commented Nov 2, 2018

Test build #98379 has finished for PR 22482 at commit ee67bca.

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

@SparkQA
Copy link

SparkQA commented Nov 2, 2018

Test build #98383 has finished for PR 22482 at commit b6ccecd.

  • This patch fails to build.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Nov 2, 2018

Test build #98385 has finished for PR 22482 at commit 75c7611.

  • This patch fails due to an unknown error code, -9.
  • This patch merges cleanly.
  • This patch adds no public classes.

@HeartSaVioR
Copy link
Contributor Author

retest this, please

@SparkQA
Copy link

SparkQA commented Nov 2, 2018

Test build #98387 has finished for PR 22482 at commit 75c7611.

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

@AmplabJenkins
Copy link

Can one of the admins verify this patch?

@github-actions
Copy link

github-actions bot commented Jan 6, 2020

We're closing this PR because it hasn't been updated in a while.
This isn't a judgement on the merit of the PR in any way. It's just
a way of keeping the PR queue manageable.

If you'd like to revive this PR, please reopen it!

@github-actions github-actions bot added the Stale label Jan 6, 2020
@github-actions github-actions bot closed this Jan 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants