Skip to content

Conversation

@zsxwing
Copy link
Member

@zsxwing zsxwing commented Aug 19, 2015

Test failure: https://amplab.cs.berkeley.edu/jenkins/job/Spark-Master-Maven-with-YARN/HADOOP_PROFILE=hadoop-2.4,label=spark-test/3305/testReport/junit/org.apache.spark.streaming/StreamingContextSuite/stop_gracefully/

There is a race condition that setting trackerState to Started could happen after calling startReceiver. Then startReceiver won't start the receivers because it uses ! isTrackerStarted to check if ReceiverTracker is stopping or stopped. But actually, trackerState is Initialized and will be changed to Started soon.

Therefore, we should use isTrackerStopping || isTrackerStopped.

@zsxwing
Copy link
Member Author

zsxwing commented Aug 19, 2015

BTW, other usages of isTrackerStarted are right because they are in the synchronized body.

@zsxwing zsxwing changed the title [SPARK-9504][Streaming]Fix a race condition that startReceiver may happen before setting trackerState to Started [SPARK-10102][Streaming]Fix a race condition that startReceiver may happen before setting trackerState to Started Aug 19, 2015
@zsxwing
Copy link
Member Author

zsxwing commented Aug 19, 2015

Fired a new JIRA since this is a bug in the main codes.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you make this a internal function? def shouldStartReceiver: Boolean = !(isTrackerStopping || isTrackerStopped)
and then use if (!shouldStartReceiver) { ... }

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And add comments that its okay to start when the state = initialized / started

@tdas
Copy link
Contributor

tdas commented Aug 19, 2015

LGTM. Will merge when tests pass.

@SparkQA
Copy link

SparkQA commented Aug 19, 2015

Test build #41196 has finished for PR 8294 at commit 36e7836.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@tdas
Copy link
Contributor

tdas commented Aug 19, 2015

Merging this to master and 1.5, thanks @zsxwing for finding this issue.

asfgit pushed a commit that referenced this pull request Aug 19, 2015
… happen before setting trackerState to Started

Test failure: https://amplab.cs.berkeley.edu/jenkins/job/Spark-Master-Maven-with-YARN/HADOOP_PROFILE=hadoop-2.4,label=spark-test/3305/testReport/junit/org.apache.spark.streaming/StreamingContextSuite/stop_gracefully/

There is a race condition that setting `trackerState` to `Started` could happen after calling `startReceiver`. Then `startReceiver` won't start the receivers because it uses `! isTrackerStarted` to check if ReceiverTracker is stopping or stopped. But actually, `trackerState` is `Initialized` and will be changed to `Started` soon.

Therefore, we should use `isTrackerStopping || isTrackerStopped`.

Author: zsxwing <[email protected]>

Closes #8294 from zsxwing/SPARK-9504.

(cherry picked from commit 90273ef)
Signed-off-by: Tathagata Das <[email protected]>
@asfgit asfgit closed this in 90273ef Aug 19, 2015
@SparkQA
Copy link

SparkQA commented Aug 19, 2015

Test build #41191 timed out for PR 8294 at commit c1a6126 after a configured wait of 175m.

@zsxwing zsxwing deleted the SPARK-9504 branch August 19, 2015 05:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants