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

classic Classic list List threaded Threaded
13 messages Options
Reply | Threaded
Open this post in threaded view
|

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

Mikhailo Seledtsov
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

Reply | Threaded
Open this post in threaded view
|

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

David Holmes
Seems okay.

Thanks,
David
-----

On 29/11/2017 10:50 AM, mikhailo 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
>
Reply | Threaded
Open this post in threaded view
|

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

Bob Vandette
In reply to this post by Mikhailo Seledtsov
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 <[hidden email]> 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
>

Reply | Threaded
Open this post in threaded view
|

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

Mikhailo Seledtsov
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.

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?


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 <[hidden email]> 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
>>

Reply | Threaded
Open this post in threaded view
|

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

Bob Vandette

> On Nov 29, 2017, at 4:10 PM, mikhailo <[hidden email]> 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.

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 <[hidden email]> 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
>>>
>

Reply | Threaded
Open this post in threaded view
|

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

Mikhailo Seledtsov

On 11/29/2017 01:29 PM, Bob Vandette wrote:

>> On Nov 29, 2017, at 4:10 PM, mikhailo <[hidden email]> 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 <[hidden email]> 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
>>>>

Reply | Threaded
Open this post in threaded view
|

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

Mikhailo Seledtsov
Here is an updated webrev:

     http://cr.openjdk.java.net/~mseledtsov/8191943.01/


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
>>> <[hidden email]> 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
>>>>> <[hidden email]> 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
>>>>>
>

Reply | Threaded
Open this post in threaded view
|

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

Bob Vandette
I think you’ll  have problems on a single CPU box since the —cpuset-cpus argument to docker will most likely fail when
you try to set it to “0,1”.

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

Bob.


> On Nov 29, 2017, at 7:08 PM, mikhailo <[hidden email]> wrote:
>
> Here is an updated webrev:
>
>     http://cr.openjdk.java.net/~mseledtsov/8191943.01/
>
>
> 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 <[hidden email]> 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 <[hidden email]> 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
>>>>>>
>>
>

Reply | Threaded
Open this post in threaded view
|

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

David Holmes
In reply to this post by Mikhailo Seledtsov
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
>>>> <[hidden email]> 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
>>>>>> <[hidden email]> 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
>>>>>>
>>
>
Reply | Threaded
Open this post in threaded view
|

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

Mikhailo Seledtsov
Bob, David,

   Thank you for review.

    Here is an updated webrev addressing your comments:

     http://cr.openjdk.java.net/~mseledtsov/8191943.02/


Misha


On 11/29/2017 09:28 PM, David Holmes wrote:
> 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);
I introduced a proper check for this.
>
> 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:
I changed code to output both the original expected APC and the adjusted
APC.
>
> 155         Common.logNewTestCase("test cpu shares, shares = " + shares);
>
> Having the same check duplicated three times is a bit messy, but not
> horrendously so. :)
Introduced a common method to conditionally adjust APC.

Thank you,
Misha

>
> 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
>>>>> <[hidden email]> 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
>>>>>>> <[hidden email]> 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
>>>>>>>
>>>
>>

Reply | Threaded
Open this post in threaded view
|

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

Bob Vandette
The Cpus_allowed_list need Linux 2.6.24 but that shouldn’t be a big problem.  We’ll only skip some
of the tests on older systems.  Not sure docker would run well on older kernels anyway.

Looks good,
Bob.

> On Nov 30, 2017, at 3:01 PM, mikhailo <[hidden email]> wrote:
>
> Bob, David,
>
>   Thank you for review.
>
>    Here is an updated webrev addressing your comments:
>
>     http://cr.openjdk.java.net/~mseledtsov/8191943.02/
>
>
> Misha
>
>
> On 11/29/2017 09:28 PM, David Holmes wrote:
>> 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);
> I introduced a proper check for this.
>>
>> 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:
> I changed code to output both the original expected APC and the adjusted APC.
>>
>> 155         Common.logNewTestCase("test cpu shares, shares = " + shares);
>>
>> Having the same check duplicated three times is a bit messy, but not horrendously so. :)
> Introduced a common method to conditionally adjust APC.
>
> Thank you,
> Misha
>>
>> 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 <[hidden email]> 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 <[hidden email]> 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
>>>>>>>>
>>>>
>>>
>

Reply | Threaded
Open this post in threaded view
|

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

David Holmes
In reply to this post by Mikhailo Seledtsov
Seems okay.

Thanks,
David

On 1/12/2017 6:01 AM, mikhailo wrote:

> Bob, David,
>
>    Thank you for review.
>
>     Here is an updated webrev addressing your comments:
>
>      http://cr.openjdk.java.net/~mseledtsov/8191943.02/
>
>
> Misha
>
>
> On 11/29/2017 09:28 PM, David Holmes wrote:
>> 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);
> I introduced a proper check for this.
>>
>> 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:
> I changed code to output both the original expected APC and the adjusted
> APC.
>>
>> 155         Common.logNewTestCase("test cpu shares, shares = " + shares);
>>
>> Having the same check duplicated three times is a bit messy, but not
>> horrendously so. :)
> Introduced a common method to conditionally adjust APC.
>
> Thank you,
> Misha
>>
>> 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
>>>>>> <[hidden email]> 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
>>>>>>>> <[hidden email]> 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
>>>>>>>>
>>>>
>>>
>
Reply | Threaded
Open this post in threaded view
|

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

Mikhailo Seledtsov
Bob, David,

Thank you for review. I will run final testing and then integrate.


Misha


On 11/30/2017 05:10 PM, David Holmes wrote:

> Seems okay.
>
> Thanks,
> David
>
> On 1/12/2017 6:01 AM, mikhailo wrote:
>> Bob, David,
>>
>>    Thank you for review.
>>
>>     Here is an updated webrev addressing your comments:
>>
>>      http://cr.openjdk.java.net/~mseledtsov/8191943.02/
>>
>>
>> Misha
>>
>>
>> On 11/29/2017 09:28 PM, David Holmes wrote:
>>> 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);
>> I introduced a proper check for this.
>>>
>>> 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:
>> I changed code to output both the original expected APC and the
>> adjusted APC.
>>>
>>> 155         Common.logNewTestCase("test cpu shares, shares = " +
>>> shares);
>>>
>>> Having the same check duplicated three times is a bit messy, but not
>>> horrendously so. :)
>> Introduced a common method to conditionally adjust APC.
>>
>> Thank you,
>> Misha
>>>
>>> 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
>>>>>>> <[hidden email]> 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
>>>>>>>>> <[hidden email]> 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
>>>>>>>>>
>>>>>
>>>>
>>