RFR: 8197589: Update CPU count algorithm when both cpu shares and quotas are used

[Bob Vandette]

Please review the updated webrev for changes that alter the CPU count selection on Linux
based systems with cgroups enabled.

I’ve update the bug synopsis to make it clearer that this is an update to the previous algorithm.

The new algorithm provides two modes to allow the most flexibility. If the flag PreferContainerQuotaForCPUCount
is set to true (default), take the cpu quota value, if provided, without exceeding the number of physical cpus on the
system. If the flag PreferContainerQuotaForCPUCount is false, use the minimum of cpu shares or cpu quotas, if set,
without exceeding the number of physical cpus in the system. If only one of cpu shares or cpu quotas is provided,
then use the specified value limited by the number of physical processors in the system. 

BUG:

https://bugs.openjdk.java.net/browse/JDK-8197589

WEBREV:

http://cr.openjdk.java.net/~bobv/8197589/webrev.01


Bob.


> On Feb 13, 2018, at 5:44 PM, David Holmes <David.Holmes at oracle.com> wrote:
> 
> On 14/02/2018 12:33 AM, Bob Vandette wrote:
>>> On Feb 12, 2018, at 10:34 PM, David Holmes <david.holmes at oracle.com> wrote:
>>> 
>>> PS. Bob you will need to file a CSR request for this change in behaviour. But I think you're going to have to make this selectable somehow.
>>> 
>> I’ll file a CSR but I’m not sure that it’s really necessary.  I’m not adding/changing any flags and the behavior documented and
>> implemented in the RFE JDK-8146115 is not changing.  The original behavior focused on setting CPUs based on the container
>> having only one or the other cpu options set.  This is still the way the new algorithm works.  The only change is that
>> we are now considering the impact of having both shares and quotas set.
> 
> I don't agree with that description. You previously chose the minimum of quotas/shares and now you choose the maximum. That's a significant change in behaviour that could impact existing container deployments.
> 
>> As for being selectable, I’d really like to avoid adding yet another obscure hotspot flag for this.  There are many ways to
>> achieve the current desired behavior.  Simply alter the k8s/docker options to avoid setting both shares and quotas and set
>> the one you want.  The other option is to specify the -XX:ActiveProcessorCount.  Given that Kubernetes, the most popular
>> container orchestration tool, is documenting the shares+quota option for developers leads me to want to make the new
>> algorithm the default behavior.
> 
> But the issue is NOT how Kubernetes uses shares+quota, it is how that information is to be interpreted by the JVM. Kubernetes defines a "request" and a "limit" - with the limit being higher than the request to allow for bursty CPU usage. The question then is how the JVM attempts to guess the number of processors to report: does it use the "limit" or the "request"? Current behaviour is it uses the "request"; the new behaviour is it uses the "limit". Neither is more correct than the other. Maybe it should use the average? There's no correct interpretation here - the JVM doesn't know how the user wants these values to be treated.
> 
> David
> 
>> Bob.
>>> David
>>> 
>>> On 13/02/2018 12:16 PM, David Holmes wrote:
>>>> On 13/02/2018 12:04 PM, Bob Vandette wrote:
>>>>>> On Feb 12, 2018, at 8:01 PM, David Holmes <david.holmes at oracle.com> wrote:
>>>>>> 
>>>>>> Hi Bob,
>>>>>> 
>>>>>> On 13/02/2018 5:53 AM, Bob Vandette wrote:
>>>>>>> Updated webrev URL.
>>>>>>>> On Feb 12, 2018, at 2:41 PM, Bob Vandette <bob.vandette at oracle.com> wrote:
>>>>>>>> 
>>>>>>>> Please review this change to the cpu count selection logic used when running in docker containers.
>>>>>>>> 
>>>>>>>> BUG:
>>>>>>>> 
>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8197589
>>>>>>>> 
>>>>>>>> WEBREV:
>>>>>>>> 
>>>>>>>> http://cr.openjdk.java.net/~bobv/8197589/webrev.00/
>>>>>>>> 
>>>>>>>> The algorithm implemented in JDK-8146115 for selecting the number of active cpus assumed that a container
>>>>>>>> would not be configured to use both cpu shares AND cpu quotas at the same time. This was an invalid assumption
>>>>>>>> since this how containers are configured to allow bursting activity [1].  A Java process running in a container with
>>>>>>>> cpu shares set to the equivalent of 2 cpus and a quota set to 6, will end up configuring the VM for 2 CPUs rather
>>>>>>>> than 6.
>>>>>>>> 
>>>>>>>> [1] https://kubernetes.io/docs/tasks/configure-pod-container/assign-cpu-resource/#motivation-for-cpu-requests-and-limits
>>>>>> 
>>>>>> This really makes very little sense to me. They seem to be confusing utilization with number of physical processors that are available to use. The end result seems somewhat random to me. The number of threads the VM creates in response to the expected number of CPUs it has available is only indirectly related to the load those threads may then impose on the system. But I would have thought you would try to size the VM's use of threads based on the "request" not the "limit”.
>>>>> 
>>>>> If we chose the request (which is the way it works before this change), then we risk under utilizing the available CPU resources.
>>>>> Since there’s no risk with over utilizing, I think the new algorithm is a better choice.
>>>> So "limit" doesn't mean limit? I would expect there to be a real risk with over utilising.
>>>> The change does what you want so in that sense consider it reviewed. But as to whether what it does is reasonable ... I can easily imagine someone else would want the current behaviour, just as readily as someone wants the new behvaiour.
>>>> David
>>>>> Bob.
>>>>> 
>>>>> 
>>>>>> 
>>>>>> David
>>>>>> 
>>>>>>>> 
>>>>>>>> Bob.
>>>>>>>> 
>>>>>>>> 
>>>>>