-
Notifications
You must be signed in to change notification settings - Fork 28.7k
[SPARK-7767] [STREAMING] Added test for checkpoint serialization in StreamingContext.start() #6292
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
Conversation
Merged build triggered. |
@zsxwing Can you take a look? |
Merged build started. |
Test build #33167 has started for PR 6292 at commit |
} | ||
|
||
// Test whether start() fails early when checkpointing is enabled | ||
intercept[IllegalArgumentException] { |
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.
Because IllegalArgumentException
is a common exception, it's better to check the exception message so that if other place throws IllegalArgumentException
, this test will fail.
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.
Its unlikely to that start() will throw any other illegal argument, but I agree that this may change in the future. Best to add the message check.
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.
Also I am thinking its probably not the best idea to throw IllegalArgumentException, as there isnt a specific argument here. And other places like DStream.map and RDD.map throw NotSerializableException. So I am changing it to NotSerializableException.
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 for NotSerializableException
LGTM (since you are already changing the exception type) |
Merged build triggered. |
Merged build started. |
LGTM |
Test build #33174 has started for PR 6292 at commit |
Test build #33167 has finished for PR 6292 at commit
|
Merged build finished. Test PASSed. |
Test PASSed. |
Test build #33174 has finished for PR 6292 at commit
|
Merged build finished. Test PASSed. |
Test PASSed. |
All right, thanks all for reviewing. Merging this. |
…treamingContext.start() Currently, the background checkpointing thread fails silently if the checkpoint is not serializable. It is hard to debug and therefore its best to fail fast at `start()` when checkpointing is enabled and the checkpoint is not serializable. Author: Tathagata Das <[email protected]> Closes #6292 from tdas/SPARK-7767 and squashes the following commits: 51304e6 [Tathagata Das] Addressed comments. c35237b [Tathagata Das] Added test for checkpoint serialization in StreamingContext.start() (cherry picked from commit 3c434cb) Signed-off-by: Tathagata Das <[email protected]>
…treamingContext.start() Currently, the background checkpointing thread fails silently if the checkpoint is not serializable. It is hard to debug and therefore its best to fail fast at `start()` when checkpointing is enabled and the checkpoint is not serializable. Author: Tathagata Das <[email protected]> Closes apache#6292 from tdas/SPARK-7767 and squashes the following commits: 51304e6 [Tathagata Das] Addressed comments. c35237b [Tathagata Das] Added test for checkpoint serialization in StreamingContext.start()
…treamingContext.start() Currently, the background checkpointing thread fails silently if the checkpoint is not serializable. It is hard to debug and therefore its best to fail fast at `start()` when checkpointing is enabled and the checkpoint is not serializable. Author: Tathagata Das <[email protected]> Closes apache#6292 from tdas/SPARK-7767 and squashes the following commits: 51304e6 [Tathagata Das] Addressed comments. c35237b [Tathagata Das] Added test for checkpoint serialization in StreamingContext.start()
…treamingContext.start() Currently, the background checkpointing thread fails silently if the checkpoint is not serializable. It is hard to debug and therefore its best to fail fast at `start()` when checkpointing is enabled and the checkpoint is not serializable. Author: Tathagata Das <[email protected]> Closes apache#6292 from tdas/SPARK-7767 and squashes the following commits: 51304e6 [Tathagata Das] Addressed comments. c35237b [Tathagata Das] Added test for checkpoint serialization in StreamingContext.start()
Currently, the background checkpointing thread fails silently if the checkpoint is not serializable. It is hard to debug and therefore its best to fail fast at
start()
when checkpointing is enabled and the checkpoint is not serializable.