incubator-uniffle [#1149] fix: GC logs in JDK 11 do not include date and time stamps.

What changes were proposed in this pull request?

(Please outline the changes and how this PR fixes the issue.)

Why are the changes needed?

(Please clarify why the changes are needed. For instance,

If you propose a new API, clarify the use case for a new API.
If you fix a bug, describe the bug.)

Fix: #1149

Does this PR introduce any user-facing change?

(Please list the user-facing changes introduced by your change, including

Change in user-facing APIs.
Addition or removal of property keys.)

No.

How was this patch tested?

(Please test your changes, and provide instructions on how to test it:

If you add a feature or fix a bug, add a test to cover your changes.
If you fix a flaky test, repeat it for many times to prove it works.)

Manually tested

Oct 16 '23 06:10 qijiale76

According to this documentation https://openjdk.org/jeps/158, the correct format should be

-Xlog:gc[:[<output>][:[<decorators>][:<output-options>]]]

The previous option missed a : symbol, thus tags,time,uptime,level is not working

Oct 16 '23 07:10 qijiale76

@zhengchenyu Could you help review this pr?

Nov 04 '23 14:11 jerqi

@zhengchenyu @qijiale76 Any process here?

Apr 16 '24 12:04 rickyma

I will push forward with this PR again this week.

May 20 '24 02:05 qijiale76

Do we support higher versions of JDK here, like JDK 21? @qijiale76

May 20 '24 08:05 rickyma

Do we support higher versions of JDK here, like JDK 21? @qijiale76

We don't support high version now.

May 20 '24 08:05 jerqi

We don't support high version now.

I have run Uniffle on JDK 21 and didn't find any issues. I think Uniffle is compatible with JDK 21 already.

May 20 '24 08:05 rickyma

We don't support high version now.

I have run Uniffle on JDK 21 and didn't find any issues. I think Uniffle is compatible with JDK 21 already.

We should add a JDK 21 pipeline first.

May 20 '24 11:05 jerqi

@qijiale76 can this go ahead?

May 22 '24 07:05 EnricoMi

Test Results

2 419 files +14 2 419 suites +14 4h 57m 59s :stopwatch: +10s 933 tests + 2 932 :white_check_mark: + 2 1 :zzz: ±0 0 :x: ±0 10 819 runs +28 10 805 :white_check_mark: +28 14 :zzz: ±0 0 :x: ±0

Results for commit f0f20135. ± Comparison against base commit 168cf741.

:recycle: This comment has been updated with latest results.

May 22 '24 08:05 github-actions[bot]

@qijiale76 @zhengchenyu @jerqi is this ready for review?

May 23 '24 09:05 EnricoMi

I don't think we need to support all versions of the JDK, the user should configure it according to their own environment. But the docker image is using JDK11, the pom is using JDK8. Then I don't know which version to support.

May 23 '24 10:05 zhengchenyu

I don't think we need to support all versions of the JDK, the user should configure it according to their own environment. But the docker image is using JDK11, the pom is using JDK8. Then I don't know which version to support.

You can see the pipelines https://github.com/apache/incubator-uniffle/blob/master/.github/workflows/build.yml to find which version to support. Spark supports JDK8, JDK11 and JDK17.

May 23 '24 11:05 jerqi

I don't think we need to support all versions of the JDK, the user should configure it according to their own environment. But the docker image is using JDK11, the pom is using JDK8. Then I don't know which version to support.

You can see the pipelines https://github.com/apache/incubator-uniffle/blob/master/.github/workflows/build.yml to find which version to support. Spark supports JDK8, JDK11 and JDK17.

OK, we should support JDK17!

May 23 '24 12:05 zhengchenyu

I just fixed the issue causing the test failure. I'll test the Java 17 configuration tomorrow.

May 23 '24 12:05 qijiale76

@qijiale76 Could you update the description? @zhengchenyu @xianjingfeng Could you help me review this pull request?

May 27 '24 10:05 jerqi

I'm not very familiar with latest Java GC configurations. If only for JDK 8 and 11, LGTM+1!

May 27 '24 11:05 zhengchenyu