[TESTBUG] runtime/containers/docker/TestCPUAwareness.java failed in docker not supporting --cpus
Bob Vandette
bob.vandette at oracle.com
Tue Feb 5 15:25:24 UTC 2019
You change has been integrated.
Bob.
> On Feb 3, 2019, at 10:46 PM, Ao Qi <aoqi at loongson.cn> wrote:
>
> Hi Bob,
>
> I added reviewer information:
> http://cr.openjdk.java.net/~aoqi/8217597/webrev.03/
>
> I used hg export, so I create all the information needed to push into
> jdk/jdk according to my understanding. If there is something wrong
> about these info, it's ok to adjust. Thanks, Bob.
>
> On Mon, Feb 4, 2019 at 2:24 AM Bob Vandette <bob.vandette at oracle.com> wrote:
>>
>> Sure. Can you send me a patch ready to hg import and I’ll submit for you.
>>
>> Bob.
>>
>>
>>> On Feb 3, 2019, at 12:20 PM, Ao Qi <aoqi at loongson.cn> wrote:
>>>
>>> Hi,
>>>
>>> Bob Vandette and David Holmes have reviewed. Could someone help to sponsor?
>>>
>>> Thanks,
>>> Ao Qi
>>>
>>>> On Fri, Feb 1, 2019 at 6:52 PM Ao Qi <aoqi at loongson.cn> wrote:
>>>>
>>>>> On Fri, Feb 1, 2019 at 11:58 AM David Holmes <david.holmes at oracle.com> wrote:
>>>>>
>>>>> Reviewed.
>>>>>
>>>>
>>>> Thanks, David.
>>>>
>>>>> I assume you also need a sponsor?
>>>>
>>>> Yes :)
>>>>
>>>>>
>>>>> Thanks,
>>>>> David
>>>>>
>>>>>> On 1/02/2019 11:56 am, Ao Qi wrote:
>>>>>> Thanks, Bob. You help a lot.
>>>>>>
>>>>>> Could someone help to review this small change?
>>>>>>
>>>>>>> On Fri, Feb 1, 2019 at 12:30 AM Bob Vandette <bob.vandette at oracle.com> wrote:
>>>>>>>
>>>>>>> I’m not a “R” reviewer but I’m ok with your change.
>>>>>>>
>>>>>>> Bob.
>>>>>>>
>>>>>>>
>>>>>>>> On Jan 31, 2019, at 11:00 AM, Ao Qi <aoqi at loongson.cn> wrote:
>>>>>>>>
>>>>>>>> I leave the change, and update copyright year:
>>>>>>>> http://cr.openjdk.java.net/~aoqi/8217597/webrev.02/ Could you help to
>>>>>>>> review this?
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Ao Qi
>>>>>>>>
>>>>>>>>> On Thu, Jan 31, 2019 at 10:08 PM Bob Vandette <bob.vandette at oracle.com> wrote:
>>>>>>>>>
>>>>>>>>> Yes, that does provide a bit more unique testing so leave the change as you had it.
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> Bob.
>>>>>>>>>
>>>>>>>>>> On Jan 28, 2019, at 8:41 PM, Ao Qi <aoqi at loongson.cn> wrote:
>>>>>>>>>>
>>>>>>>>>> Hi Bob,
>>>>>>>>>>
>>>>>>>>>> Thanks! I am not a containers expert and have one small question. The
>>>>>>>>>> maximum amount of --cpus is 4 (equivalent to both setting —cpu-period
>>>>>>>>>> and —cpu-quota) in the already existing test. Is it valuable to keep
>>>>>>>>>> testCpus(i, i) according to the max num of available CPUs? If not, I
>>>>>>>>>> would also prefer removing the lines. In addition, I think I forgot to
>>>>>>>>>> update the copyright year, it will be fixed in the next version of
>>>>>>>>>> wevrev.
>>>>>>>>>>
>>>>>>>>>>> On Mon, Jan 28, 2019 at 11:35 PM Bob Vandette <bob.vandette at oracle.com> wrote:
>>>>>>>>>>>
>>>>>>>>>>> There is already a test that verifies —cpu-period and —cpu-quota.
>>>>>>>>>>> I would just remove these lines.
>>>>>>>>>>>
>>>>>>>>>>> 62 // leave one CPU for system and tools, otherwise this test may be unstable
>>>>>>>>>>> 63 int maxNrOfAvailableCpus = availableCPUs - 1;
>>>>>>>>>>> 64 for (int i=1; i < maxNrOfAvailableCpus; i = i * 2) {
>>>>>>>>>>> 65 testCpus(i, i);
>>>>>>>>>>> 66 }
>>>>>>>>>>>
>>>>>>>>>>> 129 private static void testCpus(int valueToSet, int expectedTraceValue) throws Exception {
>>>>>>>>>>> 130 Common.logNewTestCase("test cpus: " + valueToSet);
>>>>>>>>>>> 131 DockerRunOptions opts = Common.newOpts(imageName)
>>>>>>>>>>> 132 .addDockerOpts("--cpus", "" + valueToSet);
>>>>>>>>>>> 133 Common.run(opts)
>>>>>>>>>>> 134 .shouldMatch("active_processor_count.*" + expectedTraceValue);
>>>>>>>>>>> 135 }
>>>>>>>>>>>
>>>>>>>>>>> Bob.
>>>>>>>>>>>
>>>>>>>>>>> On Jan 28, 2019, at 10:22 AM, Ao Qi <aoqi at loongson.cn> wrote:
>>>>>>>>>>>
>>>>>>>>>>> Hi,
>>>>>>>>>>>
>>>>>>>>>>> Since —cpus is a shortcut for of setting both --cpu-period and
>>>>>>>>>>> --cpu-quota and the test is not intended to verify that docker works
>>>>>>>>>>> correctly, I did not check the docker version and just replaced
>>>>>>>>>>> setting --cpus with setting both --cpu-period and --cpu-quota. What do
>>>>>>>>>>> you think of this patch:
>>>>>>>>>>> http://cr.openjdk.java.net/~aoqi/8217597/webrev.01/ ?
>>>>>>>>>>>
>>>>>>>>>>> Cheers,
>>>>>>>>>>> Ao Qi
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> On Wed, Jan 23, 2019 at 11:37 PM Bob Vandette <bob.vandette at oracle.com> wrote:
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Since —cpus is just a shortcut for of setting both --cpu-period and --cpu-quota”, I’d
>>>>>>>>>>> be ok with removing this test. The tests are intended to test the container/cgroup
>>>>>>>>>>> configuration detection logic and not to verify that docker works correctly.
>>>>>>>>>>>
>>>>>>>>>>> An alternate solution would be to add version detection to the Docker test check in
>>>>>>>>>>> DockerTestUtils.java . We already exec “docker ps” to see if docker is available
>>>>>>>>>>> and enabled.
>>>>>>>>>>>
>>>>>>>>>>> % docker --version
>>>>>>>>>>> Docker version 17.03.1-ce, build 276fd32
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Bob.
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> On Jan 22, 2019, at 10:19 PM, Ao Qi <aoqi at loongson.cn> wrote:
>>>>>>>>>>>
>>>>>>>>>>> On Wed, Jan 23, 2019 at 10:55 AM David Holmes <david.holmes at oracle.com> wrote:
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> On 23/01/2019 11:58 am, Ao Qi wrote:
>>>>>>>>>>>
>>>>>>>>>>> Hi David,
>>>>>>>>>>>
>>>>>>>>>>> On Wed, Jan 23, 2019 at 5:24 AM David Holmes <david.holmes at oracle.com> wrote:
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> cc'ing Bob as our containers expert ...
>>>>>>>>>>>
>>>>>>>>>>> On 23/01/2019 1:10 am, Ao Qi wrote:
>>>>>>>>>>>
>>>>>>>>>>> Hi,
>>>>>>>>>>>
>>>>>>>>>>> --cpus is available in Docker 1.13 and higher [1], so
>>>>>>>>>>> runtime/containers/docker/TestCPUAwareness.java failed in docker which
>>>>>>>>>>> does not support --cpus.
>>>>>>>>>>>
>>>>>>>>>>> Webrev: http://cr.openjdk.java.net/~aoqi/docker/webrev.00/
>>>>>>>>>>>
>>>>>>>>>>> This patch skips the test if --cpus is not supported. I tested
>>>>>>>>>>> runtime/containers/docker/TestCPUAwareness.java on a Fedora 25 (Docker
>>>>>>>>>>> version 1.12.6, build ae7d637/1.12.6, not supporting --cpus) and
>>>>>>>>>>> Ubuntu 16.04 (Docker version 17.03.2-ce, build f5ec1e2, supporting
>>>>>>>>>>> --cpus)
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> The patch causes the test to pass if launching Docker fails for any
>>>>>>>>>>> reason so that is not good.
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> I tested two versions of docker which does not support --cpus. Their
>>>>>>>>>>> exit values when using --cpus are 2 and 125, and outputs are:
>>>>>>>>>>>
>>>>>>>>>>> flag provided but not defined: --cpus
>>>>>>>>>>> See 'docker run --help'.
>>>>>>>>>>>
>>>>>>>>>>> and
>>>>>>>>>>>
>>>>>>>>>>> unknown flag: --cpus
>>>>>>>>>>> See 'docker run --help'.
>>>>>>>>>>>
>>>>>>>>>>> My initial thought was that the else condition of
>>>>>>>>>>> "output.getExitValue() == 0" should match the condition of "--cpus not
>>>>>>>>>>> supported". Firstly I used output.shouldMatch("docker run --help"),
>>>>>>>>>>> but I am not sure if all the docker version behaves this way when
>>>>>>>>>>> --cpus is not supported and "docker run --help" does not certainly
>>>>>>>>>>> indicate "--cpus not supported", so I removed the else condition.
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> I think we need to try and find a way to clearly identifyt eh failing
>>>>>>>>>>> condition. Is there are "docker --version" we coudl check?
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> I will do more research. Checking docker version may be one option,
>>>>>>>>>>> and checking whether one option is support by docker may be also one
>>>>>>>>>>> option. I will try them later, while waiting if there are some other
>>>>>>>>>>> opinions :)
>>>>>>>>>>>
>>>>>>>>>>> I am not sure if this is a testbug, so I did not file it on JBS. In
>>>>>>>>>>> fact, I am not quite sure what kind of issue can be filed on JBS. Is
>>>>>>>>>>> there any guidance document?
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Any/all issues can be filed on JBS. You don't need to pre-classify as a
>>>>>>>>>>> testbug, simple create an issue that a test is failing under specific
>>>>>>>>>>> conditions. Whomever works on the bug will then determine whether it is
>>>>>>>>>>> a testbug or product issue or something else. (We don't seem to have any
>>>>>>>>>>> docs on using JBS ...)
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> What if the issue is not a bug or no body cares the issue? The issue
>>>>>>>>>>> will be open on JBS forever?
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Possibly :) But each component team performs regular triage of the bugs
>>>>>>>>>>> that get filed and eventually things will be examined enough to see if
>>>>>>>>>>> they are indeed a bug, and if not they will be closed as not an issue.
>>>>>>>>>>> If a bug but low priority it may eventually get closed as "will not fix"
>>>>>>>>>>> just to keep the open bug count down.
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> I was a little afraid that filing issues that are not bugs or nobody
>>>>>>>>>>> cares would increase the workload of others and frustrate myself, so I
>>>>>>>>>>> was not sure what kind of issue should be filed. Now I basically
>>>>>>>>>>> clear, thanks David.
>>>>>>>>>>>
>>>>>>>>>>> Cheers,
>>>>>>>>>>> David
>>>>>>>>>>>
>>>>>>>>>>> Thanks for your explanation, and I filed this issue on JBS:
>>>>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8217597
>>>>>>>>>>>
>>>>>>>>>>> In this case I'm not sure whether we require a docker version that
>>>>>>>>>>> supports --cpus, and the test should be skipped otherwise. Though
>>>>>>>>>>> ideally this would involve an explicit version check so we don't just
>>>>>>>>>>> pass if the docker process fails.
>>>>>>>>>>>
>>>>>>>>>>> Thanks,
>>>>>>>>>>> David
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Cheers,
>>>>>>>>>>> Ao Qi
>>>>>>>>>>>
>>>>>>>>>>> [1] https://docs.docker.com/config/containers/resource_constraints/#cpu
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>
>>>>>>>
>>
More information about the hotspot-runtime-dev
mailing list