[SPARK-9651] Fix UnsafeExternalSorterSuite. #7970
Conversation
First, it's probably a bad idea to call generated Scala methods from Java. In this case, the method being called wasn't actually "Utils.createTempDir()", but actually the method that returns the first default argument to the actual createTempDir method, which is just the location of java.io.tmpdir; meaning that all tests in the class were using the same temp dir, and thus affecting each other. Second, spillingOccursInResponseToMemoryPressure was not writing enough records to actually cause a spill.
|
/cc @rxin @JoshRosen |
There was a problem hiding this comment.
This has to be deleted in tearDown too I presume in order to fix this? or else it's still reusing a same dir?
I checked and there is a similar problem in some other Java tests:
JavaSaveLoadSuite
JavaMetastoreDataSourcesSuite
JavaAPISuite
There was a problem hiding this comment.
In this case no; setUp and tearDown are per-test, so each test gets a separate temp dir.
I'll take a look at the other tests. Might be worth it to add the temp dir helper method in some common java class.
There was a problem hiding this comment.
Oh right it makes a subdir every time anyway. Well, it'd be better to clean up the temp dir in tearDown if possible.
There was a problem hiding this comment.
Ok, I'll do this. I'll fix this test this way, then file a separate bug to have a shared, Java createTempDir() and use it from everywhere. How does that sound?
This is the only unit test that consistently fails in our builds so I'd like to at least unblock it.
There was a problem hiding this comment.
Actually all the other tests you mention do the right thing. This is the only place that was calling the wrong Scala method.
|
Seems fine to me, pending Jenkins. |
There was a problem hiding this comment.
This didn't fail for me locally, which is kind of odd.
There was a problem hiding this comment.
Fails for me consistently if I use 100000.
Running org.apache.spark.util.collection.unsafe.sort.UnsafeExternalSorterSuite
Tests run: 2, Failures: 2, Errors: 0, Skipped: 0, Time elapsed: 0.99 sec <<< FAILURE! - in org.apache.spark.util.collection.unsafe.sort.UnsafeExternalSorterSuite
spillingOccursInResponseToMemoryPressure(org.apache.spark.util.collection.unsafe.sort.UnsafeExternalSorterSuite) Time elapsed: 0.426 sec <<< FAILURE!
java.lang.AssertionError:
Expected: a value equal to or greater than <1>
got: <0>
at org.junit.Assert.assertThat(Assert.java:780)
at org.junit.Assert.assertThat(Assert.java:738)
at org.apache.spark.util.collection.unsafe.sort.UnsafeExternalSorterSuite.spillingOccursInResponseToMemoryPressure(UnsafeExternalSorterSuite.java:243)
spillingOccursInResponseToMemoryPressure(org.apache.spark.util.collection.unsafe.sort.UnsafeExternalSorterSuite) Time elapsed: 0.429 sec <<< FAILURE!
java.lang.AssertionError: expected:<0> but was:<69206016>
at org.junit.Assert.fail(Assert.java:93)
at org.junit.Assert.failNotEquals(Assert.java:647)
at org.junit.Assert.assertEquals(Assert.java:128)
at org.junit.Assert.assertEquals(Assert.java:472)
at org.junit.Assert.assertEquals(Assert.java:456)
at org.apache.spark.util.collection.unsafe.sort.UnsafeExternalSorterSuite.tearDown(UnsafeExternalSorterSuite.java:150)
Results :
Failed tests:
org.apache.spark.util.collection.unsafe.sort.UnsafeExternalSorterSuite.spillingOccursInResponseToMemoryPressure(org.apache.spark.util.collection.unsafe.sort.UnsafeExternalSorterSuite)
Run 1: UnsafeExternalSorterSuite.spillingOccursInResponseToMemoryPressure:243
Expected: a value equal to or greater than <1>
got: <0>
Run 2: UnsafeExternalSorterSuite.tearDown:150 expected:<0> but was:<69206016>
|
Test build #39903 has finished for PR 7970 at commit
|
|
Well the test being changed did pass... retest this please. |
|
Test build #39909 timed out for PR 7970 at commit |
|
Test build #239 has finished for PR 7970 at commit
|
|
I'm going to merge this. |
First, it's probably a bad idea to call generated Scala methods from Java. In this case, the method being called wasn't actually "Utils.createTempDir()", but actually the method that returns the first default argument to the actual createTempDir method, which is just the location of java.io.tmpdir; meaning that all tests in the class were using the same temp dir, and thus affecting each other. Second, spillingOccursInResponseToMemoryPressure was not writing enough records to actually cause a spill. Author: Marcelo Vanzin <vanzin@cloudera.com> Closes #7970 from vanzin/SPARK-9651 and squashes the following commits: 74d357f [Marcelo Vanzin] Clean up temp dir on test tear down. a64f36a [Marcelo Vanzin] [SPARK-9651] Fix UnsafeExternalSorterSuite. (cherry picked from commit 4399b7b) Signed-off-by: Reynold Xin <rxin@databricks.com>
|
Test build #39930 has finished for PR 7970 at commit
|
5 participants
First, it's probably a bad idea to call generated Scala methods
from Java. In this case, the method being called wasn't actually
"Utils.createTempDir()", but actually the method that returns the
first default argument to the actual createTempDir method, which
is just the location of java.io.tmpdir; meaning that all tests in
the class were using the same temp dir, and thus affecting each
other.
Second, spillingOccursInResponseToMemoryPressure was not writing
enough records to actually cause a spill.