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-21235][TESTS] UTest should clear temp results when run case #18474

Closed
wants to merge 1 commit into from

Conversation

wangjiaochun
Copy link
Contributor

Signed-off-by: 10087686 wang.jiaochun@zte.com.cn

What changes were proposed in this pull request?

when run this case encryptionTest("on-disk storage") end, it has temp result not clear
Users...\AppData\Local\Temp\blockmgr-865114ea-8e5c-4b20-9a25-1224cfe5545b\01\test_a3
Users...\AppData\Local\Temp\blockmgr-865114ea-8e5c-4b20-9a25-1224cfe5545b\01\test_a2
Users...\AppData\Local\Temp\blockmgr-865114ea-8e5c-4b20-9a25-1224cfe5545b\01\test_a1

so,I think it's best to clear result file;
(Please fill in changes proposed in this fix)
store.removeBlock("a1")
store.removeBlock("a2")
store.removeBlock("a3")

How was this patch tested?

(Please explain how this patch was tested. E.g. unit tests, integration tests, manual tests)
(If this patch involves UI changes, please attach a screenshot; otherwise, remove this)
Run encryptionTest("on-disk storage")
Please review http://spark.apache.org/contributing.html before opening a pull request.

Signed-off-by: 10087686 <wang.jiaochun@zte.com.cn>
@AmplabJenkins
Copy link

Can one of the admins verify this patch?

@felixcheung
Copy link
Member

hi - I don't think [SPARKR] in the title is right. Sounds like this should be [core]

@jiangxb1987
Copy link
Contributor

We should not need this, because in afterEach() we will stop each store, which should clear both the memoryStore and the disk blocks.

@wangjiaochun wangjiaochun changed the title [SPARK-21235][SPARKR] UTest should clear temp results when run case [SPARK-21235][TESTS] UTest should clear temp results when run case Jul 3, 2017
@wangjiaochun
Copy link
Contributor Author

I have run this case many times,the memoryStore temp file will be cleared,but the disk blocks is really not clear.

@jiangxb1987
Copy link
Contributor

Sorry but I can't repro this on my local environment. Could you provide more detail on this? Thanks!

@wangjiaochun
Copy link
Contributor Author

  1. Test environment and test method:IDEA project direct Run BlockManagerSuite.scala.
  2. I test this case again use step through,find this case encryptionTest("on-disk storage") Runs a test twice, if SparkConf object with encryption off(false), the disk blocks will clear. if encryption is on(set ture), disk blocks not clear。

@HyukjinKwon
Copy link
Member

@wangjiaochun Are you running this on Windows?

@wangjiaochun
Copy link
Contributor Author

Yes, Running this on Windows7.

@HyukjinKwon
Copy link
Member

@wangjiaochun, can you add a small test to verify this? I can run automated test on Windows via AppVeyor and share the results.

@kiszk
Copy link
Member

kiszk commented Aug 7, 2017

In my Windows10 environment with Intellj, to leave temp result does not occur for "on-disk storage".
It happens for "SPARK-20640: Shuffle registration timeout and maxAttempts conf are working". I am looking for another test case that causes this. I will identify tomorrow.

I confirmed the followings:
"on-disk storage": one top directory for temp result was left
"LRU with mixed storage levels": one top directory for temp result was left
"LRU with mixed storage levels and streams": one top directory for temp result was left
"SPARK-20640: Shuffle registration timeout and maxAttempts conf are working": three top directories for temp result was left

In summary, I can reproduce this problem. In addition to this test case, there are other test cases that leave temp result directory.

@HyukjinKwon
Copy link
Member

@wangjiaochun, let's close this if you are unable to explain why this happens in a certain environment. Looks hard to reproduce.

@wangjiaochun
Copy link
Contributor Author

Thanks,I will resolve all problems. @kiszk

@HyukjinKwon
Copy link
Member

Thanks for reproduction @kiszk. @wangjiaochun, I think the cause should be explained here. I remember I reviewed similar PRs before where the cause was Utils.getOrCreateLocalRootDirs is being cached.

However, what I still don't get is, if my understanding is correct, we are moving all temp directories in shutdown hooks at least. Did I maybe miss something - @jiangxb1987?

@jiangxb1987
Copy link
Contributor

Yea, I agree we should find the root cause why these temp directories are not deleted, this should be a minor issue though.

@kiszk
Copy link
Member

kiszk commented Sep 8, 2017

@wangjiaochun kindly ping

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