-
Notifications
You must be signed in to change notification settings - Fork 28k
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
[SPARK-47784][SS] Merge TTLMode and TimeoutMode into a single TimeMode. #45960
Conversation
32816ea
to
1e819de
Compare
1e819de
to
1189993
Compare
@HeartSaVioR @anishshri-db PTAL. |
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.
LGTM - pending minor nits
11ab935
to
2c57f1e
Compare
@HeartSaVioR PTAL. |
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.
Only nits. Looks great in overall.
sql/api/src/main/java/org/apache/spark/sql/streaming/TimeMode.java
Outdated
Show resolved
Hide resolved
sql/api/src/main/java/org/apache/spark/sql/streaming/TimeMode.java
Outdated
Show resolved
Hide resolved
sql/core/src/main/scala/org/apache/spark/sql/execution/streaming/TimerStateImpl.scala
Outdated
Show resolved
Hide resolved
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.
+1 pending CI
The CI failure happened in known flakiness - SparkSessionE2ESuite. |
Thanks! Merging to master. |
What changes were proposed in this pull request?
This PR merges the
TimeoutMode
andTTLMode
parameter fortransformWithState
into a singleTimeMode
. Currently, users need to specify the notion of time (ProcessingTime/EventTime) for timers and ttl separately. This allows users to use a single parameter.We do not expect users to use mix/match EventTime/ProcessingTime for timers and ttl in a single query because it makes hard to reason about the time semantics (when will timer be fired?, when will the state be evicted? etc.). Its simpler to stick to one notion of time throughout timers and ttl.
Why are the changes needed?
Changes are needed to simplify Arbitrary State API
transformWithState
interface by merging TTLMode/TimeoutMode into a single TimeMode.Does this PR introduce any user-facing change?
Yes, this PR changes the API parameters for
transformWithState
.How was this patch tested?
All existing testcases for
transformWithState
API pass.Was this patch authored or co-authored using generative AI tooling?
No