Skip to content

Small fixes for jemalloc profiler#87614

Merged
azat merged 4 commits intomasterfrom
fix-jemalloc-profiler
Sep 27, 2025
Merged

Small fixes for jemalloc profiler#87614
azat merged 4 commits intomasterfrom
fix-jemalloc-profiler

Conversation

@antonio2368
Copy link
Copy Markdown
Member

@antonio2368 antonio2368 commented Sep 25, 2025 •
edited by clickhouse-gh bot
Loading

Changelog category (leave one):

  • Not for changelog (changelog entry is not required)

Documentation entry for user-facing changes

  • Documentation is written (mandatory for new features)

@clickhouse-gh
Copy link
Copy Markdown
Contributor

clickhouse-gh bot commented Sep 25, 2025 •
edited
Loading

Workflow [PR], commit [71ce43c]

Summary: ✅

@clickhouse-gh clickhouse-gh bot added the pr-not-for-changelog This PR should not be mentioned in the changelog label Sep 25, 2025
@UnamedRus
Copy link
Copy Markdown
Contributor

UnamedRus commented Sep 25, 2025

Why remove it?

If it not secure to have in query, let it be server level setting

@antonio2368
Copy link
Copy Markdown
Member Author

antonio2368 commented Sep 25, 2025

Yeah it's not secure.
This is a quick fix before 25.9. I'll look into alternatives later.
You can still control destination with MALLOC_CONF env variable.

azat
azat approved these changes Sep 25, 2025
View reviewed changes
docs/en/operations/allocation-profiling.md

You can also specify a different prefix for the file in the query:
```sql
SYSTEM JEMALLOC FLUSH PROFILE TO '/tmp/my_own_prefix'
Copy link
Copy Markdown
Member

@azat azat Sep 25, 2025

Choose a reason for hiding this comment

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

This was actually a killer feature for me, I would instead remove the ability to execute query w/o profile path (and then we can remove thread local variable with profile path in Jemalloc.cpp), but I guess fine as a "hot fix".

@azat
Copy link
Copy Markdown
Member

azat commented Sep 25, 2025

And I guess it should be marked as bug-fix, since it should be backported

@azat azat added this pull request to the merge queue Sep 27, 2025
Merged via the queue into master with commit 0a217f6 Sep 27, 2025
123 checks passed
@azat azat deleted the fix-jemalloc-profiler branch September 27, 2025 11:58
@azat
Copy link
Copy Markdown
Member

azat commented Sep 27, 2025

I guess we need to backport it to 25.9?

@robot-clickhouse-ci-1 robot-clickhouse-ci-1 mentioned this pull request Sep 27, 2025
Merged
@robot-ch-test-poll1 robot-ch-test-poll1 added the pr-synced-to-cloud The PR is synced to the cloud repo label Sep 27, 2025
@robot-ch-test-poll4 robot-ch-test-poll4 added pr-backports-created-cloud deprecated label, NOOP pr-must-backport-synced The `*-must-backport` labels are synced into the cloud Sync PR labels Sep 27, 2025
robot-ch-test-poll3 added a commit that referenced this pull request Sep 27, 2025
@robot-ch-test-poll3
Merge pull request #87755 from ClickHouse/cherrypick/25.9/87614
667048a 
Cherry pick #87614 to 25.9: Small fixes for jemalloc profiler
robot-clickhouse added a commit that referenced this pull request Sep 27, 2025
@robot-clickhouse
Backport #87614 to 25.9: Small fixes for jemalloc profiler
af88a11
@robot-ch-test-poll3 robot-ch-test-poll3 mentioned this pull request Sep 27, 2025
Merged
@robot-ch-test-poll2 robot-ch-test-poll2 added the pr-backports-created Backport PRs are successfully created, it won't be processed by CI script anymore label Sep 27, 2025
antonio2368 added a commit that referenced this pull request Sep 29, 2025
@antonio2368
Merge pull request #87776 from ClickHouse/backport/25.9/87614
ed7515c 
Backport #87614 to 25.9: Small fixes for jemalloc profiler
azat added a commit that referenced this pull request Sep 29, 2025
@azat
Merge remote-tracking branch 'u/25.9' into backport/25.9/87584
91b3e43 
* u/25.9:
  Backport #87614 to 25.9: Small fixes for jemalloc profiler
  Backport #84020 to 25.9: Fix geoparquet types breaking client-server protocol (#87744)
  Backport #78873 to 25.9: Fix RMV tryEnqueueReplicatedDDL shutdown crash again (#87753)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@azat azat azat approved these changes

@alexey-milovidov alexey-milovidov alexey-milovidov approved these changes

Assignees

@azat azat

Labels

pr-backports-created Backport PRs are successfully created, it won't be processed by CI script anymore pr-backports-created-cloud deprecated label, NOOP pr-must-backport-synced The `*-must-backport` labels are synced into the cloud Sync PR pr-not-for-changelog This PR should not be mentioned in the changelog pr-synced-to-cloud The PR is synced to the cloud repo

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@antonio2368 @UnamedRus @azat @alexey-milovidov @robot-ch-test-poll1 @robot-ch-test-poll2 @robot-ch-test-poll4