[GitHub] spark issue #16387: [SPARK-18986][Core] ExternalAppendOnlyMap shouldn't fail...

Github user viirya commented on the issue: https://github.com/apache/spark/pull/16387 Thanks! @vanzin Yes we can always come back to fix things later if there is an issue. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as

[GitHub] spark issue #16387: [SPARK-18986][Core] ExternalAppendOnlyMap shouldn't fail...

Github user vanzin commented on the issue: https://github.com/apache/spark/pull/16387 Alright. I'm merging this to master, we can always fix things later if there's an issue. Sorry for the delay @viirya --- If your project is set up for it, you can reply to this email and

[GitHub] spark issue #16387: [SPARK-18986][Core] ExternalAppendOnlyMap shouldn't fail...

Github user samkum commented on the issue: https://github.com/apache/spark/pull/16387 I can't commit on the timelines. I don't want to block things at my end. -Sameer. On Thu, Feb 16, 2017 at 1:23 AM, Marcelo Vanzin wrote: >

[GitHub] spark issue #16387: [SPARK-18986][Core] ExternalAppendOnlyMap shouldn't fail...

Github user vanzin commented on the issue: https://github.com/apache/spark/pull/16387 @samkum if you can't get back to us with the information we asked for, I'll just assume that the problem is on your side and push this change. It's been sitting here already reviewed for long

[GitHub] spark issue #16387: [SPARK-18986][Core] ExternalAppendOnlyMap shouldn't fail...

Github user vanzin commented on the issue: https://github.com/apache/spark/pull/16387 Go to the top of this page. Click on the "commits" tab. Look at the last two on the list. Remove those ("git reset" or "git revert" or whatever you prefer). Build, test. --- If your project is set

[GitHub] spark issue #16387: [SPARK-18986][Core] ExternalAppendOnlyMap shouldn't fail...

Github user samkum commented on the issue: https://github.com/apache/spark/pull/16387 Hi Marcelo, Are you referring to following commit. 5645533( https://github.com/apache/spark/pull/16387/commits/5645533ede8bb37aa316afbc0d003d042bf5d52d ) -Sameer.

[GitHub] spark issue #16387: [SPARK-18986][Core] ExternalAppendOnlyMap shouldn't fail...

Github user vanzin commented on the issue: https://github.com/apache/spark/pull/16387 @samkum the code is in this PR! Just revert the last two commits. --- 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

[GitHub] spark issue #16387: [SPARK-18986][Core] ExternalAppendOnlyMap shouldn't fail...

Github user samkum commented on the issue: https://github.com/apache/spark/pull/16387 Please share the commit id...i think that will make it clear. I can't share the code as its propriety to organization,. but i am just reducing a map phase, reduce by key and reducebykeyandwindow.

[GitHub] spark issue #16387: [SPARK-18986][Core] ExternalAppendOnlyMap shouldn't fail...

Github user vanzin commented on the issue: https://github.com/apache/spark/pull/16387 > I will testing with changes just to TaskManager class. No, we're asking you to test without the last two commits in this PR. Alternatively, if possible, you could post the code somewhere

[GitHub] spark issue #16387: [SPARK-18986][Core] ExternalAppendOnlyMap shouldn't fail...

Github user samkum commented on the issue: https://github.com/apache/spark/pull/16387 Hi liang, I got pulled into another project. Will need few more days to catch up on bandwidth. Just for confirmation, I will testing with changes just to TaskManager class.

[GitHub] spark issue #16387: [SPARK-18986][Core] ExternalAppendOnlyMap shouldn't fail...

Github user viirya commented on the issue: https://github.com/apache/spark/pull/16387 @samkum Any update? --- 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,

[GitHub] spark issue #16387: [SPARK-18986][Core] ExternalAppendOnlyMap shouldn't fail...

Github user vanzin commented on the issue: https://github.com/apache/spark/pull/16387 > wouldnt the commit fail at the assertion No. We want to know if testing without that commit shows the same GC behavior you saw with that commit. There's no assert in the code when you just

[GitHub] spark issue #16387: [SPARK-18986][Core] ExternalAppendOnlyMap shouldn't fail...

Github user samkum commented on the issue: https://github.com/apache/spark/pull/16387 @mridulm - need one clarification: if I test it with out ut @viirya commit b1ef9ec,wouldnt the commit fail at the assertion.(

[GitHub] spark issue #16387: [SPARK-18986][Core] ExternalAppendOnlyMap shouldn't fail...

Github user samkum commented on the issue: https://github.com/apache/spark/pull/16387 ok..I will get back to you in next couple of days. -Sameer. On Thu, Jan 26, 2017 at 3:56 AM, Marcelo Vanzin wrote: > Nope, I didn't tested it

[GitHub] spark issue #16387: [SPARK-18986][Core] ExternalAppendOnlyMap shouldn't fail...

Github user vanzin commented on the issue: https://github.com/apache/spark/pull/16387 > Nope, I didn't tested it in isolation. Could you do that? To make sure that it's really caused by this change? --- If your project is set up for it, you can reply to this email and have

[GitHub] spark issue #16387: [SPARK-18986][Core] ExternalAppendOnlyMap shouldn't fail...

Github user samkum commented on the issue: https://github.com/apache/spark/pull/16387 Nope, I didn't tested it in isolation. -Sameer. On Jan 24, 2017 10:09 PM, "Marcelo Vanzin" wrote: > No the question is whether you tested without

[GitHub] spark issue #16387: [SPARK-18986][Core] ExternalAppendOnlyMap shouldn't fail...

Github user vanzin commented on the issue: https://github.com/apache/spark/pull/16387 No the question is whether you tested without @viirya commit `b1ef9ec` (the last one that forces spills of in-memory maps), or just the very last version of the patch. --- If your project is set

[GitHub] spark issue #16387: [SPARK-18986][Core] ExternalAppendOnlyMap shouldn't fail...

Github user samkum commented on the issue: https://github.com/apache/spark/pull/16387 Hi, No, I havent test this with out the ExternalAppendOnlyMap fix if this is what you are asking. I tested it with changes to both the classes: TaskMemoryManager and

[GitHub] spark issue #16387: [SPARK-18986][Core] ExternalAppendOnlyMap shouldn't fail...

Github user vanzin commented on the issue: https://github.com/apache/spark/pull/16387 @samkum can you reply to the questions above? This PR is basically blocked on your feedback that it makes things worse. > @samkum did you try this patch without the "spill map on every

[GitHub] spark issue #16387: [SPARK-18986][Core] ExternalAppendOnlyMap shouldn't fail...

Github user viirya commented on the issue: https://github.com/apache/spark/pull/16387 @samkum I've expected this and your testing inspired me to change how the memory consumers are spilled in order. I've submitted a PR #16603 for this. Simply said, even I added a check and

[GitHub] spark issue #16387: [SPARK-18986][Core] ExternalAppendOnlyMap shouldn't fail...

Github user samkum commented on the issue: https://github.com/apache/spark/pull/16387 The fix is better but still produces visible GC issues. After removing the fix, GC is no longer a problem.

[GitHub] spark issue #16387: [SPARK-18986][Core] ExternalAppendOnlyMap shouldn't fail...

Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/16387 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 does not have this feature

[GitHub] spark issue #16387: [SPARK-18986][Core] ExternalAppendOnlyMap shouldn't fail...

Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/16387 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/71429/ Test PASSed. ---

[GitHub] spark issue #16387: [SPARK-18986][Core] ExternalAppendOnlyMap shouldn't fail...

Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/16387 **[Test build #71429 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/71429/testReport)** for PR 16387 at commit

[GitHub] spark issue #16387: [SPARK-18986][Core] ExternalAppendOnlyMap shouldn't fail...

Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/16387 **[Test build #71429 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/71429/testReport)** for PR 16387 at commit

[GitHub] spark issue #16387: [SPARK-18986][Core] ExternalAppendOnlyMap shouldn't fail...

Github user viirya commented on the issue: https://github.com/apache/spark/pull/16387 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 this feature enabled and wishes so,

[GitHub] spark issue #16387: [SPARK-18986][Core] ExternalAppendOnlyMap shouldn't fail...

Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/16387 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 does not have this feature

[GitHub] spark issue #16387: [SPARK-18986][Core] ExternalAppendOnlyMap shouldn't fail...

Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/16387 Test FAILed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/71425/ Test FAILed. ---

[GitHub] spark issue #16387: [SPARK-18986][Core] ExternalAppendOnlyMap shouldn't fail...

Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/16387 **[Test build #71425 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/71425/testReport)** for PR 16387 at commit

[GitHub] spark issue #16387: [SPARK-18986][Core] ExternalAppendOnlyMap shouldn't fail...

Github user viirya commented on the issue: https://github.com/apache/spark/pull/16387 @samkum Thanks for testing this. I think it is because every time `forceSpill` is called now, it will spill the map anyway. I will add a check to only spill the map if the map is not empty. --- If

[GitHub] spark issue #16387: [SPARK-18986][Core] ExternalAppendOnlyMap shouldn't fail...

Github user samkum commented on the issue: https://github.com/apache/spark/pull/16387 I have tested this, but I found a very strange observation. GC frequency has increased many folds...and majority of the time is spend in GC. --- If your project is set up for it, you can reply to

[GitHub] spark issue #16387: [SPARK-18986][Core] ExternalAppendOnlyMap shouldn't fail...

Github user viirya commented on the issue: https://github.com/apache/spark/pull/16387 @vanzin ok, thanks for review. --- 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] spark issue #16387: [SPARK-18986][Core] ExternalAppendOnlyMap shouldn't fail...

Github user vanzin commented on the issue: https://github.com/apache/spark/pull/16387 LGTM, I'll leave it open a bit in case someone else has comments. --- 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

[GitHub] spark issue #16387: [SPARK-18986][Core] ExternalAppendOnlyMap shouldn't fail...

Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/16387 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/71208/ Test PASSed. ---

[GitHub] spark issue #16387: [SPARK-18986][Core] ExternalAppendOnlyMap shouldn't fail...

Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/16387 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 does not have this feature

[GitHub] spark issue #16387: [SPARK-18986][Core] ExternalAppendOnlyMap shouldn't fail...

Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/16387 **[Test build #71208 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/71208/testReport)** for PR 16387 at commit

[GitHub] spark issue #16387: [SPARK-18986][Core] ExternalAppendOnlyMap shouldn't fail...

Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/16387 **[Test build #71208 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/71208/testReport)** for PR 16387 at commit

[GitHub] spark issue #16387: [SPARK-18986][Core] ExternalAppendOnlyMap shouldn't fail...

Github user viirya commented on the issue: https://github.com/apache/spark/pull/16387 @vanzin Is this looking good for you now? Thanks. --- 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

[GitHub] spark issue #16387: [SPARK-18986][Core] ExternalAppendOnlyMap shouldn't fail...

Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/16387 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 does not have this feature

[GitHub] spark issue #16387: [SPARK-18986][Core] ExternalAppendOnlyMap shouldn't fail...

Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/16387 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/71072/ Test PASSed. ---

[GitHub] spark issue #16387: [SPARK-18986][Core] ExternalAppendOnlyMap shouldn't fail...

Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/16387 **[Test build #71072 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/71072/testReport)** for PR 16387 at commit

[GitHub] spark issue #16387: [SPARK-18986][Core] ExternalAppendOnlyMap shouldn't fail...

Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/16387 **[Test build #71072 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/71072/testReport)** for PR 16387 at commit

[GitHub] spark issue #16387: [SPARK-18986][Core] ExternalAppendOnlyMap shouldn't fail...

Github user viirya commented on the issue: https://github.com/apache/spark/pull/16387 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 this feature enabled and wishes so,

[GitHub] spark issue #16387: [SPARK-18986][Core] ExternalAppendOnlyMap shouldn't fail...

Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/16387 Test FAILed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/71068/ Test FAILed. ---

[GitHub] spark issue #16387: [SPARK-18986][Core] ExternalAppendOnlyMap shouldn't fail...

Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/16387 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 does not have this feature

[GitHub] spark issue #16387: [SPARK-18986][Core] ExternalAppendOnlyMap shouldn't fail...

Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/16387 **[Test build #71068 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/71068/testReport)** for PR 16387 at commit

[GitHub] spark issue #16387: [SPARK-18986][Core] ExternalAppendOnlyMap shouldn't fail...

Github user viirya commented on the issue: https://github.com/apache/spark/pull/16387 ping @rxin @zsxwing @JoshRosen @davies --- 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

[GitHub] spark issue #16387: [SPARK-18986][Core] ExternalAppendOnlyMap shouldn't fail...

Github user viirya commented on the issue: https://github.com/apache/spark/pull/16387 cc @rxin @zsxwing too --- 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

[GitHub] spark issue #16387: [SPARK-18986][Core] ExternalAppendOnlyMap shouldn't fail...

Github user viirya commented on the issue: https://github.com/apache/spark/pull/16387 cc @JoshRosen Can you take a look? Thanks. --- 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

[GitHub] spark issue #16387: [SPARK-18986][Core] ExternalAppendOnlyMap shouldn't fail...

Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/16387 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/70543/ Test PASSed. ---

[GitHub] spark issue #16387: [SPARK-18986][Core] ExternalAppendOnlyMap shouldn't fail...

Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/16387 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 does not have this feature

[GitHub] spark issue #16387: [SPARK-18986][Core] ExternalAppendOnlyMap shouldn't fail...

Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/16387 **[Test build #70543 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/70543/testReport)** for PR 16387 at commit

[GitHub] spark issue #16387: [SPARK-18986][Core] ExternalAppendOnlyMap shouldn't fail...

Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/16387 **[Test build #70543 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/70543/testReport)** for PR 16387 at commit

[GitHub] spark issue #16387: [SPARK-18986][Core] ExternalAppendOnlyMap shouldn't fail...

Github user viirya commented on the issue: https://github.com/apache/spark/pull/16387 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 this feature enabled and wishes so,

[GitHub] spark issue #16387: [SPARK-18986][Core] ExternalAppendOnlyMap shouldn't fail...

Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/16387 Test FAILed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/70538/ Test FAILed. ---

[GitHub] spark issue #16387: [SPARK-18986][Core] ExternalAppendOnlyMap shouldn't fail...

Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/16387 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 does not have this feature

[GitHub] spark issue #16387: [SPARK-18986][Core] ExternalAppendOnlyMap shouldn't fail...

Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/16387 **[Test build #70538 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/70538/testReport)** for PR 16387 at commit