RFR(XS): 8191943: [TESTBUG] docker/TestCPUAwareness fails on machine with 2 CPUs

[David Holmes]

On 30/11/2017 10:08 AM, mikhailo wrote:
> Here is an updated webrev:
> 
>      http://cr.openjdk.java.net/~mseledtsov/8191943.01/

As Bob noted you may still need the guard here:

   55             testAPCCombo("0,1", 200*1000, 100*1000, 4*1024, 2);
   56             testAPCCombo("0,1", 200*1000, 100*1000, 1*1024, 2);

though perhaps docker ignores invalid cpu-set values?

161         Common.logNewTestCase("expectedAPC = " + expectedAPC);

That should still be a System.out.println - you already set the test 
case prior:

155         Common.logNewTestCase("test cpu shares, shares = " + shares);

Having the same check duplicated three times is a bit messy, but not 
horrendously so. :)

Thanks,
David

> 
> Misha
> 
> 
> On 11/29/2017 01:32 PM, mikhailo wrote:
>>
>> On 11/29/2017 01:29 PM, Bob Vandette wrote:
>>>> On Nov 29, 2017, at 4:10 PM, mikhailo 
>>>> <mikhailo.seledtsov at oracle.com> wrote:
>>>>
>>>> Hi Bob,
>>>>
>>>> The failure was caused by invoking this test case:
>>>>
>>>>      testCpuShares(4096, 4);
>>>>      The test case sets --cpu-shares to 4096, expects active 
>>>> processor count to be 4; ==> actual APC is 2.
>>> Ahh, that makes sense.  I thought docker was complaining due to the 
>>> arguments being passed going
>>> beyond the available cpus.
>>>
>>>> Command:
>>>>
>>>>      docker run --tty=true --rm --cpu-shares=4096 
>>>> jdk-internal:test-cpu /jdk/bin/java -Xlog:os+container=trace -version
>>>>
>>>> Detailed log is attached at:
>>>>
>>>> https://bugs.openjdk.java.net/secure/attachment/73820/test-case-failure-cpu-shares-4096.txt 
>>>>
>>>>
>>>>
>>>> Once I saw this issue, I decided to put limits on other test cases 
>>>> based on the number of processors available on a machine, just to be 
>>>> on a safe side.
>>>>
>>>> Perhaps a better approach here is to change the expected APC inside 
>>>> the testCpuShares() to be no greater than max number of available 
>>>> processors?
>>> Yes, the selection algorithm returns the smallest number of computed 
>>> shares, quotas or active processors.  Using this approach, you’d 
>>> actually be validating the algorithm more precisely.
>> OK. I will update the tests to do this kind of validation, and post a 
>> new webrev.
>>
>>
>> Misha
>>>
>>> Bob.
>>>
>>>
>>>>
>>>> Misha
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>> On 11/29/2017 06:02 AM, Bob Vandette wrote:
>>>>> Misha,
>>>>>
>>>>> Which specific subtest failed on a 2 cpu system?
>>>>>
>>>>> I don’t see any subtests that should have failed.  You should be able
>>>>> to pass any quota, period and share value to docker independent of the
>>>>> number of CPUs.  testCpus and testAPCCombo don’t try to test more 
>>>>> than 2 cpus.
>>>>>
>>>>> I’m ok with the change but just wondering why the guards are 
>>>>> necessary.
>>>>>
>>>>> Bob.
>>>>>
>>>>>> On Nov 28, 2017, at 7:50 PM, mikhailo 
>>>>>> <mikhailo.seledtsov at oracle.com> wrote:
>>>>>>
>>>>>> Could you, please, review this simple fix? It limits test cases in 
>>>>>> TestCPUAwareness.java
>>>>>> based on the number of processors available.
>>>>>>
>>>>>>      JBS: https://bugs.openjdk.java.net/browse/JDK-8191943
>>>>>>      Webrev: http://cr.openjdk.java.net/~mseledtsov/8191943.00/
>>>>>>      Testing:
>>>>>>          1. Locally: exercised the affected test locally on Linux-x64
>>>>>>          2. Exercised the affected test on 2 processor machine
>>>>>>          3. Exercise the affected test via automated distributed 
>>>>>> test system
>>>>>>             In progress
>>>>>>
>>>>>> Misha
>>>>>>
>>
>