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-5661]function hasShutdownDeleteTachyonDir should use shutdownDeleteTachyonPaths to determine whether contains file #4418

Closed
wants to merge 3 commits into from

Conversation

viper-kun
Copy link
Contributor

hasShutdownDeleteTachyonDir(file: TachyonFile) should use shutdownDeleteTachyonPaths(not shutdownDeletePaths) to determine Whether contain file. To solve it ,delete two unused function.

@viper-kun viper-kun changed the title remove unused function Remove unused function Feb 6, 2015
@viper-kun
Copy link
Contributor Author

cc @haoyuan

@srowen
Copy link
Member

srowen commented Feb 6, 2015

These do appear unused, at the moment, but what's the need to delete them? They could plausibly be useful later; it's not completely useless code. (Normally changes need a JIRA too, although this is borderline)

@viper-kun
Copy link
Contributor Author

@srowen
ok. if it is useful later; we should change it like this
def hasShutdownDeleteTachyonDir(file: TachyonFile): Boolean = {
val absolutePath = file.getPath()
shutdownDeleteTachyonPaths.synchronized {
shutdownDeleteTachyonPaths.contains(absolutePath)
}
}

@viper-kun viper-kun changed the title Remove unused function function hasShutdownDeleteTachyonDir should use shutdownDeleteTachyonPaths to determine whether contains file Feb 6, 2015
@JoshRosen
Copy link
Contributor

I think we should create a JIRA for this, if only to help us keep track of where this change is committed.

@haoyuan
Copy link
Contributor

haoyuan commented Feb 6, 2015

Thanks @viper-kun

Agree w/ @JoshRosen

@viper-kun viper-kun changed the title function hasShutdownDeleteTachyonDir should use shutdownDeleteTachyonPaths to determine whether contains file [SPARK-5661]function hasShutdownDeleteTachyonDir should use shutdownDeleteTachyonPaths to determine whether contains file Feb 7, 2015
@viper-kun
Copy link
Contributor Author

@JoshRosen @haoyuan
ok. i will create a JIRA.

@srowen
Copy link
Member

srowen commented Feb 9, 2015

ok to test

@srowen
Copy link
Member

srowen commented Feb 9, 2015

OK, that change seems reasonable as a bug fix.
Isn't it weird to have Tachyon specific support code in such a common Utils class? Shouldn't this go to the Tachyon block manager or something?

@viper-kun
Copy link
Contributor Author

@srowen it is a static function and unused now. I think we should better leave it in Utils class now.

@SparkQA
Copy link

SparkQA commented Feb 9, 2015

Test build #27069 has finished for PR 4418 at commit 87340eb.

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

@srowen
Copy link
Member

srowen commented Feb 9, 2015

@haoyuan what do you think about moving these methods to TachyonBlockManager? one is unused and the other is used only in that class. I understand Tachyon deps are already part of core, but, it seems funny to have them show up in the common Utils class. Along the way we have to either fix the bug in the has method or delete it, yes.

asfgit pushed a commit that referenced this pull request Feb 17, 2015
…eleteTachyonPaths to determine whether contains file

hasShutdownDeleteTachyonDir(file: TachyonFile) should use shutdownDeleteTachyonPaths(not shutdownDeletePaths) to determine Whether contain file. To solve it ,delete two unused function.

Author: xukun 00228947 <xukun.xu@huawei.com>
Author: viper-kun <xukun.xu@huawei.com>

Closes #4418 from viper-kun/deleteunusedfun and squashes the following commits:

87340eb [viper-kun] fix style
3d6c69e [xukun 00228947] fix bug
2bc397e [xukun 00228947] deleteunusedfun

(cherry picked from commit b271c26)
Signed-off-by: Sean Owen <sowen@cloudera.com>
@asfgit asfgit closed this in b271c26 Feb 17, 2015
@viper-kun viper-kun deleted the deleteunusedfun branch January 18, 2017 09:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants