Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[fix](java) should use JAVA_OPTS_FOR_JDK_17 instead of JAVA_OPTS #47913

Merged
merged 3 commits into from
Feb 15, 2025
Merged

[fix](java) should use JAVA_OPTS_FOR_JDK_17 instead of JAVA_OPTS #47913

merged 3 commits into from
Feb 15, 2025

Conversation

morningman
Copy link
Contributor

@morningman morningman commented Feb 14, 2025 •
edited
Loading

What problem does this PR solve?

Related PR: #47299

Problem Summary:

The master branch only support java 17+, so should remote all JAVA_OPTS and use JAVA_OPTS_FOR_JDK_17

Release note

None

Check List (For Author)

  • Test

    • Regression test
    • Unit Test
    • Manual test (add detailed scripts or steps below)
    • No need to test or manual test. Explain why:
      • This is a refactor/code format and no logic has been changed.
      • Previous test can cover this change.
      • No code files have been changed.
      • Other reason

  • Behavior changed:

    • No.
    • Yes.

  • Does this need documentation?

    • No.
    • Yes.

Check List (For Reviewer who merge this PR)

  • Confirm the release note
  • Confirm test cases
  • Confirm document
  • Add branch pick label

@hello-stephen
Copy link
Contributor

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.

Please clearly describe your PR:

  1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
  2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
  3. What features were added. Why was this function added?
  4. Which code was refactored and why was this part of the code refactored?
  5. Which functions were optimized and what is the difference before and after the optimization?

@morningman
Copy link
Contributor Author

run buidall

@morningman
Copy link
Contributor Author

run buildall

@morningman
Copy link
Contributor Author

run buildall

@doris-robot
Copy link

TPC-H: Total hot run time: 31735 ms 
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit 85f6eb72668f9a481697936259aa6b65142f59b1, data reload: false

------ Round 1 ----------------------------------
q1	17581	5228	5146	5146
q2	2070	305	165	165
q3	10586	1378	702	702
q4	10245	1009	554	554
q5	7748	2394	2360	2360
q6	187	169	132	132
q7	912	742	609	609
q8	9297	1289	1033	1033
q9	4932	4801	4654	4654
q10	6817	2297	1899	1899
q11	486	267	255	255
q12	345	352	221	221
q13	17766	3718	3108	3108
q14	230	234	211	211
q15	502	467	453	453
q16	611	605	580	580
q17	584	893	345	345
q18	6746	6197	6185	6185
q19	1216	952	559	559
q20	306	331	192	192
q21	3080	2200	2054	2054
q22	356	327	318	318
Total cold run time: 102603 ms
Total hot run time: 31735 ms

----- Round 2, with runtime_filter_mode=off -----
q1	5153	5125	5132	5125
q2	234	327	233	233
q3	2160	2637	2306	2306
q4	1438	1817	1368	1368
q5	4239	4138	4134	4134
q6	203	166	125	125
q7	1861	1820	1758	1758
q8	2631	2662	2638	2638
q9	7233	7049	7076	7049
q10	2966	3192	2745	2745
q11	647	528	477	477
q12	670	775	593	593
q13	3514	3917	3340	3340
q14	267	295	258	258
q15	512	455	443	443
q16	619	683	643	643
q17	1146	1624	1341	1341
q18	7531	7166	7206	7166
q19	775	800	856	800
q20	1949	2002	1919	1919
q21	5476	5028	4779	4779
q22	657	598	566	566
Total cold run time: 51881 ms
Total hot run time: 49806 ms

@doris-robot
Copy link

TPC-DS: Total hot run time: 183279 ms 
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpcds-tools
TPC-DS sf100 test result on commit 85f6eb72668f9a481697936259aa6b65142f59b1, data reload: false

query1	966	391	387	387
query2	6536	1816	1798	1798
query3	6810	214	209	209
query4	26585	23687	23040	23040
query5	4326	668	501	501
query6	310	195	205	195
query7	4605	504	312	312
query8	303	246	231	231
query9	8640	2539	2540	2539
query10	465	314	259	259
query11	15356	15278	14856	14856
query12	157	110	104	104
query13	1664	527	388	388
query14	9214	7036	6960	6960
query15	218	195	179	179
query16	7355	621	443	443
query17	1212	720	583	583
query18	1946	408	312	312
query19	196	183	162	162
query20	121	116	117	116
query21	214	128	109	109
query22	4061	4328	4155	4155
query23	33864	32814	32849	32814
query24	7653	2428	2405	2405
query25	544	451	387	387
query26	1231	297	165	165
query27	2074	495	342	342
query28	3870	2403	2385	2385
query29	749	552	424	424
query30	233	187	151	151
query31	912	868	788	788
query32	74	64	63	63
query33	558	358	303	303
query34	787	846	509	509
query35	807	844	730	730
query36	970	999	894	894
query37	117	96	73	73
query38	4188	4218	4043	4043
query39	1449	1395	1417	1395
query40	208	113	103	103
query41	54	51	49	49
query42	124	109	105	105
query43	506	522	483	483
query44	1284	798	779	779
query45	183	169	164	164
query46	873	1064	658	658
query47	1752	1760	1671	1671
query48	375	427	293	293
query49	795	513	411	411
query50	699	738	428	428
query51	4144	4210	4074	4074
query52	104	104	97	97
query53	238	264	186	186
query54	494	483	400	400
query55	82	84	78	78
query56	272	312	249	249
query57	1134	1121	1065	1065
query58	259	245	243	243
query59	2659	2604	2390	2390
query60	283	271	261	261
query61	126	128	118	118
query62	818	738	669	669
query63	232	198	216	198
query64	4356	976	657	657
query65	3275	3112	3166	3112
query66	1138	420	307	307
query67	15814	15437	15336	15336
query68	7841	756	502	502
query69	485	295	268	268
query70	1185	1160	1087	1087
query71	415	297	287	287
query72	5842	3624	3735	3624
query73	735	757	346	346
query74	9054	9215	8735	8735
query75	3240	3165	2686	2686
query76	3154	1184	770	770
query77	478	382	276	276
query78	9944	10103	9300	9300
query79	2502	820	602	602
query80	682	525	468	468
query81	493	273	238	238
query82	678	124	96	96
query83	185	172	160	160
query84	244	91	74	74
query85	772	343	330	330
query86	338	324	289	289
query87	4434	4489	4319	4319
query88	3688	2196	2205	2196
query89	391	327	289	289
query90	1897	196	191	191
query91	139	142	109	109
query92	78	67	58	58
query93	1734	998	595	595
query94	712	443	306	306
query95	353	272	256	256
query96	492	563	270	270
query97	2763	2808	2746	2746
query98	234	236	235	235
query99	1365	1429	1278	1278
Total cold run time: 270168 ms
Total hot run time: 183279 ms

@doris-robot
Copy link

ClickBench: Total hot run time: 30.14 s 
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/clickbench-tools
ClickBench test result on commit 85f6eb72668f9a481697936259aa6b65142f59b1, data reload: false

query1	0.04	0.03	0.03
query2	0.09	0.03	0.04
query3	0.23	0.06	0.07
query4	1.64	0.10	0.10
query5	0.42	0.43	0.40
query6	1.17	0.67	0.64
query7	0.02	0.02	0.02
query8	0.04	0.03	0.04
query9	0.59	0.52	0.51
query10	0.58	0.57	0.57
query11	0.17	0.10	0.10
query12	0.14	0.11	0.11
query13	0.61	0.60	0.60
query14	2.71	2.74	2.74
query15	0.91	0.85	0.84
query16	0.38	0.38	0.37
query17	1.01	1.02	1.05
query18	0.21	0.19	0.19
query19	1.90	1.75	2.01
query20	0.01	0.01	0.01
query21	15.36	0.91	0.56
query22	0.74	1.10	0.66
query23	15.01	1.38	0.61
query24	11.25	0.93	0.54
query25	0.35	0.16	0.12
query26	0.89	0.17	0.14
query27	0.05	0.04	0.05
query28	6.11	0.79	0.43
query29	12.52	3.97	3.30
query30	0.24	0.08	0.06
query31	2.84	0.60	0.40
query32	3.22	0.55	0.47
query33	2.93	2.96	2.99
query34	15.84	5.30	4.46
query35	4.50	4.49	4.56
query36	0.67	0.49	0.48
query37	0.09	0.06	0.06
query38	0.05	0.03	0.05
query39	0.03	0.02	0.02
query40	0.17	0.14	0.12
query41	0.08	0.03	0.02
query42	0.04	0.02	0.02
query43	0.03	0.03	0.03
Total cold run time: 105.88 s
Total hot run time: 30.14 s

@github-actions github-actions bot added the approved Indicates a PR has been approved by one committer. label Feb 14, 2025
@github-actions GitHub Actions
Copy link
Contributor

PR approved by at least one committer and no changes requested.

@github-actions GitHub Actions
Copy link
Contributor

PR approved by anyone and no changes requested.

dataroaring
dataroaring approved these changes Feb 15, 2025
View reviewed changes
Copy link
Contributor

@dataroaring dataroaring left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@morningman morningman merged commit ae2fb25 into apache:master Feb 15, 2025
25 of 27 checks passed
github-actions bot pushed a commit that referenced this pull request Feb 15, 2025
@morningman
[fix](java) should use JAVA_OPTS_FOR_JDK_17 instead of JAVA_OPTS (#47913
579cc37 
)

### What problem does this PR solve?

Related PR: #47299

Problem Summary:

The master branch only support java 17+, so should remote all
`JAVA_OPTS` and use `JAVA_OPTS_FOR_JDK_17`
@github-actions github-actions bot mentioned this pull request Feb 15, 2025
Closed
This was referenced Feb 18, 2025
Closed
Merged
morningman added a commit that referenced this pull request Feb 18, 2025
@morningman
Revert "[fix](java) should use JAVA_OPTS_FOR_JDK_17 instead of JAVA_O…
c7acec4 
…PTS" (#48017)

Reverts #47913
Before this PR, the BE's JVM heap size does not follow the
JAVA_OPTS_FOR_JDK17 in be.conf,
it use default value, which is 1/4 of physical memory.
After this PR, the BE's JVM heap size is set to 2GB, and it cause some
test pipeline OOM.
So I revert this PR temporarily to try investigate the usage of BE's JVM
first.
lzyy2024 pushed a commit to lzyy2024/doris that referenced this pull request Feb 21, 2025
@morningman @lzyy2024
[fix](java) should use JAVA_OPTS_FOR_JDK_17 instead of JAVA_OPTS (apa…
9eda61e 
…che#47913)

### What problem does this PR solve?

Related PR: apache#47299

Problem Summary:

The master branch only support java 17+, so should remote all
`JAVA_OPTS` and use `JAVA_OPTS_FOR_JDK_17`
lzyy2024 pushed a commit to lzyy2024/doris that referenced this pull request Feb 21, 2025
@morningman @lzyy2024
Revert "[fix](java) should use JAVA_OPTS_FOR_JDK_17 instead of JAVA_O…
0be25fd 
…PTS" (apache#48017)

Reverts apache#47913
Before this PR, the BE's JVM heap size does not follow the
JAVA_OPTS_FOR_JDK17 in be.conf,
it use default value, which is 1/4 of physical memory.
After this PR, the BE's JVM heap size is set to 2GB, and it cause some
test pipeline OOM.
So I revert this PR temporarily to try investigate the usage of BE's JVM
first.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zy-kkk zy-kkk zy-kkk approved these changes

@dataroaring dataroaring dataroaring approved these changes

Assignees
No one assigned
Labels
approved Indicates a PR has been approved by one committer. reviewed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@morningman @hello-stephen @doris-robot @zy-kkk @dataroaring