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

lib: Memory spike reduction for sh cmds at scale #16672

Merged
merged 1 commit into from
Aug 28, 2024

Conversation

raja-rajasekar
Copy link
Contributor

The output buffer vty->obuf is a linked list where each element is of 4KB.
Currently, when a huge sh command like is executed on a large scale, all the vty_outs are processed and the entire data is accumulated.
After the entire vty execution, vtysh_flush proceeses and puts this data in the socket (131KB at a time).

Problem here is the memory spike for such heavy duty show commands.

The fix here is to chunkify the output on VTY shell by flushing it intermediately for every 128 KB of output accumulated and free the memory allocated for the buffer data.

This way, we achieve ~25-30% reduction in the memory spike.

Fixes: #16498
Note: This is a continuation of MR #16498

The output buffer vty->obuf is a linked list where
each element is of 4KB.
Currently, when a huge sh command  like <show ip route json>
is executed on a large scale, all the vty_outs are
processed and the entire data is accumulated.
After the entire vty execution, vtysh_flush proceeses
and puts this data in the socket (131KB at a time).

Problem here is the memory spike for such heavy duty
show commands.

The fix here is to chunkify the output on VTY shell by
flushing it intermediately for every 128 KB of output
accumulated and free the memory allocated for the buffer data.

This way, we achieve ~25-30% reduction in the memory spike.

Fixes: FRRouting#16498
Note: This is a continuation of MR FRRouting#16498

Signed-off-by: Srujana <skanchisamud@nvidia.com>

Signed-off-by: Rajasekar Raja <rajasekarr@nvidia.com>
@donaldsharp
Copy link
Member

As a note this is a resubmittal of #16498 because that work was done by an intern and they have gone back to school and we do not expect them to continue this work at this moment. We just wanted to get it in. Rajasekar has addressed all the comments from that PR.

Copy link
Member

@ton31337 ton31337 left a comment

Choose a reason for hiding this comment

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

LGTM

@mjstapp mjstapp merged commit 79e0c6a into FRRouting:master Aug 28, 2024
16 checks passed
Copy link
Member

@pguibert6WIND pguibert6WIND left a comment

Choose a reason for hiding this comment

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

I have a comment about sh cmds at scale.
what happens if a vty_out command is done with a big big buffer greater than 1024 ?

see 9112fb3#diff-35a25e3a633413a63808f112260bca3ac308fd18325d4044c95e709899640981L258

afaik, that local buffer will only use the first 1024 bytes of the buffer, and will ignore the remaining. right ?

is it worth extending that buffer size too ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants