You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by arunmahadevan <gi...@git.apache.org> on 2018/06/04 17:44:13 UTC
[GitHub] spark pull request #21490: SPARK-24462: Initialize the offsets correctly whe...
GitHub user arunmahadevan opened a pull request:
https://github.com/apache/spark/pull/21490
SPARK-24462: Initialize the offsets correctly when restarting a query with saved state
## What changes were proposed in this pull request?
Initialize the `currentOffset` and `lastOffsetCommitted` correctly when the query is restarted with saved state (startOffset has some value)
## How was this patch tested?
existing unit tests
Please review http://spark.apache.org/contributing.html before opening a pull request.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/arunmahadevan/spark SPARK-24462
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/21490.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 #21490
----
commit c5ff731f2172b52fd1b42fa40ba38d564d203434
Author: Arun Mahadevan <ar...@...>
Date: 2018-06-04T17:38:14Z
SPARK-24462: Initialize the offsets correctly when restarting a query with saved state
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21490: [SPARK-24462][SS] Initialize the offsets correctly when ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21490
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21490: SPARK-24462: Initialize the offsets correctly when resta...
Posted by arunmahadevan <gi...@git.apache.org>.
Github user arunmahadevan commented on the issue:
https://github.com/apache/spark/pull/21490
@tdas @jose-torres @jerryshao @HeartSaVioR
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21490: [SPARK-24462][SS] Initialize the offsets correctly when ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21490
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/91453/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21490: [SPARK-24462][SS] Initialize the offsets correctly when ...
Posted by jerryshao <gi...@git.apache.org>.
Github user jerryshao commented on the issue:
https://github.com/apache/spark/pull/21490
@arunmahadevan I have already proposed a fix for this issue https://github.com/apache/spark/pull/20958, but seems it is not so necessary to fix based on the comments, and pending to review.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21490: SPARK-24462: Initialize the offsets correctly when resta...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21490
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 #21490: [SPARK-24462][SS] Initialize the offsets correctly when ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21490
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/94525/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21490: [SPARK-24462][SS] Initialize the offsets correctly when ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21490
**[Test build #94525 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/94525/testReport)** for PR 21490 at commit [`c5ff731`](https://github.com/apache/spark/commit/c5ff731f2172b52fd1b42fa40ba38d564d203434).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21490: [SPARK-24462][SS] Initialize the offsets correctly when ...
Posted by jose-torres <gi...@git.apache.org>.
Github user jose-torres commented on the issue:
https://github.com/apache/spark/pull/21490
(sorry, I've been busy with Spark Summit)
The problem I see is that fault tolerance might not be cleanly separable from query stop tolerance. If a user stops the query at the wrong time, will they see strange results which make them think Spark is doing something wrong?
If we can come up with a clean story for what users will see on restart, and it makes sense no matter when the query was stopped, I agree that this change or #20958 would be reasonable to add.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21490: [SPARK-24462][SS] Initialize the offsets correctly when ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21490
**[Test build #91453 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91453/testReport)** for PR 21490 at commit [`c5ff731`](https://github.com/apache/spark/commit/c5ff731f2172b52fd1b42fa40ba38d564d203434).
* 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 #21490: [SPARK-24462][SS] Initialize the offsets correctly when ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21490
**[Test build #94525 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/94525/testReport)** for PR 21490 at commit [`c5ff731`](https://github.com/apache/spark/commit/c5ff731f2172b52fd1b42fa40ba38d564d203434).
* 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 #21490: [SPARK-24462][SS] Initialize the offsets correctly when ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21490
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21490: [SPARK-24462][SS] Initialize the offsets correctly when ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/21490
**[Test build #91453 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91453/testReport)** for PR 21490 at commit [`c5ff731`](https://github.com/apache/spark/commit/c5ff731f2172b52fd1b42fa40ba38d564d203434).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21490: [SPARK-24462][SS] Initialize the offsets correctly when ...
Posted by jose-torres <gi...@git.apache.org>.
Github user jose-torres commented on the issue:
https://github.com/apache/spark/pull/21490
The proposed strategy still won't work correctly, though. If a batch fails before commit, the restart won't replay the same records that were in that batch, but the new ones incoming from the socket.
I agree that the current state is non-ideal, but IMO we should either make batches actually replayable or add an exception explicitly stating that it's not allowed to restart a text socket source.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21490: [SPARK-24462][SS] Initialize the offsets correctly when ...
Posted by jose-torres <gi...@git.apache.org>.
Github user jose-torres commented on the issue:
https://github.com/apache/spark/pull/21490
What's the motivation for this change? Does something go wrong if they aren't initialized correctly? (And note that we document that the socket source doesn't support fault recovery at all - not sure whether that should affect this PR.)
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21490: [SPARK-24462][SS] Initialize the offsets correctly when ...
Posted by arunmahadevan <gi...@git.apache.org>.
Github user arunmahadevan commented on the issue:
https://github.com/apache/spark/pull/21490
@jerryshao , was not aware of your PR before raising this, so looks like we both encountered similar issues. I think the intention is similar here and attempts to start from last saved offset (offsets are contiguous which may be slightly different from your PR). And maybe we don't need a separate flag, since the socket source is not expected to be fault tolerant.
The intention is with the fix socket source can work across restarts and allows users to play with the state in sample programs. Overall I think this will make the socket source more usable.
@tdas can you comment?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21490: [SPARK-24462][SS] Initialize the offsets correctly when ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/21490
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 #21490: [SPARK-24462][SS] Initialize the offsets correctly when ...
Posted by arunmahadevan <gi...@git.apache.org>.
Github user arunmahadevan commented on the issue:
https://github.com/apache/spark/pull/21490
If "checkpointLocation" is set and the SS application is restarted, it throws the below exception because the currentOffset and lastOffsetCommitted are initialized to -1.
```
18/06/01 22:47:04 ERROR MicroBatchExecution: Query [id = 0bdc4428-5d21-4237-9d64-898ae65f28f3, runId = f6822423-2bd2-47c1-8ed6-799d1c481195] terminated with error
java.lang.RuntimeException: Offsets committed out of order: 2 followed by -1
at scala.sys.package$.error(package.scala:27)
at org.apache.spark.sql.execution.streaming.sources.TextSocketMicroBatchReader.commit(socket.scala:197)
at org.apache.spark.sql.execution.streaming.MicroBatchExecution$$anonfun$org$apache$spark$sql$execution$streaming$MicroBatchExecution$$constructNextBatch$1$$anonfun$apply$mcZ$sp$2$$anonfun$apply$mcV$sp$5.apply(MicroBatchExecution.scala:377)
```
The proposed patch fixes this so that the query can be restarted with saved state. This could be used for running sample programs with saved state and examining the results (though its not fully fault tolerant)
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #21490: [SPARK-24462][SS] Initialize the offsets correctly when ...
Posted by arunmahadevan <gi...@git.apache.org>.
Github user arunmahadevan commented on the issue:
https://github.com/apache/spark/pull/21490
The patch does not attempt to make the micro-batch text-socket source fault tolerant and I suppose that may not be very trivial.
This just makes the source work when query is restarted allowing one to play with the sate in example programs (and could even produce correct results if the restart is done gracefully). Not allowing restarts may be too restrictive IMO.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org