Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/4783
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enab
Github user srowen commented on the pull request:
https://github.com/apache/spark/pull/4783#issuecomment-98350827
LGTM
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/4783#issuecomment-98349631
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/4783#issuecomment-98349632
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/316
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/4783#issuecomment-98349628
[Test build #31663 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/31663/consoleFull)
for PR 4783 at commit
[`c4dcb41`](https://gith
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/4783#issuecomment-98342259
[Test build #31663 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/31663/consoleFull)
for PR 4783 at commit
[`c4dcb41`](https://githu
Github user srowen commented on the pull request:
https://github.com/apache/spark/pull/4783#issuecomment-98342214
Jenkins, retest this please.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/4783#issuecomment-98342225
Merged build started.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/4783#issuecomment-9834
Merged build triggered.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not ha
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/4783#issuecomment-98259159
Merged build finished. Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/4783#issuecomment-98259160
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/316
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/4783#issuecomment-98259151
[Test build #31624 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/31624/consoleFull)
for PR 4783 at commit
[`c4dcb41`](https://gith
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/4783#issuecomment-98257964
[Test build #31624 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/31624/consoleFull)
for PR 4783 at commit
[`c4dcb41`](https://githu
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/4783#issuecomment-98257790
Merged build triggered.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not ha
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/4783#issuecomment-98257798
Merged build started.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have
Github user advancedxy commented on the pull request:
https://github.com/apache/spark/pull/4783#issuecomment-98257433
@JoshRosen @srowen, I already tested in the test suite. Just want to make
sure if I don't miss anything obvious.
Here is the standalone reproduce sample.
https://
Github user JoshRosen commented on the pull request:
https://github.com/apache/spark/pull/4783#issuecomment-98202386
@advancedxy I can't answer the `ResetSystemPrroperties` question off the
top of my head and am a bit busy with other stuff right now, so why not try
writing a simple st
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/4783#issuecomment-98132414
[Test build #31548 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/31548/consoleFull)
for PR 4783 at commit
[`3f80640`](https://gith
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/4783#issuecomment-98132424
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/315
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/4783#issuecomment-98132423
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project
Github user advancedxy commented on the pull request:
https://github.com/apache/spark/pull/4783#issuecomment-98126019
A closer look at the `ResetSystemProperties` and `SizeEstimatorSuite`, the
`beforeEach` in `ResetSystemProperties` is overriden by `SizeEstimatorSuite`.
Should call `s
Github user srowen commented on the pull request:
https://github.com/apache/spark/pull/4783#issuecomment-98121489
OK that all looks good. Let's see what Jenkins says.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If you
Github user advancedxy commented on a diff in the pull request:
https://github.com/apache/spark/pull/4783#discussion_r29498601
--- Diff:
core/src/test/scala/org/apache/spark/util/SizeEstimatorSuite.scala ---
@@ -135,5 +160,20 @@ class SizeEstimatorSuite
assertResult(64)(Si
Github user advancedxy commented on the pull request:
https://github.com/apache/spark/pull/4783#issuecomment-98116848
`initialize()` is invoked multiple times in the test suite. Maybe it's a
hook for tests
---
If your project is set up for it, you can reply to this email and have
Github user advancedxy commented on a diff in the pull request:
https://github.com/apache/spark/pull/4783#discussion_r29498301
--- Diff:
core/src/test/scala/org/apache/spark/util/SizeEstimatorSuite.scala ---
@@ -59,6 +68,22 @@ class SizeEstimatorSuite
assertResult(24)(Size
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/4783#issuecomment-98112201
[Test build #31548 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/31548/consoleFull)
for PR 4783 at commit
[`3f80640`](https://githu
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/4783#issuecomment-98111075
Merged build triggered.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not ha
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/4783#issuecomment-9806
Merged build started.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have
Github user srowen commented on the pull request:
https://github.com/apache/spark/pull/4783#issuecomment-98099024
Needs a rebase.
I know it's not changed in this PR, but why are all those fields `var`?
they can be initialized once if the `initialize()` method is removed.
--
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/4783#discussion_r29496820
--- Diff:
core/src/test/scala/org/apache/spark/util/SizeEstimatorSuite.scala ---
@@ -135,5 +160,20 @@ class SizeEstimatorSuite
assertResult(64)(SizeEs
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/4783#discussion_r29496696
--- Diff:
core/src/test/scala/org/apache/spark/util/SizeEstimatorSuite.scala ---
@@ -59,6 +68,22 @@ class SizeEstimatorSuite
assertResult(24)(SizeEsti
Github user advancedxy commented on the pull request:
https://github.com/apache/spark/pull/4783#issuecomment-98093859
ping @rxin or @srowen .
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have t
Github user shivaram commented on the pull request:
https://github.com/apache/spark/pull/4783#issuecomment-96815195
Thanks @advancedxy for fixing the tests. This change LGTM. @rxin @srowen
Could you also take a final look at this ?
---
If your project is set up for it, you can reply
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/4783#issuecomment-96800849
[Test build #31061 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/31061/consoleFull)
for PR 4783 at commit
[`db1e948`](https://gith
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/4783#issuecomment-96775081
[Test build #31061 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/31061/consoleFull)
for PR 4783 at commit
[`db1e948`](https://githu
Github user shivaram commented on the pull request:
https://github.com/apache/spark/pull/4783#issuecomment-96773741
Jenkins, ok to test
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this fe
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/4783#issuecomment-96769562
Can one of the admins verify this patch?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your pro
Github user shivaram commented on the pull request:
https://github.com/apache/spark/pull/4783#issuecomment-96759699
Jenkins, test this please
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have t
Github user advancedxy commented on the pull request:
https://github.com/apache/spark/pull/4783#issuecomment-96685117
Ping @shivaram, @rxin. I finally got some time to look at the code.
The ExternalSorterSuite kept failing because there was no spilling when we
just insert 10 el
Github user advancedxy commented on the pull request:
https://github.com/apache/spark/pull/4783#issuecomment-83878165
@rxin as I mentioned in the previous comment, I use the method in this
article.
http://www.javaworld.com/article/2077496/testing-debugging/java-tip-130--do-you-kno
Github user rxin commented on the pull request:
https://github.com/apache/spark/pull/4783#issuecomment-83749071
What do you mean in practice Tuple2 is 24 bytes? How did you measure that?
If the object layout tool shows it takes 32 bytes, doesn't it just take 32
bytes?
---
If your p
Github user advancedxy commented on the pull request:
https://github.com/apache/spark/pull/4783#issuecomment-83410489
@shivaram @srowen Tuple2(Int, Int) got specialized to Tuple2$mcII$sp class.
But the Tuple2$mcII$sp is a subclass of Tuple2. So in our implementation, the
specialized c
Github user advancedxy commented on the pull request:
https://github.com/apache/spark/pull/4783#issuecomment-78435831
@shivaram The reason why ExternalSorter failed is that It doesn't spilling
files for these two failure tests.
( If we increase the input from `0 until 10` to `
Github user shivaram commented on the pull request:
https://github.com/apache/spark/pull/4783#issuecomment-78411597
I don't know much about the ExternalSorter. You could try to walk through
the code and try to see how the size estimator changes are affecting it.
@mateiz @rxin
Github user advancedxy commented on the pull request:
https://github.com/apache/spark/pull/4783#issuecomment-78410311
@shivaram ExternalSorterSuite failed on my local machine too. Do you have
any thought why ExternalSortedSuite is failing.
---
If your project is set up for it, you ca
Github user advancedxy commented on the pull request:
https://github.com/apache/spark/pull/4783#issuecomment-78407837
I just run test-only for SizeEstimatorSuite. It do pass the tests. I
thought the ExternalSorter was unrelated. I will test the ExternalSorter.
---
If your project is
Github user shivaram commented on the pull request:
https://github.com/apache/spark/pull/4783#issuecomment-78407056
@advancedxy Do the tests pass on your local machine ? It looks the
ExternalSorter uses a size-tracking hash map which in turn depends on the
SizeEstimator. So I am wonde
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/4783#issuecomment-78406725
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/28
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/4783#issuecomment-78406716
[Test build #28486 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/28486/consoleFull)
for PR 4783 at commit
[`9f92469`](https://gith
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/4783#issuecomment-78401317
[Test build #28486 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/28486/consoleFull)
for PR 4783 at commit
[`9f92469`](https://githu
Github user advancedxy commented on a diff in the pull request:
https://github.com/apache/spark/pull/4783#discussion_r26209062
--- Diff: core/src/main/scala/org/apache/spark/util/SizeEstimator.scala ---
@@ -279,8 +315,8 @@ private[spark] object SizeEstimator extends Logging {
Github user advancedxy commented on a diff in the pull request:
https://github.com/apache/spark/pull/4783#discussion_r26208786
--- Diff: core/src/main/scala/org/apache/spark/util/SizeEstimator.scala ---
@@ -257,21 +262,52 @@ private[spark] object SizeEstimator extends Logging {
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/4783#discussion_r26208393
--- Diff: core/src/main/scala/org/apache/spark/util/SizeEstimator.scala ---
@@ -257,21 +262,52 @@ private[spark] object SizeEstimator extends Logging {
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/4783#discussion_r26208364
--- Diff: core/src/main/scala/org/apache/spark/util/SizeEstimator.scala ---
@@ -279,8 +315,8 @@ private[spark] object SizeEstimator extends Logging {
n
Github user advancedxy commented on the pull request:
https://github.com/apache/spark/pull/4783#issuecomment-78223640
ping @shivaram, @srowen and @mateiz
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project do
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/4783#issuecomment-77672792
[Test build #28363 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/28363/consoleFull)
for PR 4783 at commit
[`cb0af95`](https://gith
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/4783#issuecomment-77672798
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/28
57 matches
Mail list logo