Skip to content
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

[SPARK-20243][TESTS] DebugFilesystem.assertNoOpenStreams thread race #17592

Closed
wants to merge 2 commits into from

Conversation

bogdanrdc
Copy link
Contributor

What changes were proposed in this pull request?

Synchronize access to openStreams map.

How was this patch tested?

Existing tests.


def clearOpenStreams(): Unit = {
def addOpenStream(stream: FSDataInputStream): Unit = synchronized {
Copy link
Member

Choose a reason for hiding this comment

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

It's a little safer and tidier to synchronize on openStream rather than the containing object. Looks good to me though.

@hvanhovell
Copy link
Contributor

LGTM

@SparkQA
Copy link

SparkQA commented Apr 10, 2017

Test build #75661 has finished for PR 17592 at commit 19ca5a1.

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

@SparkQA
Copy link

SparkQA commented Apr 10, 2017

Test build #75662 has finished for PR 17592 at commit d486d60.

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

@hvanhovell
Copy link
Contributor

Merging to master. Thanks!

@asfgit asfgit closed this in 4f7d49b Apr 10, 2017
@rxin
Copy link
Contributor

rxin commented Apr 10, 2017

Should this go into branch-2.1 as well?

asfgit pushed a commit that referenced this pull request Apr 14, 2017
## What changes were proposed in this pull request?

Synchronize access to openStreams map.

## How was this patch tested?

Existing tests.

Author: Bogdan Raducanu <bogdan@databricks.com>

Closes #17592 from bogdanrdc/SPARK-20243.
@hvanhovell
Copy link
Contributor

I cherry picked this into branch-2.1

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.

5 participants