Skip to content

Shrink queue process state in case of abnormal queue [member] process termination to reduce peak memory footprint #14523

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 1 commit into from
Sep 15, 2025
Merged

Shrink queue process state in case of abnormal queue [member] process termination to reduce peak memory footprint #14523

merged 1 commit into from
Sep 15, 2025

Conversation

lukebakken
Copy link
Collaborator

@lukebakken lukebakken commented Sep 9, 2025 •
edited
Loading

  • Call Mod:format_state/1 if exported to possibly truncate huge states
  • Add more information about truncated ram_pending_ack and disk_pending_ack
  • Add log.error_logger_format_depth cuttlefish schema value

You can use this branch to test these changes - it calls exit() the first time a delivery is attempted to a consumer:

https://github.com/lukebakken/rmq-rabbitmq-server/tree/lukebakken/fix-format-status-test-exit

I've used it with PerfTest to crash the queue process:

cd rabbitmq-perf-test
make ARGS='--producers 1 --consumers 1 --pmessages 1010 --flag persistent' run

Finally, see this discussion!

#14349

@lukebakken lukebakken requested a review from dcorbacho September 9, 2025 17:02
@lukebakken lukebakken self-assigned this Sep 9, 2025
@lukebakken lukebakken requested review from michaelklishin, the-mikedavis, SimonUnge and dumbbell and removed request for dcorbacho September 9, 2025 17:02
@lukebakken lukebakken force-pushed the lukebakken/fix-format-status branch from ccdb859 to 459c0bc Compare September 9, 2025 18:31
lhoguin
lhoguin reviewed Sep 10, 2025
View reviewed changes
@lukebakken lukebakken marked this pull request as draft September 10, 2025 14:06
@lukebakken lukebakken force-pushed the lukebakken/fix-format-status branch 2 times, most recently from fe3e6d9 to 9f51488 Compare September 10, 2025 17:44
@lukebakken lukebakken requested a review from lhoguin September 10, 2025 17:47
@lukebakken lukebakken force-pushed the lukebakken/fix-format-status branch from 9f51488 to 4a64e41 Compare September 10, 2025 17:52
SimonUnge
SimonUnge reviewed Sep 10, 2025
View reviewed changes
@lukebakken lukebakken force-pushed the lukebakken/fix-format-status branch from 4a64e41 to 0263ae0 Compare September 10, 2025 18:18
@lukebakken lukebakken marked this pull request as ready for review September 10, 2025 18:18
@lukebakken lukebakken force-pushed the lukebakken/fix-format-status branch from 0263ae0 to 40772c2 Compare September 10, 2025 22:04
lhoguin
lhoguin reviewed Sep 11, 2025
View reviewed changes
lhoguin
lhoguin reviewed Sep 11, 2025
View reviewed changes
@lukebakken lukebakken force-pushed the lukebakken/fix-format-status branch from 40772c2 to 296c014 Compare September 12, 2025 16:20
@michaelklishin michaelklishin changed the title Code to clean up queue process state when crashed Shrink queue process state in case of abnormal queue [member] process termination to reduce peak memory footprint Sep 12, 2025
@lukebakken lukebakken marked this pull request as draft September 12, 2025 18:37
@lukebakken lukebakken force-pushed the lukebakken/fix-format-status branch 2 times, most recently from 2cf753b to 1022b9c Compare September 12, 2025 22:58
@lukebakken lukebakken requested a review from lhoguin September 12, 2025 22:59
@lukebakken lukebakken marked this pull request as ready for review September 12, 2025 22:59
lhoguin
lhoguin reviewed Sep 15, 2025
View reviewed changes
@lukebakken lukebakken force-pushed the lukebakken/fix-format-status branch from 1022b9c to 2ebb1b2 Compare September 15, 2025 14:34
@lukebakken lukebakken requested a review from lhoguin September 15, 2025 14:34
@lhoguin
Copy link
Contributor

lhoguin commented Sep 15, 2025

CI has lots of config related failures, could they be caused by the config changes in this PR?

@michaelklishin
Copy link
Collaborator

The OCI failure has to do with the fact that @lukebakken is an external collaborator. I will re-trigger the rest, they seem to be flakes.

@michaelklishin michaelklishin added this to the 4.3.0 milestone Sep 15, 2025
michaelklishin
michaelklishin reviewed Sep 15, 2025
View reviewed changes
@lukebakken lukebakken force-pushed the lukebakken/fix-format-status branch from 2ebb1b2 to 29e39c8 Compare September 15, 2025 15:38
michaelklishin
michaelklishin requested changes Sep 15, 2025
View reviewed changes
Copy link
Collaborator

@michaelklishin michaelklishin left a comment •
edited
Loading

Choose a reason for hiding this comment

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

The failures are legit and have to do with the fact that a cuttlefish run now produces one extra key.

The solution is to use cuttlefish:unset/0 for the rabbitmq.conf values that are unset/undefined.

@lukebakken lukebakken force-pushed the lukebakken/fix-format-status branch from 29e39c8 to 44a9fb5 Compare September 15, 2025 15:41
@lukebakken
Copy link
Collaborator Author

@michaelklishin sorry, I clicked only the first failing action, saw that it had something to do with the generic-unix package, and didn't think to click the other ones. Anyway, make -C deps/rabbit ct-config_schema runs successfully on my machine now, so hopefully that's all there is to it.

@michaelklishin
Copy link
Collaborator

I confirm that the Cuttlefish schema suites in several plugins now pass for me locally.

@lukebakken
Copy link
Collaborator Author

I spoke too soon. Dialyzer time.

@the-mikedavis
Copy link
Collaborator

I gave this a try locally and setting both options really cuts down on the exit report 👍. I see 1166 lines of error logs with nothing set and just 172 lines (mostly from the error logger not printing all messages in the mailbox) with log.error_logger_format_depth at 10 and log.summarize_process_state set to true.

@lukebakken lukebakken force-pushed the lukebakken/fix-format-status branch from 44a9fb5 to 246e01a Compare September 15, 2025 16:16
@lukebakken lukebakken marked this pull request as draft September 15, 2025 16:22
@lukebakken
Copy link
Collaborator Author

TIL: asdf installs elixir builds that don't have debug_info 😡 thus dialyzer won't work...

@lukebakken lukebakken force-pushed the lukebakken/fix-format-status branch from 246e01a to afbbf01 Compare September 15, 2025 16:25
@lukebakken lukebakken marked this pull request as ready for review September 15, 2025 16:35
@lukebakken
Copy link
Collaborator Author

@michaelklishin looks like I figured out the dialyzer checks.

@lukebakken
Code to clean up queue process state when crashed
db1291d 
* Call `Mod:format_state/1` if exported to possibly truncate huge states
* Add more information about truncated ram_pending_ack and disk_pending_ack
* Add `log.error_logger_format_depth` cuttlefish schema value
* Add `format_state/1` to `rabbit_channel`
* Add `log.summarize_process_state`, default is `false`, to enable summarizing process state for crash logs.
* Added `format_state` to `rabbit_classic_queue_index_v2` and `rabbit_classic_queue_store_v2`
* Ensure `rabbit_channel:format_state/1` uses `summarize_process_state_when_logged`
* Do not set `summarize_process_state_when_logged` value by default.
* Type specs
@lukebakken lukebakken force-pushed the lukebakken/fix-format-status branch from afbbf01 to db1291d Compare September 15, 2025 16:37
@michaelklishin michaelklishin merged commit aadc89b into rabbitmq:main Sep 15, 2025
285 of 287 checks passed
@mergify mergify bot mentioned this pull request Sep 15, 2025
Merged
michaelklishin added a commit that referenced this pull request Sep 15, 2025
@michaelklishin
Merge pull request #14548 from rabbitmq/mergify/bp/v4.2.x/pr-14523
7445003 
Shrink queue process state in case of abnormal queue [member] process termination to reduce peak memory footprint (backport #14523)
@lukebakken lukebakken mentioned this pull request Sep 16, 2025
Merged
@mergify mergify bot mentioned this pull request Sep 16, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@michaelklishin michaelklishin michaelklishin approved these changes

@the-mikedavis the-mikedavis Awaiting requested review from the-mikedavis

@dumbbell dumbbell Awaiting requested review from dumbbell

@SimonUnge SimonUnge Awaiting requested review from SimonUnge

@lhoguin lhoguin Awaiting requested review from lhoguin

Assignees

@lukebakken lukebakken

Labels
backport-v4.2.x
Projects
None yet
Milestone
4.3.0
Development

Successfully merging this pull request may close these issues.
5 participants
@lukebakken @lhoguin @michaelklishin @the-mikedavis @SimonUnge