[PING2!] RFR: 8230305: Cgroups v2: Container awareness

[David Holmes]

No objections from me.

Thanks,
David

On 17/01/2020 6:08 am, Severin Gehwolf wrote:
> Hi Bob, David,
> 
> On Thu, 2020-01-16 at 09:13 -0500, Bob Vandette wrote:
>>> On Jan 15, 2020, at 5:13 PM, David Holmes <david.holmes at oracle.com> wrote:
>>>
>>> Hi Bob, Severin,
>>>
>>> On 15/01/2020 6:04 am, Bob Vandette wrote:
>>>> Cgroup V2 is about to go mainstream this year for popular distros such as Oracle Linux 8, Redhat Linux 8 and Fedora
>>>> so this fix it’s important to get into JDK 15 so we can start shaking out this support.
>>>> Please take a look and help get this change reviewed.
>>>
>>> I've taken a look and the overall structure and approach seems fine. I can't attest to the details of cgroups v1 versus v2 but trust the exports.
>>>
>>> Reviewed.
>>>
>>> Please ensure all copyrights updated to 2020.
>>>
>>> Bob: I assume you have tested this on our systems?
>>
>> I’m in the process of setting up a cgroup v2 system to test with but I’ll grab the patch and verify the
>> container tests on my cgroup v1 setup today.
> 
> Final webrev I intend to push tomorrow for this:
> http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8230305/07/webrev/
> 
> This passed jdk/submit and testing on cgroups v1 hotspot docker tests.
> On cgroups v2 this fails the following test since it also tests the
> OperatingSystemMXBean's memory awareness which uses core-libs Metrics
> code not updated with this webrev (passes all others):
> 
> hotspot/jtreg/containers/docker/TestMemoryAwareness.java
> 
> It's caused by JDK-8226575 which went in during the time I've started
> this and intented push time. This test failure will be fixed with JDK-
> 8231111 once I get another review from a Reviewer. I doubt many people
> will notice though since cgroupv2 enabled systems aren't that
> widespread just yet.
> 
> Please let me know if there are any objections.
> 
> Thanks,
> Severin
> 
> 
>> Bob.
>>
>>
>>> Thanks,
>>> David
>>>
>>>> Thanks,
>>>> Bob Vandette
>>>>> On Nov 29, 2019, at 4:04 AM, Severin Gehwolf <sgehwolf at redhat.com> wrote:
>>>>>
>>>>> On Fri, 2019-11-15 at 17:56 +0100, Severin Gehwolf wrote:
>>>>>> On Fri, 2019-11-08 at 15:21 +0100, Severin Gehwolf wrote:
>>>>>>> Hi Bob,
>>>>>>>
>>>>>>> On Wed, 2019-11-06 at 10:47 +0100, Severin Gehwolf wrote:
>>>>>>>> On Tue, 2019-11-05 at 16:54 -0500, Bob Vandette wrote:
>>>>>>>>> Severin,
>>>>>>>>>
>>>>>>>>> Thanks for taking on this cgroup v2 improvement.
>>>>>>>>>
>>>>>>>>> In general I like the implementation and the refactoring.  The CachedMetric class is nice.
>>>>>>>>> We can add any metric we want to cache in a more general way.
>>>>>>>>>
>>>>>>>>> Is this the latest version of the webrev?
>>>>>>>>>
>>>>>>>>> http://cr.openjdk.java.net/~sgehwolf/webrevs/cgroupsv2-hotspot/03/webrev/src/hotspot/os/linux/cgroupV2Subsystem_linux.cpp.html
>>>>>>>>>
>>>>>>>>> It looks like you need to add the caching support for active_processor_count (JDK-8227006).
>>>>>>> [...]
>>>>>>>> I'll do a proper rebase ASAP.
>>>>>>>
>>>>>>> Latest webrev:
>>>>>>> http://cr.openjdk.java.net/~sgehwolf/webrevs/cgroupsv2-hotspot/05/webrev/
>>>>>>>
>>>>>>>>> I’m not sure it’s worth providing different strings for Unlimited versus Max or Scaled shares.
>>>>>>>>> I’d just try to be compatible with the cgroupv2 output so you don’t have to change the test.
>>>>>>>>
>>>>>>>> OK. Will do.
>>>>>>>
>>>>>>> Unfortunately, there is no way of NOT changing TestCPUAwareness.java as
>>>>>>> it expects CPU Shares to be written to the cgroup filesystem verbatim.
>>>>>>> That's no longer the case for cgroups v2 (at least for crun). Either
>>>>>>> way, most test changes are gone now.
>>>>>>>
>>>>>>>>> I wonder if it’s worth trying to synthesize memory_max_usage_in_bytes() by keeping the highest
>>>>>>>>> value ever returned by the API.
>>>>>>>>
>>>>>>>> Interesting idea. I'll ponder this a bit and get back to you.
>>>>>>>
>>>>>>> This has been implemented. I'm not sure this is correct, though. It
>>>>>>> merely piggy-backs on calls to memory_usage_in_bytes() and keeps the
>>>>>>> high watermark value of that.
>>>>>>>
>>>>>>> Testing passed on F31 with cgroups v2 controllers properly configured
>>>>>>> (podman) and hybrid (legacy hierarchy) with docker/podman.
>>>>>>>
>>>>>>> Thoughts?
>>>>>>
>>>>>> Ping?
>>>>>
>>>>> Anyone willing to review this? It would be nice to make some progress.
>>>>>
>>>>> Thanks,
>>>>> Severin
>>>>>
>>>>>> Metrics work proposed for RFR here:
>>>>>> http://mail.openjdk.java.net/pipermail/core-libs-dev/2019-November/063464.html
>>>>>>
>>>>>> Thanks,
>>>>>> Severin
>