You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by yashs360 <gi...@git.apache.org> on 2018/06/12 17:38:49 UTC
[GitHub] spark pull request #21541: [SPARK-20168][Streaming Kinesis] Setting the time...
GitHub user yashs360 opened a pull request:
https://github.com/apache/spark/pull/21541
[SPARK-20168][Streaming Kinesis] Setting the timestamp directly would cause exception on …
Setting the timestamp directly would cause exception on reading stream, it can be set directly only if the mode is not AT_TIMESTAMP
## What changes were proposed in this pull request?
The last patch in the kinesis streaming receiver sets the timestamp for the mode AT_TIMESTAMP, but this mode can only be set via the
`baseClientLibConfiguration.withTimestampAtInitialPositionInStream()
`
and can't be set directly using
`.withInitialPositionInStream()`
This patch fixes the issue.
## How was this patch tested?
Kinesis Receiver doesn't expose the internal state outside, so couldn't find the right way to test this change. Seeking for tips from other contributors here.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/yashs360/spark ysharma/fix_kinesis_bug
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/21541.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #21541
----
commit 133ba8721319efa9bfa64fbb53a690b5140c957b
Author: Yash Sharma <ys...@...>
Date: 2018-06-12T17:33:51Z
SPARK-20168: Setting the timestamp directly would cause exception on reading stream, it can be set directly only if the mode is not AT_TIMESTAMP
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21541: [SPARK-20168][Streaming Kinesis] Setting the timestamp d...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21541
**[Test build #4209 has started](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/4209/testReport)** for PR 21541 at commit [`133ba87`](https://github.com/apache/spark/commit/133ba8721319efa9bfa64fbb53a690b5140c957b).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21541: [SPARK-20168][Streaming Kinesis] Setting the timestamp d...
Posted by yashs360 <gi...@git.apache.org>.
Github user yashs360 commented on the issue:
https://github.com/apache/spark/pull/21541
No this code path only gets invoked when the resume mode is specified as
AT_TIMESTAMP. And it later usus Amazon's client which fails now. Having
this change would not set the timestamp directly but via a different method
which sets the timestamp for us and the resume mode as AT_TIMESTAMP.
On Fri., 6 Jul. 2018, 11:43 pm Sean Owen, <no...@github.com> wrote:
> I don't know this code myself, but the change looks reasonable. A test is
> ideal, but some failure modes are hard to test. However isn't this code
> path always invoked when this receiver is used? would it not have failed
> commonly already? But again I don't know this code.
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/spark/pull/21541#issuecomment-403037972>, or mute
> the thread
> <https://github.com/notifications/unsubscribe-auth/Ag5-M8BzYkFoT8LvsXRhVFEIWQAcEozNks5uD2lvgaJpZM4Uk3Hq>
> .
>
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21541: [SPARK-20168][Streaming Kinesis] Setting the timestamp d...
Posted by yashs360 <gi...@git.apache.org>.
Github user yashs360 commented on the issue:
https://github.com/apache/spark/pull/21541
Any reviews here please.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #21541: [SPARK-20168][Streaming Kinesis] Setting the time...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/21541
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21541: [SPARK-20168][Streaming Kinesis] Setting the timestamp d...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/21541
Merged to master
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21541: [SPARK-20168][Streaming Kinesis] Setting the timestamp d...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21541
**[Test build #4210 has started](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/4210/testReport)** for PR 21541 at commit [`1b10c61`](https://github.com/apache/spark/commit/1b10c611a7c5f7d5ac39b691d6b1b8b59fc60068).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21541: [SPARK-20168][Streaming Kinesis] Setting the timestamp d...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21541
Can one of the admins verify this patch?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21541: [SPARK-20168][Streaming Kinesis] Setting the timestamp d...
Posted by yashs360 <gi...@git.apache.org>.
Github user yashs360 commented on the issue:
https://github.com/apache/spark/pull/21541
Thanks @srowen
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21541: [SPARK-20168][Streaming Kinesis] Setting the timestamp d...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21541
Can one of the admins verify this patch?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21541: [SPARK-20168][Streaming Kinesis] Setting the timestamp d...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21541
**[Test build #4210 has finished](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/4210/testReport)** for PR 21541 at commit [`1b10c61`](https://github.com/apache/spark/commit/1b10c611a7c5f7d5ac39b691d6b1b8b59fc60068).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21541: [SPARK-20168][Streaming Kinesis] Setting the timestamp d...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the issue:
https://github.com/apache/spark/pull/21541
I don't know this code myself, but the change looks reasonable. A test is ideal, but some failure modes are hard to test. However isn't this code path always invoked when this receiver is used? would it not have failed commonly already? But again I don't know this code.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21541: [SPARK-20168][Streaming Kinesis] Setting the timestamp d...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21541
**[Test build #4209 has finished](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/4209/testReport)** for PR 21541 at commit [`133ba87`](https://github.com/apache/spark/commit/133ba8721319efa9bfa64fbb53a690b5140c957b).
* This patch **fails Java style tests**.
* This patch **does not merge cleanly**.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21541: [SPARK-20168][Streaming Kinesis] Setting the timestamp d...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21541
Can one of the admins verify this patch?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21541: [SPARK-20168][Streaming Kinesis] Setting the timestamp d...
Posted by yashs360 <gi...@git.apache.org>.
Github user yashs360 commented on the issue:
https://github.com/apache/spark/pull/21541
@brkyvz for your review
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org