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.

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

[fix](jvm) the jvm opt should only be set once #48335

Merged
merged 1 commit into from
Feb 26, 2025

Conversation

morningman
Copy link
Contributor

@morningman morningman commented Feb 25, 2025

What problem does this PR solve?

Introduced from #47299
the SetEnvIfNecessary may be called multiple times, and in #47299,
we changed setenv("LIBHDFS_OPTS", libhdfs_opts.c_str(), 0); to setenv("LIBHDFS_OPTS", libhdfs_opts.c_str(), 1);
so it will add krb5 path at the end of LIBHDFS_OPTS and set it every time,
so LIBHDFS_OPTS becomes longer and longer.

This PR fix this issue by calling SetEnvIfNecessary only once

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 buildall

@doris-robot
Copy link

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

------ Round 1 ----------------------------------
q1	17596	5242	5093	5093
q2	2055	325	180	180
q3	10379	1294	745	745
q4	10205	1028	541	541
q5	7527	2424	2352	2352
q6	195	168	139	139
q7	909	755	602	602
q8	9292	1338	1123	1123
q9	4930	4825	4594	4594
q10	6819	2291	1882	1882
q11	469	294	270	270
q12	361	367	228	228
q13	17761	3774	3202	3202
q14	226	221	210	210
q15	515	475	467	467
q16	642	651	573	573
q17	592	867	361	361
q18	6692	6168	6145	6145
q19	1538	967	592	592
q20	334	336	201	201
q21	2873	2314	2036	2036
q22	373	340	306	306
Total cold run time: 102283 ms
Total hot run time: 31842 ms

----- Round 2, with runtime_filter_mode=off -----
q1	5216	5138	5202	5138
q2	239	329	237	237
q3	2177	2718	2322	2322
q4	1483	1851	1417	1417
q5	4281	4147	4197	4147
q6	215	167	126	126
q7	1900	1861	1803	1803
q8	2647	2760	2698	2698
q9	7125	7088	7120	7088
q10	3024	3248	2834	2834
q11	609	530	487	487
q12	730	800	676	676
q13	3472	3964	3373	3373
q14	269	292	288	288
q15	518	473	468	468
q16	653	692	684	684
q17	1147	1644	1341	1341
q18	7621	7284	7312	7284
q19	854	868	874	868
q20	1926	2086	1877	1877
q21	5638	5062	4805	4805
q22	638	615	541	541
Total cold run time: 52382 ms
Total hot run time: 50502 ms

@doris-robot
Copy link

TPC-DS: Total hot run time: 192905 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 14245d4686675b3702f16222e8f0a150249393c3, data reload: false

query1	1317	960	939	939
query2	6210	1938	1906	1906
query3	10995	4689	4577	4577
query4	54478	25456	23400	23400
query5	5280	634	539	539
query6	327	221	192	192
query7	4905	522	294	294
query8	310	257	233	233
query9	5688	2604	2627	2604
query10	428	315	265	265
query11	15211	15201	14918	14918
query12	165	117	113	113
query13	1060	522	420	420
query14	11002	7188	7029	7029
query15	217	212	182	182
query16	7133	617	512	512
query17	1105	781	622	622
query18	1584	448	318	318
query19	216	212	187	187
query20	133	129	137	129
query21	223	144	112	112
query22	4510	4403	4242	4242
query23	34571	33443	33380	33380
query24	5886	2561	2536	2536
query25	470	498	441	441
query26	727	303	167	167
query27	1864	528	340	340
query28	2784	2497	2504	2497
query29	608	608	447	447
query30	228	206	165	165
query31	933	921	836	836
query32	70	72	62	62
query33	460	388	315	315
query34	793	934	522	522
query35	846	879	768	768
query36	1034	1048	977	977
query37	120	105	81	81
query38	4275	4403	4213	4213
query39	1541	1498	1455	1455
query40	221	125	110	110
query41	59	50	53	50
query42	126	109	107	107
query43	508	546	498	498
query44	1365	858	823	823
query45	199	195	173	173
query46	978	1172	699	699
query47	1835	1874	1797	1797
query48	403	433	317	317
query49	716	529	433	433
query50	790	834	441	441
query51	4403	4434	4279	4279
query52	106	108	99	99
query53	244	277	197	197
query54	547	514	439	439
query55	91	78	84	78
query56	300	282	285	282
query57	1200	1215	1146	1146
query58	262	248	256	248
query59	2888	2851	2730	2730
query60	302	295	289	289
query61	126	133	118	118
query62	819	786	731	731
query63	253	193	231	193
query64	1964	1058	709	709
query65	3463	3289	3321	3289
query66	727	435	341	341
query67	15832	15498	15364	15364
query68	7413	899	508	508
query69	546	312	268	268
query70	1191	1147	1094	1094
query71	496	305	272	272
query72	5951	3869	3963	3869
query73	1339	785	356	356
query74	9333	9334	8747	8747
query75	3839	3316	2768	2768
query76	4122	1326	813	813
query77	652	396	276	276
query78	10122	10231	9284	9284
query79	3466	879	587	587
query80	733	553	454	454
query81	512	289	252	252
query82	554	133	95	95
query83	286	183	161	161
query84	276	98	79	79
query85	786	434	306	306
query86	375	309	307	307
query87	4487	4494	4399	4399
query88	3658	2215	2205	2205
query89	438	342	298	298
query90	1901	199	195	195
query91	134	140	112	112
query92	76	64	59	59
query93	2299	1019	596	596
query94	672	432	310	310
query95	354	270	263	263
query96	492	612	271	271
query97	3414	3390	3281	3281
query98	232	214	206	206
query99	1512	1468	1299	1299
Total cold run time: 302095 ms
Total hot run time: 192905 ms

@doris-robot
Copy link

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

query1	0.03	0.03	0.03
query2	0.08	0.03	0.04
query3	0.23	0.07	0.06
query4	1.62	0.10	0.10
query5	0.54	0.56	0.56
query6	1.18	0.73	0.71
query7	0.03	0.02	0.02
query8	0.04	0.03	0.04
query9	0.57	0.54	0.52
query10	0.58	0.58	0.58
query11	0.15	0.11	0.11
query12	0.14	0.11	0.13
query13	0.61	0.60	0.60
query14	2.67	2.70	2.70
query15	0.92	0.86	0.86
query16	0.39	0.38	0.39
query17	1.02	1.02	1.02
query18	0.23	0.20	0.19
query19	1.88	1.73	2.01
query20	0.01	0.01	0.01
query21	15.35	0.86	0.53
query22	0.75	1.06	0.71
query23	15.04	1.38	0.65
query24	6.77	1.78	0.74
query25	0.51	0.21	0.08
query26	0.58	0.18	0.14
query27	0.06	0.05	0.04
query28	9.23	0.85	0.42
query29	12.64	3.96	3.34
query30	0.25	0.09	0.07
query31	2.80	0.60	0.38
query32	3.23	0.56	0.46
query33	2.97	3.01	3.08
query34	15.74	5.13	4.47
query35	4.53	4.54	4.53
query36	0.68	0.50	0.49
query37	0.09	0.06	0.05
query38	0.05	0.04	0.04
query39	0.03	0.02	0.03
query40	0.16	0.14	0.13
query41	0.09	0.03	0.02
query42	0.04	0.02	0.03
query43	0.04	0.03	0.03
Total cold run time: 104.55 s
Total hot run time: 30.72 s

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

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

Copy link
Contributor

PR approved by anyone and no changes requested.

@morningman morningman merged commit a45d7bb into apache:master Feb 26, 2025
34 of 36 checks passed
github-actions bot pushed a commit that referenced this pull request Feb 26, 2025
### What problem does this PR solve?

Introduced from #47299 
the `SetEnvIfNecessary` may be called multiple times, and in #47299,
we changed `setenv("LIBHDFS_OPTS", libhdfs_opts.c_str(), 0);` to
`setenv("LIBHDFS_OPTS", libhdfs_opts.c_str(), 1);`
so it will add `krb5 path` at the end of `LIBHDFS_OPTS` and set it every
time, so `LIBHDFS_OPTS` becomes longer and longer.

This PR fix this issue by calling `SetEnvIfNecessary` only once
dataroaring pushed a commit that referenced this pull request Feb 26, 2025
morningman added a commit that referenced this pull request Feb 26, 2025
…VA_OPTS #48170 #48335 (#48284)

bp #48170 and #48335
and also pick part of #47299, only related to `jni-util.cpp`.
To pass the krb5.conf config to Jni env
zhiqiang-hhhh pushed a commit to zhiqiang-hhhh/doris that referenced this pull request Feb 27, 2025
### What problem does this PR solve?

Introduced from apache#47299 
the `SetEnvIfNecessary` may be called multiple times, and in apache#47299,
we changed `setenv("LIBHDFS_OPTS", libhdfs_opts.c_str(), 0);` to
`setenv("LIBHDFS_OPTS", libhdfs_opts.c_str(), 1);`
so it will add `krb5 path` at the end of `LIBHDFS_OPTS` and set it every
time, so `LIBHDFS_OPTS` becomes longer and longer.

This PR fix this issue by calling `SetEnvIfNecessary` only once
seawinde pushed a commit to seawinde/doris that referenced this pull request Feb 28, 2025
### What problem does this PR solve?

Introduced from apache#47299 
the `SetEnvIfNecessary` may be called multiple times, and in apache#47299,
we changed `setenv("LIBHDFS_OPTS", libhdfs_opts.c_str(), 0);` to
`setenv("LIBHDFS_OPTS", libhdfs_opts.c_str(), 1);`
so it will add `krb5 path` at the end of `LIBHDFS_OPTS` and set it every
time, so `LIBHDFS_OPTS` becomes longer and longer.

This PR fix this issue by calling `SetEnvIfNecessary` only once
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by one committer. dev/2.1.9-merged dev/3.0.5-merged p0_b reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants