Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/3629
---
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
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/3629#issuecomment-76864875
Ok LGTM I'm merging into master finally. Thanks @suyanNone and
@liyezhang556520 for uncovering this tricky issue.
---
If your project is set up for it, you can reply
Github user suyanNone commented on the pull request:
https://github.com/apache/spark/pull/3629#issuecomment-76649122
ping @andrewor14
---
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 user suyanNone commented on the pull request:
https://github.com/apache/spark/pull/3629#issuecomment-76369781
@andrewor14
If put it at the end of `accountingLock.synchronized`, as I described
above,
```
accountingLock.synchronized {
1. ensureFreeSpace()
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3629#issuecomment-76370289
[Test build #28065 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/28065/consoleFull)
for PR 3629 at commit
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/3629#issuecomment-76382309
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3629#issuecomment-76382296
[Test build #28065 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/28065/consoleFull)
for PR 3629 at commit
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/3629#discussion_r25490289
--- Diff: core/src/main/scala/org/apache/spark/storage/MemoryStore.scala ---
@@ -469,10 +485,32 @@ private[spark] class MemoryStore(blockManager:
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/3629#discussion_r25490244
--- Diff: core/src/main/scala/org/apache/spark/storage/MemoryStore.scala ---
@@ -381,6 +395,8 @@ private[spark] class MemoryStore(blockManager:
Github user suyanNone commented on a diff in the pull request:
https://github.com/apache/spark/pull/3629#discussion_r25421661
--- Diff: core/src/main/scala/org/apache/spark/storage/MemoryStore.scala ---
@@ -328,6 +330,7 @@ private[spark] class MemoryStore(blockManager:
Github user suyanNone commented on the pull request:
https://github.com/apache/spark/pull/3629#issuecomment-76324070
@andrewor14 , Already refine according comments, please review~
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3629#issuecomment-76324144
[Test build #28042 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/28042/consoleFull)
for PR 3629 at commit
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3629#issuecomment-76328927
[Test build #28042 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/28042/consoleFull)
for PR 3629 at commit
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/3629#issuecomment-76328933
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/3629#issuecomment-75179521
@suyanNone @liyezhang556520 sorry for slipping on this again. I just took a
close look at the patch again and I am convinced that it does the right thing.
I will
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/3629#discussion_r25046378
--- Diff: core/src/main/scala/org/apache/spark/storage/MemoryStore.scala ---
@@ -468,11 +474,18 @@ private[spark] class MemoryStore(blockManager:
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/3629#discussion_r25046037
--- Diff: core/src/main/scala/org/apache/spark/storage/MemoryStore.scala ---
@@ -283,12 +284,13 @@ private[spark] class MemoryStore(blockManager:
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/3629#issuecomment-75181856
@suyanNone I have left mostly documentation and code style comments. The
only slightly less trivial thing is to revert the `pending` flag added to
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/3629#discussion_r25045986
--- Diff: core/src/main/scala/org/apache/spark/storage/MemoryStore.scala ---
@@ -46,6 +46,7 @@ private[spark] class MemoryStore(blockManager:
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/3629#discussion_r25046324
--- Diff: core/src/main/scala/org/apache/spark/storage/MemoryStore.scala ---
@@ -328,6 +330,7 @@ private[spark] class MemoryStore(blockManager:
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/3629#discussion_r25046107
--- Diff: core/src/main/scala/org/apache/spark/storage/MemoryStore.scala ---
@@ -283,12 +284,13 @@ private[spark] class MemoryStore(blockManager:
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/3629#discussion_r25046147
--- Diff: core/src/main/scala/org/apache/spark/storage/MemoryStore.scala ---
@@ -46,6 +46,7 @@ private[spark] class MemoryStore(blockManager:
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/3629#issuecomment-70208700
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3629#issuecomment-70208695
[Test build #25633 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/25633/consoleFull)
for PR 3629 at commit
Github user suyanNone commented on the pull request:
https://github.com/apache/spark/pull/3629#issuecomment-70077674
@andrewor14 @liyezhang556520
yes, because in current memorystore, it keep a `previousMemoryReserved` for
current Thread, that var is design to reserved the unroll
Github user suyanNone commented on the pull request:
https://github.com/apache/spark/pull/3629#issuecomment-70203787
@andrewor14
I will still releasePendingUnrollMemory() before [this
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3629#issuecomment-70204945
[Test build #25633 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/25633/consoleFull)
for PR 3629 at commit
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/3629#issuecomment-69654114
To summarize, it seems that the problem can be explained in the following
way. The following is the sequence of function calls if `unrollSafely`
successfully unrolls
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/3629#issuecomment-69653790
after unroll, may the block can put in memoryStore, it will return a
unrolled data(array), and then call blockManager.putArray, it will finally to
call
Github user liyezhang556520 commented on the pull request:
https://github.com/apache/spark/pull/3629#issuecomment-69684345
Hi @andrewor14 , I think @suyanNone 's concern is for code on
Github user suyanNone commented on the pull request:
https://github.com/apache/spark/pull/3629#issuecomment-69527640
@andrewor14
`shouldn't we release the pending memory after we actually put the block
(i.e. after this line), not before?`
Agree. I think [this
Github user liyezhang556520 commented on the pull request:
https://github.com/apache/spark/pull/3629#issuecomment-69528544
Hi @andrewor14 , I think @suyanNone is correct about the case in
`CacheManager`. And this issue is solve in the same way in
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3629#issuecomment-69529992
[Test build #25395 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/25395/consoleFull)
for PR 3629 at commit
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/3629#issuecomment-69529997
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3629#issuecomment-69528177
[Test build #25395 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/25395/consoleFull)
for PR 3629 at commit
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/3629#issuecomment-69422290
I see, thanks for your detailed explanations @suyanNone @liyezhang556520.
If the problem is that we double count after we put the block in memory,
shouldn't we also
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/3629#issuecomment-69423665
Also, the other issue with this patch is that `unrollSafely` is not used
exclusively with `tryToPut`; it is also used in
`CacheManager#putInBlockManager`. If we
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/3629#issuecomment-69281334
Sorry, I don't understand what you're saying. Why will there be double
counting? Can you list the steps that causes this? (What is Value A?)
---
If your project is
Github user suyanNone commented on the pull request:
https://github.com/apache/spark/pull/3629#issuecomment-69301441
@andrewor14 sorry for my poor english, and @liyezhang556520 has explained
well.
now, I just talk the situation if just remove the memory release in
Github user liyezhang556520 commented on the pull request:
https://github.com/apache/spark/pull/3629#issuecomment-69282553
Hi @andrewor14 , I think @suyanNone 's explain is correct. What @suyanNone
wants to say that if we just remove the [memory
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/3629#issuecomment-69122238
@suyanNone Having taken a detailed look over the patch, I believe the
correct thing to do here is to just remove the [memory
Github user suyanNone commented on the pull request:
https://github.com/apache/spark/pull/3629#issuecomment-69127678
@andrewor14
Hi, yes, but the valueA (unrollmemoryForThisThread) can't used by others
even after this value already added to memoryStore part. it will be double
Github user suyanNone closed the pull request at:
https://github.com/apache/spark/pull/3629
---
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
Github user suyanNone commented on the pull request:
https://github.com/apache/spark/pull/3629#issuecomment-67126072
It is already resolved in [SPARK-3000][CORE] drop old blocks to disk in
parallel when free memory is not enough for caching new blocks #2134
---
If your project is
Github user liyezhang556520 commented on the pull request:
https://github.com/apache/spark/pull/3629#issuecomment-67126547
Hi @suyanNone , you don't need to close this PR, since
[#2134](https://github.com/apache/spark/pull/2134) is not merged yet. And the
bug still exists in current
Github user suyanNone commented on the pull request:
https://github.com/apache/spark/pull/3629#issuecomment-67127309
Reopen
---
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 user suyanNone commented on the pull request:
https://github.com/apache/spark/pull/3629#issuecomment-67127268
@liyezhang556520 I not familiar with the process about pull request = =,
ok, I will reopen it...
---
If your project is set up for it, you can reply to this email
GitHub user suyanNone reopened a pull request:
https://github.com/apache/spark/pull/3629
[SPARK-4777][CORE] Some block memory after unrollSafely not count into used
memory(memoryStore.entrys or unrollMemory)
Some memory not count into memory used by memoryStore or unrollMemory.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3629#issuecomment-67127610
[Test build #24491 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/24491/consoleFull)
for PR 3629 at commit
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/3629#issuecomment-67133754
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3629#issuecomment-67133743
[Test build #24491 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/24491/consoleFull)
for PR 3629 at commit
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/3629#issuecomment-66588176
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3629#issuecomment-66588161
[Test build #24352 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/24352/consoleFull)
for PR 3629 at commit
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/3629#issuecomment-66529851
add to whitelist
---
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 user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3629#issuecomment-66530100
[Test build #24321 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/24321/consoleFull)
for PR 3629 at commit
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3629#issuecomment-66530403
[Test build #24321 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/24321/consoleFull)
for PR 3629 at commit
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/3629#issuecomment-66530405
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3629#issuecomment-66560672
[Test build #24345 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/24345/consoleFull)
for PR 3629 at commit
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/3629#issuecomment-66564884
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3629#issuecomment-66564876
[Test build #24345 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/24345/consoleFull)
for PR 3629 at commit
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/3629#issuecomment-66581493
[Test build #24352 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/24352/consoleFull)
for PR 3629 at commit
GitHub user suyanNone opened a pull request:
https://github.com/apache/spark/pull/3629
[SPARK-4777][CORE] Some block memory after unrollSafely not count into used
memory(memoryStore.entrys or unrollMemory)
Some memory not count into memory used by memoryStore or unrollMemory.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/3629#issuecomment-65894783
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
63 matches
Mail list logo
