Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Set LD_PRELOAD to load jemalloc in Dockerfile-workers. #14182

Conversation

realtyem
Copy link
Contributor

@realtyem realtyem commented Oct 14, 2022

Pull Request Checklist

  • Pull request is based on the develop branch
  • Pull request includes a changelog file. The entry should:
    • Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from EventStore to EventWorkerStore.".
    • Use markdown where necessary, mostly for code blocks.
    • End with either a period (.) or an exclamation mark (!).
    • Start with a capital letter.
    • Feel free to credit yourself, by adding a sentence "Contributed by @github_username." or "Contributed by [Your Name]." to the end of the entry.
  • Pull request includes a sign off
  • Code style is correct
    (run the linters)

Using jemalloc instead of malloc is supposed to be more efficient at long running instances of Synapse. It's used in start.py for simple single process homeservers started by the simple Dockerfile. Stick it into Dockerfile-workers too.

As discussed in #synapse-dev

Signed-off-by: Jason Little realtyem@gmail.com

@realtyem
Copy link
Contributor Author

realtyem commented Oct 14, 2022

At line 637, there is a os.execl(). Should that be a os.execle() so it passes this environment through?

@realtyem realtyem marked this pull request as ready for review October 14, 2022 09:52
@realtyem realtyem requested a review from a team as a code owner October 14, 2022 09:52
@DMRobertson
Copy link
Contributor

I think the test failure is likely a flake (#14183). Complement-with-workers job succeeded otherwise. I'll rerun to confirm.

@realtyem
Copy link
Contributor Author

I think the test failure is likely a flake (#14183). Complement-with-workers job succeeded otherwise. I'll rerun to confirm.

Not the first time I've seen that flake. You should see the test failures on my personal repo 😁

@realtyem
Copy link
Contributor Author

Yeah, according to this I do need to switch to os.execle()

@DMRobertson
Copy link
Contributor

CI passes. How can we confirm that we really are using jemalloc within complement? Does Synapse detect this and log something?

@DMRobertson
Copy link
Contributor

There is a debug line here:

if not jemalloc_path:
# No loaded jemalloc was found.
logger.debug("jemalloc not found")
return
logger.debug("Found jemalloc at %s", jemalloc_path)

@realtyem
Copy link
Contributor Author

I enabled DEBUG real quick and ran for like 30 seconds. I got:

root@a403a22a8731:/data/logs# cat master.log | grep -e jemalloc
2022-10-14 07:23:12,663 - synapse.metrics.jemalloc - 174 - DEBUG - sentinel - Found jemalloc at /usr/lib/x86_64-linux-gnu/libjemalloc.so.2
2022-10-14 07:23:12,667 - synapse.metrics.jemalloc - 231 - DEBUG - sentinel - Added jemalloc stats

Sound right?

@realtyem
Copy link
Contributor Author

Can Prometheus pick up jemalloc stats? Perhaps with a blackbox exporter?

@DMRobertson
Copy link
Contributor

I enabled DEBUG real quick and ran for like 30 seconds. I got:

root@a403a22a8731:/data/logs# cat master.log | grep -e jemalloc
2022-10-14 07:23:12,663 - synapse.metrics.jemalloc - 174 - DEBUG - sentinel - Found jemalloc at /usr/lib/x86_64-linux-gnu/libjemalloc.so.2
2022-10-14 07:23:12,667 - synapse.metrics.jemalloc - 231 - DEBUG - sentinel - Added jemalloc stats

Thanks, that's perfect.

Can Prometheus pick up jemalloc stats? Perhaps with a blackbox exporter?

Not sure. Looks like the stats are only used here as part of cache pruning.

# determine if we're evicting due to memory
jemalloc_interface = get_jemalloc_stats()
if jemalloc_interface and autotune_config:
try:
jemalloc_interface.refresh_stats()
mem_usage = jemalloc_interface.get_stat("allocated")
if mem_usage > max_cache_memory_usage:
logger.info("Begin memory-based cache eviction.")
evicting_due_to_memory = True
except Exception:
logger.warning(
"Unable to read allocated memory, skipping memory-based cache eviction."
)

@DMRobertson DMRobertson merged commit c744690 into matrix-org:develop Oct 14, 2022
@realtyem realtyem deleted the realtyem/load-jemalloc-in-dockerfile-workers branch October 14, 2022 12:35
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants