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-8688][YARN]Bug fix: disable the cache fs to gain the HDFS connection. #7069

Closed
wants to merge 4 commits into from

Conversation

SaintBacchus
Copy link
Contributor

If fs.hdfs.impl.disable.cache was false(default), FileSystem will use the cached DFSClient which use old token.
AMDelegationTokenRenewer

    val credentials = UserGroupInformation.getCurrentUser.getCredentials
    credentials.writeTokenStorageFile(tempTokenPath, discachedConfiguration)

Although the credentials had the new Token, but it still use the cached client and old token.
So It's better to set the fs.hdfs.impl.disable.cache as true to avoid token expired.

Jira

@AmplabJenkins
Copy link

Merged build triggered.

@AmplabJenkins
Copy link

Merged build started.

@SparkQA
Copy link

SparkQA commented Jun 28, 2015

Test build #35935 has started for PR 7069 at commit cf776a1.

*/
private[spark] def getDiscachedConf(hadoopConf: Configuration, path: Path): Configuration = {
val newConf = new Configuration(hadoopConf)
val confKey = s"fs.${path.toUri.getScheme}.impl.disable.cache"
Copy link
Member

Choose a reason for hiding this comment

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

"discached" isn't a word so I'd suggest ...ConfBypassingFSCache? wordy, but accurate

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, rename it to be getConfBypassingFSCache

@AmplabJenkins
Copy link

Merged build triggered.

@SparkQA
Copy link

SparkQA commented Jun 28, 2015

Test build #35935 has finished for PR 7069 at commit cf776a1.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • class Module(object):

@AmplabJenkins
Copy link

Merged build finished. Test PASSed.

@AmplabJenkins
Copy link

Merged build started.

@SparkQA
Copy link

SparkQA commented Jun 28, 2015

Test build #35937 has started for PR 7069 at commit 0cd55c9.

@SparkQA
Copy link

SparkQA commented Jun 28, 2015

Test build #35937 has finished for PR 7069 at commit 0cd55c9.

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

@AmplabJenkins
Copy link

Merged build finished. Test PASSed.

@@ -65,6 +65,8 @@ private[yarn] class AMDelegationTokenRenewer(
sparkConf.getInt("spark.yarn.credentials.file.retention.days", 5)
private val numFilesToKeep =
sparkConf.getInt("spark.yarn.credentials.file.retention.count", 5)
private val discachedConfiguration =
Copy link
Contributor

Choose a reason for hiding this comment

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

echoing @srowen's comment, can you rename this? I don't understand what discached means

Copy link
Contributor

Choose a reason for hiding this comment

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

I would just call this freshHadoopConf to be more explicit

@AmplabJenkins
Copy link

Merged build triggered.

@AmplabJenkins
Copy link

Merged build started.

@SparkQA
Copy link

SparkQA commented Jun 30, 2015

Test build #36074 has started for PR 7069 at commit 8fb9eb9.

@SparkQA
Copy link

SparkQA commented Jun 30, 2015

Test build #36074 has finished for PR 7069 at commit 8fb9eb9.

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

@AmplabJenkins
Copy link

Merged build finished. Test PASSed.

* This is to prevent the DFSClient from using an old cached token to connect to the NameNode.
*/
private[spark]
def getConfBypassingFSCache(hadoopConf: Configuration, path: Path): Configuration = {
Copy link
Member

Choose a reason for hiding this comment

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

One line, and this does not need a Path arg but just a scheme, perhaps.

Copy link
Contributor

Choose a reason for hiding this comment

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

if it doesn't fit on one line:

private[spark] def getConfBypassingFSCache(
    hadoopConf: Configuration,
    scheme: String): Configuration = {
  ...
}

@andrewor14
Copy link
Contributor

Looks great. I think this is basically ready for merge. @harishreedharan can you do a sign off?

@AmplabJenkins
Copy link

Merged build triggered.

@AmplabJenkins
Copy link

Merged build started.

@SparkQA
Copy link

SparkQA commented Jul 1, 2015

Test build #36221 has started for PR 7069 at commit f94cd0b.

@SparkQA
Copy link

SparkQA commented Jul 1, 2015

Test build #36221 has finished for PR 7069 at commit f94cd0b.

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

@AmplabJenkins
Copy link

Merged build finished. Test PASSed.

@harishreedharan
Copy link
Contributor

I am on vacation till Sunday. I can take a look after. If you want to merge
it before that, please do. I can look at it and file follow up jiras if
required.

On Tuesday, June 30, 2015, UCB AMPLab notifications@github.com wrote:

Merged build finished. Test PASSed.


Reply to this email directly or view it on GitHub
#7069 (comment).

Thanks,
Hari

@andrewor14
Copy link
Contributor

Alright then, I'm merging this into master. Thanks @SaintBacchus.

@asfgit asfgit closed this in 646366b Jul 2, 2015
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