Skip to content

Conversation

@JoshRosen
Copy link
Contributor

This patch fixes two difficult-to-reproduce Jenkins test failures in InputOutputMetricsSuite (SPARK-5227 and SPARK-5679). The problem was that WholeTextFileRecordReaderSuite modifies the fs.local.block.size Hadoop configuration and this change was affecting subsequent test suites due to Hadoop's caching of FileSystem instances (see HADOOP-8490 for more details).

The fix implemented here is to disable FileSystem caching in WholeTextFileRecordReaderSuite.

@SparkQA
Copy link

SparkQA commented Feb 13, 2015

Test build #27461 has started for PR 4599 at commit 47dc447.

  • This patch merges cleanly.

@JoshRosen
Copy link
Contributor Author

I SSH'ed into an AMPLab Jenkins box to reproduce the original failures and have confirmed that this patch fixes them.

/cc @ksakellis

@JoshRosen
Copy link
Contributor Author

Choose a reason for hiding this comment

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

So, do you think we should disable it across all tests? just in case there are other tests that also modify the hadoop configuration thinking that the config objects are local to them? It might bite someone else in the butt later if we don't globally do this. I don't think there is a global test class that every tests inherits, maybe we can add it in SparkSparkContext since a lot of the new tests written use that trait?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good question. If we wanted to disable this in all tests, then I think the right place to do that would be in the Maven and SBT builds via system properties.

I chose not to do that here because I wasn't sure whether doing so might mask bugs, since most users of Spark will run with FileSystem caching enabled (I think that disabling it across the board may harm performance, since it sounds like a lot of Hadoop code assumes that FileSystem.get is cheap, and, accordingly, calls it many times).

Copy link
Contributor

Choose a reason for hiding this comment

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

My opinion is that doing it for all tests would be a bit drastic. It's not that modifications to Configuration objects arbitrarily affect other Configuration objects. It's that this test specifically relies on some underlying properties set on the FileSystem, and the FS Cache allows these to leak to other FS instances.

@SparkQA
Copy link

SparkQA commented Feb 14, 2015

Test build #27461 has finished for PR 4599 at commit 47dc447.

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

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/27461/
Test PASSed.

@JoshRosen
Copy link
Contributor Author

/cc @pwendell @andrewor14

@ksakellis
Copy link

Ok, this LTGM

@pwendell
Copy link
Contributor

LGTM .

asfgit pushed a commit that referenced this pull request Feb 14, 2015
…cordReaderSuite

This patch fixes two difficult-to-reproduce Jenkins test failures in InputOutputMetricsSuite (SPARK-5227 and SPARK-5679).  The problem was that WholeTextFileRecordReaderSuite modifies the `fs.local.block.size` Hadoop configuration and this change was affecting subsequent test suites due to Hadoop's caching of FileSystem instances (see HADOOP-8490 for more details).

The fix implemented here is to disable FileSystem caching in WholeTextFileRecordReaderSuite.

Author: Josh Rosen <joshrosen@databricks.com>

Closes #4599 from JoshRosen/inputoutputsuite-fix and squashes the following commits:

47dc447 [Josh Rosen] [SPARK-5227] [SPARK-5679] Disable FileSystem cache in WholeTextFileRecordReaderSuite

(cherry picked from commit d06d5ee)
Signed-off-by: Patrick Wendell <patrick@databricks.com>
guavuslabs-builder pushed a commit to ThalesGroup/spark that referenced this pull request Feb 14, 2015
…cordReaderSuite

This patch fixes two difficult-to-reproduce Jenkins test failures in InputOutputMetricsSuite (SPARK-5227 and SPARK-5679).  The problem was that WholeTextFileRecordReaderSuite modifies the `fs.local.block.size` Hadoop configuration and this change was affecting subsequent test suites due to Hadoop's caching of FileSystem instances (see HADOOP-8490 for more details).

The fix implemented here is to disable FileSystem caching in WholeTextFileRecordReaderSuite.

Author: Josh Rosen <joshrosen@databricks.com>

Closes apache#4599 from JoshRosen/inputoutputsuite-fix and squashes the following commits:

47dc447 [Josh Rosen] [SPARK-5227] [SPARK-5679] Disable FileSystem cache in WholeTextFileRecordReaderSuite

(cherry picked from commit d06d5ee)
Signed-off-by: Patrick Wendell <patrick@databricks.com>
@asfgit asfgit closed this in d06d5ee Feb 14, 2015
@JoshRosen JoshRosen deleted the inputoutputsuite-fix branch February 14, 2015 01:51
markhamstra pushed a commit to markhamstra/spark that referenced this pull request Feb 24, 2015
…cordReaderSuite

This patch fixes two difficult-to-reproduce Jenkins test failures in InputOutputMetricsSuite (SPARK-5227 and SPARK-5679).  The problem was that WholeTextFileRecordReaderSuite modifies the `fs.local.block.size` Hadoop configuration and this change was affecting subsequent test suites due to Hadoop's caching of FileSystem instances (see HADOOP-8490 for more details).

The fix implemented here is to disable FileSystem caching in WholeTextFileRecordReaderSuite.

Author: Josh Rosen <joshrosen@databricks.com>

Closes apache#4599 from JoshRosen/inputoutputsuite-fix and squashes the following commits:

47dc447 [Josh Rosen] [SPARK-5227] [SPARK-5679] Disable FileSystem cache in WholeTextFileRecordReaderSuite

(cherry picked from commit d06d5ee)
Signed-off-by: Patrick Wendell <patrick@databricks.com>
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.

6 participants