[jira] [Comment Edited] (YARN-8059) Resource type is ignored when FS decide to preempt

2018-11-03 Thread Szilard Nemeth (JIRA)


[ 
https://issues.apache.org/jira/browse/YARN-8059?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16674232#comment-16674232
 ] 

Szilard Nemeth edited comment on YARN-8059 at 11/3/18 10:28 PM:


The single test failure seems unrelated, as it passed for me locally (25/25 
consecutive runs)


was (Author: snemeth):
The single test failure seems unrelated, as it passed me locally (25/25 
consecutive runs)

> Resource type is ignored when FS decide to preempt
> --
>
> Key: YARN-8059
> URL: https://issues.apache.org/jira/browse/YARN-8059
> Project: Hadoop YARN
>  Issue Type: Bug
>  Components: fairscheduler
>Affects Versions: 3.0.0
>Reporter: Yufei Gu
>Assignee: Szilard Nemeth
>Priority: Major
> Attachments: YARN-8059.001.patch, YARN-8059.002.patch, 
> YARN-8059.003.patch, YARN-8059.004.patch, YARN-8059.005.patch, 
> YARN-8059.006.patch
>
>
> Method Fairscheduler#shouldAttemptPreemption doesn't consider resources other 
> than vcore and memory. We may need to rethink it in the resource type 
> scenario. cc [~miklos.szeg...@cloudera.com], [~wilfreds] and [~snemeth].
> {code}
> if (context.isPreemptionEnabled()) {
>   return (context.getPreemptionUtilizationThreshold() < Math.max(
>   (float) rootMetrics.getAllocatedMB() /
>   getClusterResource().getMemorySize(),
>   (float) rootMetrics.getAllocatedVirtualCores() /
>   getClusterResource().getVirtualCores()));
> }
> {code}



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

-
To unsubscribe, e-mail: yarn-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org



[jira] [Comment Edited] (YARN-8059) Resource type is ignored when FS decide to preempt

2018-11-03 Thread Szilard Nemeth (JIRA)


[ 
https://issues.apache.org/jira/browse/YARN-8059?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16674127#comment-16674127
 ] 

Szilard Nemeth edited comment on YARN-8059 at 11/3/18 5:50 PM:
---

Thanks [~wilfreds] and [~haibochen] for your comments!

[~wilfreds]: Indeed, it was the case with most of the testcases that memory was 
defining the number of containers and it was the dominant resource.

With the new patch I fixed this behaviour by adding 5 times the amount of 
memory for each node, making sure that the custom resource becomes dominant.

[~haibochen]: I didn't like the boolean check either so I separated the custom 
resource type testcases completely from the others. This generated a lot of 
changes, though, meaning the size of the patch is increased, but I guess we 
have either the "boolean flag-way" or "more change" way of solving the issue.

It took me a lot of effort to finally figure out what caused the tests to fail 
when the custom resource became the dominant resource, so I'm summarizing my 
findings here:

In {{FSPreemptionThread#identifyContainersToPreempt}}, the loop goes through 
the results of {{FSAppAttempt#getStarvedResourceRequests}}.
 This is all initiated periodically from the Preemption thread. 
 With some debug logs added, I realized that 
{{FSAppAttempt#getStarvedResourceRequests}} returns an empty list in too many 
cases, this means containers are not getting preempted. 
 In this method, {{numContainersThatFit}} is calculated by this call:
{code:java}
int numContainersThatFit = (int) Math.floor(
 Resources.ratio(scheduler.getResourceCalculator(),
 pending, rr.getCapability()));{code}
As {{FairScheduler.getResourceCalculator()}} simply returns an instance of 
{{DefaultResourceCalculator}} which is only checking the ratio by checking the 
memory value of the 2 Resource objects passed in. This means that custom 
resources are not checked at all!
 So the first idea was to use the {{DominantResourceCalculator}} instance of FS.
 It had some issues, too. In some cases, where the Resource's custom resource 
value was 0, a division by zero calculation happened, ruining the previous 
results of calculations of other resource types.
 So, the {{DominantResourceCalculator}} was never checking whether the value it 
divides by is zero. I think this is debatable whether this worth a new jira or 
not, but I included it in my patch.

After my fix in {{DominantResourceCalculator}}, I still had issues with the 
calculations so not all of the tests were green yet.
 There is one special case of the ratio calculation: 
 Let's suppose we have 2 Resources:
 - {{Resource a: }}
 - {{Resource b: }}

Let's take an example of a call that computes {{numContainersThatFit}} (pasted 
above):
 {{pending}} have the value of {{Resource}} and
 {{rr.getCapability()}} has the value of {{Resource}}.
 In this case, {{DominantResourceCalculator}} would return 1 because the ratio 
of the vCores is 1/1 = 1 and dividing 6144 by 10240 produces 0.6, so that 1 is 
the larger from these 2 numbers, 1 is returned. I think this is wrong in case 
of this particular (and other similar) preemption calculation, as we want to 
take all the resources into account, so the expected value should be 0 as we 
don't have enough memory in the second resource.
 I added a very simple check for this, please see the implementation of the 
newly added {{PreemptionResourceCalculator}}.

 

Please share your thoughts about the latest patch! 


was (Author: snemeth):
Thanks [~wilfreds] and [~haibochen] for your comments!

[~wilfreds]: Indeed, it was the case with most of the testcases that memory was 
defining the number of containers and it was the dominant resource.

With the new patch I fixed this behaviour by adding 5 times the amount of 
memory for each node, making sure that the custom resource becomes dominant.

[~haibochen]: I didn't like the boolean check either so I separated the custom 
resource type testcases completely from the others. This generated a lot of 
changes, though, meaning the size of the patch is increased, but I guess we 
have eithe the "boolean flag-way" or "more change" way of solving the issue.

It took me a lot of effort to finally figure out what caused the tests to fail 
when the custom resource became the dominant resource, so I'm summarizing my 
findings here:

In {{FSPreemptionThread#identifyContainersToPreempt}}, the loop goes through 
the results of {{FSAppAttempt#getStarvedResourceRequests}}.
 This is all initiated periodically from the Preemption thread. 
 With some debug logs added, I realized that 
{{FSAppAttempt#getStarvedResourceRequests}} returns an empty list in too many 
cases, this means containers are not getting preempted. 
 In this method, {{numContainersThatFit}} is calculated by this call:
{code:java}
int numContainersThatFit = (int) Math.floor(
 Resources.ratio(scheduler.getResourceCalculator(),
 

[jira] [Comment Edited] (YARN-8059) Resource type is ignored when FS decide to preempt

2018-11-03 Thread Szilard Nemeth (JIRA)


[ 
https://issues.apache.org/jira/browse/YARN-8059?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16674127#comment-16674127
 ] 

Szilard Nemeth edited comment on YARN-8059 at 11/3/18 5:21 PM:
---

Thanks [~wilfreds] and [~haibochen] for your comments!

[~wilfreds]: Indeed, it was the case with most of the testcases that memory was 
defining the number of containers and it was the dominant resource.

With the new patch I fixed this behaviour by adding 5 times the amount of 
memory for each node, making sure that the custom resource becomes dominant.

[~haibochen]: I didn't like the boolean check either so I separated the custom 
resource type testcases completely from the others. This generated a lot of 
changes, though, meaning the size of the patch is increased, but I guess we 
have eithe the "boolean flag-way" or "more change" way of solving the issue.

It took me a lot of effort to finally figure out what caused the tests to fail 
when the custom resource became the dominant resource, so I'm summarizing my 
findings here:

In {{FSPreemptionThread#identifyContainersToPreempt}}, the loop goes through 
the results of {{FSAppAttempt#getStarvedResourceRequests}}.
 This is all initiated periodically from the Preemption thread. 
 With some debug logs added, I realized that 
{{FSAppAttempt#getStarvedResourceRequests}} returns an empty list in too many 
cases, this means containers are not getting preempted. 
 In this method, {{numContainersThatFit}} is calculated by this call:
{code:java}
int numContainersThatFit = (int) Math.floor(
 Resources.ratio(scheduler.getResourceCalculator(),
 pending, rr.getCapability()));{code}
As {{FairScheduler.getResourceCalculator()}} simply returns an instance of 
{{DefaultResourceCalculator}} which is only checking the ratio by checking the 
memory value of the 2 Resource objects passed in. This means that custom 
resources are not checked at all!
 So the first idea was to use the {{DominantResourceCalculator}} instance of FS.
 It had some issues, too. In some cases, where the Resource's custom resource 
value was 0, a division by zero calculation happened, ruining the previous 
results of calculations of other resource types.
 So, the {{DominantResourceCalculator}} was never checking whether the value it 
divides by is zero. I think this is debatable whether this worth a new jira or 
not, but I included it in my patch.

After my fix in {{DominantResourceCalculator}}, I still had issues with the 
calculations so not all of the tests were green yet.
 There is one special case of the ratio calculation: 
 Let's suppose we have 2 Resources:
 - {{Resource a: }}
 - {{Resource b: }}

Let's take an example of a call that computes {{numContainersThatFit}} (pasted 
above):
 {{pending}} have the value of {{Resource}} and
 {{rr.getCapability()}} has the value of {{Resource}}.
 In this case, {{DominantResourceCalculator}} would return 1 because the ratio 
of the vCores is 1/1 = 1 and dividing 6144 by 10240 produces 0.6, so that 1 is 
the larger from these 2 numbers, 1 is returned. I think this is wrong in case 
of preemption calculation, as we want to take all the resources into account.
 I added a very simple check for this, please see the implementation of the 
newly added {{PreemptionResourceCalculator}}.

 

Please share your thoughts about the latest patch! 


was (Author: snemeth):
Thanks [~wilfreds] and [~haibochen] for your comments!

[~wilfreds]: Indeed, it was the case with most of the testcases that memory was 
defining the number of containers and it was the dominant resource.

With the new patch I fixed this behaviour by adding 5 times the amount of 
memory for each node, making sure that the custom resource becomes dominant.

[~haibochen]: I didn't like the boolean check either so I separated the custom 
resource type testcases completely from the others. This generated a lot of 
changes, though, meaning the size of the patch is increased, but I guess we 
have eithe the "boolean flag-way" or "more change" way of solving the issue.

It took me a lot of effort to finally figure out what caused the tests to fail 
when the custom resource became the dominant resource, so I'm summarizing my 
findings here:

In {{FSPreemptionThread#identifyContainersToPreempt}}, the loop goes through 
the results of {{FSAppAttempt#getStarvedResourceRequests}}.
 This is all initiated periodically from the Preemption thread. 
 With some debug logs added, I realized that 
{{FSAppAttempt#getStarvedResourceRequests}} returns an empty list in too many 
cases, this means containers are not getting preempted. 
 In this method, {{numContainersThatFit}} is calculated by this call:
{code:java}
int numContainersThatFit = (int) Math.floor(
 Resources.ratio(scheduler.getResourceCalculator(),
 pending, rr.getCapability()));{code}
As {{FairScheduler.getResourceCalculator()}} simply returns an instance of 

[jira] [Comment Edited] (YARN-8059) Resource type is ignored when FS decide to preempt

2018-11-03 Thread Szilard Nemeth (JIRA)


[ 
https://issues.apache.org/jira/browse/YARN-8059?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16674127#comment-16674127
 ] 

Szilard Nemeth edited comment on YARN-8059 at 11/3/18 5:20 PM:
---

Thanks [~wilfreds] and [~haibochen] for your comments!

[~wilfreds]: Indeed, it was the case with most of the testcases that memory was 
defining the number of containers and it was the dominant resource.

With the new patch I fixed this behaviour by adding 5 times the amount of 
memory for each node, making sure that the custom resource becomes dominant.

[~haibochen]: I didn't like the boolean check either so I separated the custom 
resource type testcases completely from the others. This generated a lot of 
changes, though, meaning the size of the patch is increased, but I guess we 
have eithe the "boolean flag-way" or "more change" way of solving the issue.

It took me a lot of effort to finally figure out what caused the tests to fail 
when the custom resource became the dominant resource, so I'm summarizing my 
findings here:

In {{FSPreemptionThread#identifyContainersToPreempt}}, the loop goes through 
the results of {{FSAppAttempt#getStarvedResourceRequests}}.
 This is all initiated periodically from the Preemption thread. 
 With some debug logs added, I realized that 
{{FSAppAttempt#getStarvedResourceRequests}} returns an empty list in too many 
cases, this means containers are not getting preempted. 
 In this method, {{numContainersThatFit}} is calculated by this call:
{code:java}
int numContainersThatFit = (int) Math.floor(
 Resources.ratio(scheduler.getResourceCalculator(),
 pending, rr.getCapability()));{code}
As {{FairScheduler.getResourceCalculator()}} simply returns an instance of 
{{DefaultResourceCalculator}} which is only checking the ratio by checking the 
memory value of the 2 Resource objects passed in. This means that custom 
resources are not checked at all!
 So the first idea was to use the {{DominantResourceCalculator}} instance of FS.
 It had some issues, too. In some cases, where the Resource's custom resource 
value was null, a division by zero calculation happened, ruining the previous 
results of calculations of other resource types.
 So, the {{DominantResourceCalculator}} was never checking whether the value it 
divides by is zero. I think this is debatable whether this worth a new jira or 
not, but I included it in my patch.

After my fix in {{DominantResourceCalculator}}, I still had issues with the 
calculations so not all of the tests were green yet.
 There is one special case of the ratio calculation: 
 Let's suppose we have 2 Resources:
 - {{Resource a: }}
 - {{Resource b: }}

Let's take an example of a call that computes {{numContainersThatFit}} (pasted 
above):
 {{pending}} have the value of {{Resource}} and
 {{rr.getCapability()}} has the value of {{Resource}}.
 In this case, {{DominantResourceCalculator}} would return 1 because the ratio 
of the vCores is 1/1 = 1 and dividing 6144 by 10240 produces 0.6, so that 1 is 
the larger from these 2 numbers, 1 is returned. I think this is wrong in case 
of preemption calculation, as we want to take all the resources into account.
 I added a very simple check for this, please see the implementation of the 
newly added {{PreemptionResourceCalculator}}.

 

Please share your thoughts about the latest patch! 


was (Author: snemeth):
Thanks [~wilfreds] and [~haibochen] for your comments!

[~wilfreds]: Indeed, it was the case with most of the testcases that memory was 
defining the number of containers and it was the dominant resource.

With the new patch I fixed this behaviour by adding 5 times the amount of 
memory for each node, making sure that the custom resource becomes dominant.

[~haibochen]: I didn't like the boolean check either so I separated the custom 
resource type testcases completely from the others. This generated a lot of 
changes, though, meaning the size of the patch is increased, but I guess we 
have eithe the "boolean flag-way" or "more change" way of solving the issue.

It took me a lot of effort to finally figure out what caused the tests to fail 
when the custom resource became the dominant resource, so I'm summarizing my 
findings here:


In \{{FSPreemptionThread#identifyContainersToPreempt}}, the loop goes through 
the results of \{{FSAppAttempt#getStarvedResourceRequests}}.
This is all initiated periodically from the Preemption thread. 
With some debug logs added, I realized that 
\{{FSAppAttempt#getStarvedResourceRequests}} returns an empty list in too many 
cases, this means containers are not getting preempted. 
In this method, \{{numContainersThatFit}} is calculated by this call: {{ int 
numContainersThatFit = (int) Math.floor(
 Resources.ratio(scheduler.getResourceCalculator(),
 pending, rr.getCapability()));}}
As \{{FairScheduler.getResourceCalculator()}} simply returns an instance of