[jira] [Commented] (FLINK-8212) Pull EnvironmentInformation out of TaskManagerServices

2018-02-15 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/FLINK-8212?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16365366#comment-16365366
 ] 

ASF GitHub Bot commented on FLINK-8212:
---

Github user asfgit closed the pull request at:

https://github.com/apache/flink/pull/5458


> Pull EnvironmentInformation out of TaskManagerServices
> --
>
> Key: FLINK-8212
> URL: https://issues.apache.org/jira/browse/FLINK-8212
> Project: Flink
>  Issue Type: Improvement
>  Components: Local Runtime, Network
>Affects Versions: 1.5.0
>Reporter: Till Rohrmann
>Assignee: mingleizhang
>Priority: Major
> Fix For: 1.5.0
>
>
> We should pull the {{EnvironmentInformation}} out of the 
> {{TaskManagerServices}} where it is used to get access to the memory settings 
> of the executing JVM. This unnecessarily couples the former with the latter 
> and makes testing extremely hard (one has to use {{PowerMockRunner}} and mock 
> the static {{EnvironmentInformation}}).
> When addressing this issue, then we should also refactor 
> {{NetworkBufferCalculationTest}}.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (FLINK-8212) Pull EnvironmentInformation out of TaskManagerServices

2018-02-14 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/FLINK-8212?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16364076#comment-16364076
 ] 

ASF GitHub Bot commented on FLINK-8212:
---

Github user zhangminglei commented on a diff in the pull request:

https://github.com/apache/flink/pull/5458#discussion_r168180788
  
--- Diff: 
flink-runtime/src/main/java/org/apache/flink/runtime/taskexecutor/TaskManagerServices.java
 ---
@@ -530,10 +538,12 @@ public static long calculateNetworkBufferMemory(long 
totalJavaMemorySize, Config
 * .
 *
 * @param tmConfig task manager services configuration object
+* @param maxJvmHeapMemory the maximum JVM heap size
 *
 * @return memory to use for network buffers (in bytes)
 */
-   public static long 
calculateNetworkBufferMemory(TaskManagerServicesConfiguration tmConfig) {
+   public static long 
calculateNetworkBufferMemory(TaskManagerServicesConfiguration tmConfig,
+   
long maxJvmHeapMemory) {
--- End diff --

Will fix ~ 


> Pull EnvironmentInformation out of TaskManagerServices
> --
>
> Key: FLINK-8212
> URL: https://issues.apache.org/jira/browse/FLINK-8212
> Project: Flink
>  Issue Type: Improvement
>  Components: Local Runtime, Network
>Affects Versions: 1.5.0
>Reporter: Till Rohrmann
>Assignee: mingleizhang
>Priority: Major
> Fix For: 1.5.0
>
>
> We should pull the {{EnvironmentInformation}} out of the 
> {{TaskManagerServices}} where it is used to get access to the memory settings 
> of the executing JVM. This unnecessarily couples the former with the latter 
> and makes testing extremely hard (one has to use {{PowerMockRunner}} and mock 
> the static {{EnvironmentInformation}}).
> When addressing this issue, then we should also refactor 
> {{NetworkBufferCalculationTest}}.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (FLINK-8212) Pull EnvironmentInformation out of TaskManagerServices

2018-02-14 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/FLINK-8212?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16364074#comment-16364074
 ] 

ASF GitHub Bot commented on FLINK-8212:
---

Github user zhangminglei commented on a diff in the pull request:

https://github.com/apache/flink/pull/5458#discussion_r168180756
  
--- Diff: 
flink-runtime/src/main/java/org/apache/flink/runtime/taskexecutor/TaskManagerServices.java
 ---
@@ -270,10 +273,14 @@ public static TaskManagerServices fromConfiguration(
 * Creates a {@link MemoryManager} from the given {@link 
TaskManagerServicesConfiguration}.
 *
 * @param taskManagerServicesConfiguration to create the memory manager 
from
+* @param freeHeapMemoryWithDefrag an estimate of the size of the free 
heap memory
+* @param maxJvmHeapMemory the maximum JVM heap size
 * @return Memory manager
 * @throws Exception
 */
-   private static MemoryManager 
createMemoryManager(TaskManagerServicesConfiguration 
taskManagerServicesConfiguration) throws Exception {
+   private static MemoryManager 
createMemoryManager(TaskManagerServicesConfiguration 
taskManagerServicesConfiguration,
+   
 long freeHeapMemoryWithDefrag,
+   
 long maxJvmHeapMemory) throws Exception {
--- End diff --

Will fix ~


> Pull EnvironmentInformation out of TaskManagerServices
> --
>
> Key: FLINK-8212
> URL: https://issues.apache.org/jira/browse/FLINK-8212
> Project: Flink
>  Issue Type: Improvement
>  Components: Local Runtime, Network
>Affects Versions: 1.5.0
>Reporter: Till Rohrmann
>Assignee: mingleizhang
>Priority: Major
> Fix For: 1.5.0
>
>
> We should pull the {{EnvironmentInformation}} out of the 
> {{TaskManagerServices}} where it is used to get access to the memory settings 
> of the executing JVM. This unnecessarily couples the former with the latter 
> and makes testing extremely hard (one has to use {{PowerMockRunner}} and mock 
> the static {{EnvironmentInformation}}).
> When addressing this issue, then we should also refactor 
> {{NetworkBufferCalculationTest}}.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (FLINK-8212) Pull EnvironmentInformation out of TaskManagerServices

2018-02-14 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/FLINK-8212?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16364011#comment-16364011
 ] 

ASF GitHub Bot commented on FLINK-8212:
---

Github user tillrohrmann commented on a diff in the pull request:

https://github.com/apache/flink/pull/5458#discussion_r168171985
  
--- Diff: 
flink-runtime/src/main/java/org/apache/flink/runtime/taskexecutor/TaskManagerServices.java
 ---
@@ -270,10 +273,14 @@ public static TaskManagerServices fromConfiguration(
 * Creates a {@link MemoryManager} from the given {@link 
TaskManagerServicesConfiguration}.
 *
 * @param taskManagerServicesConfiguration to create the memory manager 
from
+* @param freeHeapMemoryWithDefrag an estimate of the size of the free 
heap memory
+* @param maxJvmHeapMemory the maximum JVM heap size
 * @return Memory manager
 * @throws Exception
 */
-   private static MemoryManager 
createMemoryManager(TaskManagerServicesConfiguration 
taskManagerServicesConfiguration) throws Exception {
+   private static MemoryManager 
createMemoryManager(TaskManagerServicesConfiguration 
taskManagerServicesConfiguration,
+   
 long freeHeapMemoryWithDefrag,
+   
 long maxJvmHeapMemory) throws Exception {
--- End diff --

The convention is to break parameter lists like
```
public void a(
A a,
B b) {
// foobar
}
```


> Pull EnvironmentInformation out of TaskManagerServices
> --
>
> Key: FLINK-8212
> URL: https://issues.apache.org/jira/browse/FLINK-8212
> Project: Flink
>  Issue Type: Improvement
>  Components: Local Runtime, Network
>Affects Versions: 1.5.0
>Reporter: Till Rohrmann
>Priority: Major
> Fix For: 1.5.0
>
>
> We should pull the {{EnvironmentInformation}} out of the 
> {{TaskManagerServices}} where it is used to get access to the memory settings 
> of the executing JVM. This unnecessarily couples the former with the latter 
> and makes testing extremely hard (one has to use {{PowerMockRunner}} and mock 
> the static {{EnvironmentInformation}}).
> When addressing this issue, then we should also refactor 
> {{NetworkBufferCalculationTest}}.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (FLINK-8212) Pull EnvironmentInformation out of TaskManagerServices

2018-02-14 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/FLINK-8212?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16364012#comment-16364012
 ] 

ASF GitHub Bot commented on FLINK-8212:
---

Github user tillrohrmann commented on a diff in the pull request:

https://github.com/apache/flink/pull/5458#discussion_r168172098
  
--- Diff: 
flink-runtime/src/main/java/org/apache/flink/runtime/taskexecutor/TaskManagerServices.java
 ---
@@ -530,10 +538,12 @@ public static long calculateNetworkBufferMemory(long 
totalJavaMemorySize, Config
 * .
 *
 * @param tmConfig task manager services configuration object
+* @param maxJvmHeapMemory the maximum JVM heap size
 *
 * @return memory to use for network buffers (in bytes)
 */
-   public static long 
calculateNetworkBufferMemory(TaskManagerServicesConfiguration tmConfig) {
+   public static long 
calculateNetworkBufferMemory(TaskManagerServicesConfiguration tmConfig,
+   
long maxJvmHeapMemory) {
--- End diff --

Same here with the parameters


> Pull EnvironmentInformation out of TaskManagerServices
> --
>
> Key: FLINK-8212
> URL: https://issues.apache.org/jira/browse/FLINK-8212
> Project: Flink
>  Issue Type: Improvement
>  Components: Local Runtime, Network
>Affects Versions: 1.5.0
>Reporter: Till Rohrmann
>Priority: Major
> Fix For: 1.5.0
>
>
> We should pull the {{EnvironmentInformation}} out of the 
> {{TaskManagerServices}} where it is used to get access to the memory settings 
> of the executing JVM. This unnecessarily couples the former with the latter 
> and makes testing extremely hard (one has to use {{PowerMockRunner}} and mock 
> the static {{EnvironmentInformation}}).
> When addressing this issue, then we should also refactor 
> {{NetworkBufferCalculationTest}}.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (FLINK-8212) Pull EnvironmentInformation out of TaskManagerServices

2018-02-14 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/FLINK-8212?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16363878#comment-16363878
 ] 

ASF GitHub Bot commented on FLINK-8212:
---

Github user zhangminglei commented on the issue:

https://github.com/apache/flink/pull/5458
  
@tillrohrmann I updated the code. Could you take a look ? Thanks ~


> Pull EnvironmentInformation out of TaskManagerServices
> --
>
> Key: FLINK-8212
> URL: https://issues.apache.org/jira/browse/FLINK-8212
> Project: Flink
>  Issue Type: Improvement
>  Components: Local Runtime, Network
>Affects Versions: 1.5.0
>Reporter: Till Rohrmann
>Priority: Major
> Fix For: 1.5.0
>
>
> We should pull the {{EnvironmentInformation}} out of the 
> {{TaskManagerServices}} where it is used to get access to the memory settings 
> of the executing JVM. This unnecessarily couples the former with the latter 
> and makes testing extremely hard (one has to use {{PowerMockRunner}} and mock 
> the static {{EnvironmentInformation}}).
> When addressing this issue, then we should also refactor 
> {{NetworkBufferCalculationTest}}.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (FLINK-8212) Pull EnvironmentInformation out of TaskManagerServices

2018-02-14 Thread mingleizhang (JIRA)

[ 
https://issues.apache.org/jira/browse/FLINK-8212?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16363685#comment-16363685
 ] 

mingleizhang commented on FLINK-8212:
-

Thanks [~till.rohrmann] I will correct the code again. 

> Pull EnvironmentInformation out of TaskManagerServices
> --
>
> Key: FLINK-8212
> URL: https://issues.apache.org/jira/browse/FLINK-8212
> Project: Flink
>  Issue Type: Improvement
>  Components: Local Runtime, Network
>Affects Versions: 1.5.0
>Reporter: Till Rohrmann
>Priority: Major
> Fix For: 1.5.0
>
>
> We should pull the {{EnvironmentInformation}} out of the 
> {{TaskManagerServices}} where it is used to get access to the memory settings 
> of the executing JVM. This unnecessarily couples the former with the latter 
> and makes testing extremely hard (one has to use {{PowerMockRunner}} and mock 
> the static {{EnvironmentInformation}}).
> When addressing this issue, then we should also refactor 
> {{NetworkBufferCalculationTest}}.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (FLINK-8212) Pull EnvironmentInformation out of TaskManagerServices

2018-02-12 Thread Till Rohrmann (JIRA)

[ 
https://issues.apache.org/jira/browse/FLINK-8212?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16360577#comment-16360577
 ] 

Till Rohrmann commented on FLINK-8212:
--

Hi [~mingleizhang], I think the proper way to solve this problem is to pass in 
the required information to the {{fromConfiguration}} method.

> Pull EnvironmentInformation out of TaskManagerServices
> --
>
> Key: FLINK-8212
> URL: https://issues.apache.org/jira/browse/FLINK-8212
> Project: Flink
>  Issue Type: Improvement
>  Components: Local Runtime, Network
>Affects Versions: 1.5.0
>Reporter: Till Rohrmann
>Priority: Major
> Fix For: 1.5.0
>
>
> We should pull the {{EnvironmentInformation}} out of the 
> {{TaskManagerServices}} where it is used to get access to the memory settings 
> of the executing JVM. This unnecessarily couples the former with the latter 
> and makes testing extremely hard (one has to use {{PowerMockRunner}} and mock 
> the static {{EnvironmentInformation}}).
> When addressing this issue, then we should also refactor 
> {{NetworkBufferCalculationTest}}.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (FLINK-8212) Pull EnvironmentInformation out of TaskManagerServices

2018-02-12 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/FLINK-8212?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16360482#comment-16360482
 ] 

ASF GitHub Bot commented on FLINK-8212:
---

GitHub user zhangminglei opened a pull request:

https://github.com/apache/flink/pull/5458

[FLINK-8212] [network] Pull EnvironmentInformation out of TaskManager…

## What is the purpose of the change
Pull EnvironmentInformation out of TaskManagerServices


## Brief change log
Add inner class ```TaskExecutorEnvironmentInformation``` that can give 
access to the execution environment of the JVM.

## Verifying this change
Run ```NetworkBufferCalculationTest``` is OKay.

## Does this pull request potentially affect one of the following parts:

  - Dependencies (does it add or upgrade a dependency): (no)
  - The public API, i.e., is any changed class annotated with 
`@Public(Evolving)`: (no)
  - The serializers: ( no)
  - The runtime per-record code paths (performance sensitive): (  don't 
know)
  - Anything that affects deployment or recovery: JobManager (and its 
components), Checkpointing, Yarn/Mesos, ZooKeeper: ( no )
  - The S3 file system connector: (no)

## Documentation

  - Does this pull request introduce a new feature? ( no)
  - If yes, how is the feature documented? (not documented)


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

$ git pull https://github.com/zhangminglei/flink flink-8212

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

https://github.com/apache/flink/pull/5458.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 #5458


commit a4792758d3620359f9f42c3bcffbe7578bba4048
Author: zhangminglei 
Date:   2018-02-12T09:21:42Z

[FLINK-8212] [network] Pull EnvironmentInformation out of 
TaskManagerServices




> Pull EnvironmentInformation out of TaskManagerServices
> --
>
> Key: FLINK-8212
> URL: https://issues.apache.org/jira/browse/FLINK-8212
> Project: Flink
>  Issue Type: Improvement
>  Components: Local Runtime, Network
>Affects Versions: 1.5.0
>Reporter: Till Rohrmann
>Priority: Major
> Fix For: 1.5.0
>
>
> We should pull the {{EnvironmentInformation}} out of the 
> {{TaskManagerServices}} where it is used to get access to the memory settings 
> of the executing JVM. This unnecessarily couples the former with the latter 
> and makes testing extremely hard (one has to use {{PowerMockRunner}} and mock 
> the static {{EnvironmentInformation}}).
> When addressing this issue, then we should also refactor 
> {{NetworkBufferCalculationTest}}.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (FLINK-8212) Pull EnvironmentInformation out of TaskManagerServices

2018-02-11 Thread mingleizhang (JIRA)

[ 
https://issues.apache.org/jira/browse/FLINK-8212?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16359861#comment-16359861
 ] 

mingleizhang commented on FLINK-8212:
-

I can directly move those methods to {{TaskManagerServices}} class and then 
refactor {{NetworkBufferCalculationTest}}. But I am not sure whether it is a 
better way to do that. 

> Pull EnvironmentInformation out of TaskManagerServices
> --
>
> Key: FLINK-8212
> URL: https://issues.apache.org/jira/browse/FLINK-8212
> Project: Flink
>  Issue Type: Improvement
>  Components: Local Runtime, Network
>Affects Versions: 1.5.0
>Reporter: Till Rohrmann
>Priority: Major
> Fix For: 1.5.0
>
>
> We should pull the {{EnvironmentInformation}} out of the 
> {{TaskManagerServices}} where it is used to get access to the memory settings 
> of the executing JVM. This unnecessarily couples the former with the latter 
> and makes testing extremely hard (one has to use {{PowerMockRunner}} and mock 
> the static {{EnvironmentInformation}}).
> When addressing this issue, then we should also refactor 
> {{NetworkBufferCalculationTest}}.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (FLINK-8212) Pull EnvironmentInformation out of TaskManagerServices

2018-02-10 Thread mingleizhang (JIRA)

[ 
https://issues.apache.org/jira/browse/FLINK-8212?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16359833#comment-16359833
 ] 

mingleizhang commented on FLINK-8212:
-

Hi, [~till.rohrmann] I have a question. If we do not use 
{{EnvironmentInformation}} to get {{getSizeOfFreeHeapMemoryWithDefrag}} and 
{{getMaxJvmHeapMemory}}. Is there another way to get those two guys information 
? Thanks.

> Pull EnvironmentInformation out of TaskManagerServices
> --
>
> Key: FLINK-8212
> URL: https://issues.apache.org/jira/browse/FLINK-8212
> Project: Flink
>  Issue Type: Improvement
>  Components: Local Runtime, Network
>Affects Versions: 1.5.0
>Reporter: Till Rohrmann
>Priority: Major
> Fix For: 1.5.0
>
>
> We should pull the {{EnvironmentInformation}} out of the 
> {{TaskManagerServices}} where it is used to get access to the memory settings 
> of the executing JVM. This unnecessarily couples the former with the latter 
> and makes testing extremely hard (one has to use {{PowerMockRunner}} and mock 
> the static {{EnvironmentInformation}}).
> When addressing this issue, then we should also refactor 
> {{NetworkBufferCalculationTest}}.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)