8000 fix: LocalSubscriber hanging by dunglas · Pull Request #1040 · dunglas/mercure · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

fix: LocalSubscriber hanging #1040

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
Merged
merged 13 commits into from
May 14, 2025
Merged

fix: LocalSubscriber hanging #1040

merged 13 commits into from
May 14, 2025

Conversation

dunglas
Copy link
Owner
@dunglas dunglas commented May 4, 2025 •
edited
Loading

Fixes #1024

@dunglas dunglas mentioned this pull request May 4, 2025
Open
@divine
Copy link
Contributor
divine commented May 4, 2025

Hello,

I've now deployed Mercure with this PR but got the error:

Error: adapting config using caddyfile: parsing caddyfile tokens for 'route': parsing caddyfile tokens for 'mercure': strconv.ParseInt: parsing "1e5": invalid syntax, at /etc/caddy/Caddyfile:34

lru_cache 1e5

I was forced to remove it but I need it because of high amount of subscribers, hopefully this will be fixed as well!

Thanks!

@dunglas
Copy link
Owner Author
dunglas commented May 5, 2025

Can you try 100000 instead?

@dunglas
Copy link
Owner Author
dunglas commented May 5, 2025

I fixed the docs and added support for configuring both the number of shards and the number of items per shard.

kevburnsjr
kevburnsjr reviewed May 7, 2025
View reviewed changes
@dunglas dunglas changed the title fix: close chan in handleFullChan fix!: race conditions and memory leak in LocalSubscriber May 7, 2025
@dunglas dunglas marked this pull request as ready for review May 7, 2025 13:32
@dunglas
Copy link
Owner Author
dunglas commented May 7, 2025

@divine I finally found the bug crashing the benchmark. It was in the benchmark itself.

Could you try this PR to see if you get improvements?

@dunglas dunglas changed the title fix!: race conditions and memory leak in LocalSubscriber fix: race conditions and memory leak in LocalSubscriber May 7, 2025
@dunglas dunglas changed the title fix: race conditions and memory leak in LocalSubscriber fix: memory leak in LocalSubscriber May 7, 2025
@divine
Copy link
Contributor
divine commented May 10, 2025

Hello,

I've deployed it now. I'll let you know if it fails.

Thank you guys for the quick fixes!

@dunglas dunglas changed the title fix: memory leak in LocalSubscriber fix: LocalSubscriber hanging May 12, 2025
@dunglas
Copy link
Owner Author
dunglas commented May 13, 2025

@divine any news? I would like to merge this one!
@divine
Copy link
Contributor
divine commented May 14, 2025

@divine any news? I would like to merge this one!

Works fine for me! Didn't crash and still working fine.

Thanks!

@dunglas dunglas merged commit 5fb76f9 into main May 14, 2025
37 checks passed
@dunglas dunglas deleted the fix/1024 branch May 14, 2025 05:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kevburnsjr kevburnsjr kevburnsjr left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

High memory usage with the latest version
3 participants
@dunglas @divine @kevburnsjr
0