-
Notifications
You must be signed in to change notification settings - Fork 199
Wip umm snappy commons #3
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
Conversation
dshirish
commented
Oct 21, 2015
- Added checks for critical up thresholds while allocating memory from MemoryStore and ShuffleMemoryManager
- For the above extended BlockManager and ShuffleMemoryManager and created SnappyBlockManager and SnappyShuffleMemoryManager
- Also sending a separate pull request for snappy-spark changes as those are in a different repository
9d21bd0 to
567caea
Compare
e07992c to
fcc775e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason to keep this in snappy-core? I think these should also be moved to snappy-tools -- both are loaded by reflection in any case from what I noticed in code below.
If this would have provided some self-contained functionality, then would have made more sense to keep in snappy-commons. As it stands now, snappy-core by itself will fail to function without the presence of snappy-tools. In particular, this is not just a jar dependency but also that it will work only for GemXD embedded mode -- something that requires snappy-tools in any case.
…re classes that check GemXD critical up events
92ead27 to
3a246b2
Compare
|
Incorporated all review changes. Also added SNAP-126. Will merge the code if there are no more comments |
|
Looks good -- please merge. Shirish, one thing to take care of will be the changes in these classes here: apache/spark#9127 These are fixed in latest upstream that also includes #9127 mentioned above. If you can go through #9127 and tell whether its not much work to get the port done then will proceed to update spark else will defer after 0.2 hoping we won't hit SPARK-10929 and SPARK-10783 |
|
Also the original PR that fixes the two mentioned issues is this: apache/spark#9241 . It refers to https://issues.apache.org/jira/browse/SPARK-10342 which might be useful to understanding if there are more things we need to be thinking about with GemXD in picture. |
|
Looks like apache/spark#9127 changes are based on the unified memory manager being implemented thru https://issues.apache.org/jira/browse/SPARK-10000. There is a design doc attached. The memory allocation in spark is being changed from strict fraction based maximums to single memory view (75% of heap by default) that will contain execution area and storage area . Storage is capped at 50% of this but can borrow from execution (if space is available) and vice a versa for execution. Storage will be evicted as more and more execution memory is needed. So looks like execution can acquire complete space. |