[GitHub] incubator-flink pull request: [FLINK-934] Remove default values fo...

[zentol]

GitHub user uce opened a pull request:

    https://github.com/apache/incubator-flink/pull/14

    [FLINK-934] Remove default values for JVM heap sizes in startup scripts

    This is a fix for [FLINK-934](https://issues.apache.org/jira/browse/FLINK-934). As a follow up we should have the heap config keys only as suggested entries (commented out) in [FLINK-898](https://issues.apache.org/jira/browse/FLINK-898) aka #6.
   
    With `jobmanager.heap.mb` and `taskmanager.heap.mb` not set, 16 GB of main memory and Java 7.
   
    ```bash
    $ ./bin/start-local.sh
    Starting job manager
    $ jps
    9601 JobManager
    $ jinfo -flag MaxHeapSize 9601
    -XX:MaxHeapSize=4294967296
    ```
   
    JobManager with 4294967296/2^20 = 4096 MB heap size.
   
    ---
   
    ```bash
    $ bin/start-cluster.sh
    Starting job manager
    Starting task manager on host batman
    $ jps
    10049 TaskManager
    9966 JobManager
    $ jinfo -flag MaxHeapSize 9966
    -XX:MaxHeapSize=4294967296
    $jinfo -flag MaxHeapSize 10049
    -XX:MaxHeapSize=4294967296
    ```
    Both JobManager and TaskManager with 4294967296/2^20 = 4096 MB heap size each.
   
    ---
   
    Setting `jobmanager.heap.mb` and `taskmanager.heap.mb` to 256 and 512 MB respectively.
   
    ```bash
    $ bin/start-local.sh
    Starting job manager
    $ jps
    10542 JobManager
    $ jinfo -flag MaxHeapSize 10542
    -XX:MaxHeapSize=805306368
    ```
   
    JobManager with 805306368/2^20 = 768 MB (256 for the JobManager and 512 MB for the TaskManager).
   
    ---
   
    ```bash
    $ bin/start-cluster.sh
    Starting job manager
    Starting task manager on host batman
    $ jps
    10833 JobManager
    10918 TaskManager
    $ jinfo -flag MaxHeapSize 10833
    -XX:MaxHeapSize=268435456
    $jinfo -flag MaxHeapSize 10918
    -XX:MaxHeapSize=536870912
    ```
    JobManager with 268435456/2^20 = 256 MB and TaskManager with 536870912/2^20 = 512 MB.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/uce/incubator-flink FLINK-934

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/incubator-flink/pull/14.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #14
   
----
commit f239909c57d16ad109767f9342f18b314fd412e4
Author: uce <[hidden email]>
Date:   2014-06-12T19:08:06Z

    [FLINK-934] Remove default values for JobManager and TaskManager JVM heap sizes in startup scripts

----


---
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 enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[Robert Metzger]

Testing Github <--> ML integration.


On Thu, Jun 12, 2014 at 9:19 PM, uce <[hidden email]> wrote:

> GitHub user uce opened a pull request:
>
>     https://github.com/apache/incubator-flink/pull/14
>
>     [FLINK-934] Remove default values for JVM heap sizes in startup scripts
>
>     This is a fix for [FLINK-934](
> https://issues.apache.org/jira/browse/FLINK-934). As a follow up we
> should have the heap config keys only as suggested entries (commented out)
> in [FLINK-898](https://issues.apache.org/jira/browse/FLINK-898) aka #6.
>
>     With `jobmanager.heap.mb` and `taskmanager.heap.mb` not set, 16 GB of
> main memory and Java 7.
>
>     ```bash
>     $ ./bin/start-local.sh
>     Starting job manager
>     $ jps
>     9601 JobManager
>     $ jinfo -flag MaxHeapSize 9601
>     -XX:MaxHeapSize=4294967296
>     ```
>
>     JobManager with 4294967296/2^20 = 4096 MB heap size.
>
>     ---
>
>     ```bash
>     $ bin/start-cluster.sh
>     Starting job manager
>     Starting task manager on host batman
>     $ jps
>     10049 TaskManager
>     9966 JobManager
>     $ jinfo -flag MaxHeapSize 9966
>     -XX:MaxHeapSize=4294967296
>     $jinfo -flag MaxHeapSize 10049
>     -XX:MaxHeapSize=4294967296
>     ```
>     Both JobManager and TaskManager with 4294967296/2^20 = 4096 MB heap
> size each.
>
>     ---
>
>     Setting `jobmanager.heap.mb` and `taskmanager.heap.mb` to 256 and 512
> MB respectively.
>
>     ```bash
>     $ bin/start-local.sh
>     Starting job manager
>     $ jps
>     10542 JobManager
>     $ jinfo -flag MaxHeapSize 10542
>     -XX:MaxHeapSize=805306368
>     ```
>
>     JobManager with 805306368/2^20 = 768 MB (256 for the JobManager and
> 512 MB for the TaskManager).
>
>     ---
>
>     ```bash
>     $ bin/start-cluster.sh
>     Starting job manager
>     Starting task manager on host batman
>     $ jps
>     10833 JobManager
>     10918 TaskManager
>     $ jinfo -flag MaxHeapSize 10833
>     -XX:MaxHeapSize=268435456
>     $jinfo -flag MaxHeapSize 10918
>     -XX:MaxHeapSize=536870912
>     ```
>     JobManager with 268435456/2^20 = 256 MB and TaskManager with
> 536870912/2^20 = 512 MB.
>
> You can merge this pull request into a Git repository by running:
>
>     $ git pull https://github.com/uce/incubator-flink FLINK-934
>
> Alternatively you can review and apply these changes as the patch at:
>
>     https://github.com/apache/incubator-flink/pull/14.patch
>
> To close this pull request, make a commit to your master/trunk branch
> with (at least) the following in the commit message:
>
>     This closes #14
>
> ----
> commit f239909c57d16ad109767f9342f18b314fd412e4
> Author: uce <[hidden email]>
> Date:   2014-06-12T19:08:06Z
>
>     [FLINK-934] Remove default values for JobManager and TaskManager JVM
> heap sizes in startup scripts
>
> ----
>
>
> ---
> 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 enabled but not working, please
> contact infrastructure at [hidden email] or file a JIRA ticket
> with INFRA.
> ---
>

[zentol]

Github user StephanEwen commented on the pull request:

    https://github.com/apache/incubator-flink/pull/14#issuecomment-45997121
 
    Okay, that looks good. The other flags (max perm gen size, mark sweep GC, mark sweep class unloading) are unaffected by that?


---
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 enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[zentol]

Github user StephanEwen commented on the pull request:

    https://github.com/apache/incubator-flink/pull/14#issuecomment-45997219
 
    When we pass the flags, we make sure that Xms and -Xmx are the same, to prevent the JVM from being funny and resizing the heap from time to time. I think we loose that with this option. But I guess its all right, on cluster setups we would expect those to be set anyways.


---
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 enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[zentol]

Github user uce commented on the pull request:

    https://github.com/apache/incubator-flink/pull/14#issuecomment-45997673
 
    Yes, the other flags are set as before. The `Xms` and `Xmx` flags are only set, when they are found in `stratosphere-conf.yaml`.
   
    While doing the PR, I came to the same conclusion as your second comment. But we can actually also do something more fancy and figure out the available memory ourselves and set it (thereby circumventing the defaults).


---
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 enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[zentol]

Github user StephanEwen commented on the pull request:

    https://github.com/apache/incubator-flink/pull/14#issuecomment-45998342
 
    Let's not do this ourselves. I think it is fine. If you want a robust setup, set the values. Our YARN scripts also set them automatically, based on the container size, afaik.


---
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 enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[zentol]

Github user StephanEwen commented on the pull request:

    https://github.com/apache/incubator-flink/pull/14#issuecomment-45998358
 
    I will merge this...


---
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 enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[zentol]

Github user asfgit closed the pull request at:

    https://github.com/apache/incubator-flink/pull/14


---
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 enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---