-
Notifications
You must be signed in to change notification settings - Fork 28.9k
[SPARK-9065][Streaming][PySpark] Add MessageHandler for Kafka Python API #9742
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
|
Test build #46004 has finished for PR 9742 at commit
|
|
@jerryshao After discussing with @tdas offline, I added some codes to speed up the case when the user doesn't provide a messageHandler. So that we can avoid message serialization for this case and won't reduce the performance. Could you take a look? |
|
OK, let me check. But I'm wondering if there's a huge overhead for message serialization, since we only pack some of metadatas into message, don't do serialization-deserialization on the real message itself, not sure what's actual performance loss will happen. |
|
LGTM. |
|
Test build #46033 has finished for PR 9742 at commit
|
|
retest this please |
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.
nit: incorrect indent.
python/pyspark/streaming/kafka.py
Outdated
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.
Remove this default_message_handler
|
Test build #46045 has finished for PR 9742 at commit
|
|
Test build #2068 has finished for PR 9742 at commit
|
|
Test build #46040 has finished for PR 9742 at commit
|
|
retest this please |
|
Test build #46060 has finished for PR 9742 at commit
|
|
retest this please |
|
Test build #46097 has finished for PR 9742 at commit
|
|
Test build #46115 has finished for PR 9742 at commit
|
|
All right. I am merging to master and 1.6. Thanks @jerryshao for all the effort, and @zsxwing for getting this ready. |
Fixed the merge conflicts in #7410 Closes #7410 Author: Shixiong Zhu <[email protected]> Author: jerryshao <[email protected]> Author: jerryshao <[email protected]> Closes #9742 from zsxwing/pr7410. (cherry picked from commit 75a2922) Signed-off-by: Tathagata Das <[email protected]>
Fixed the merge conflicts in #7410
Closes #7410