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

grpc: fix ListPath batch_size parameter #2859

Closed
wants to merge 1 commit into from

Conversation

Tuetuopay
Copy link
Contributor

With the append being gated by the length check, for each batch sent, we were dropped the current path. Change to always append first, then check the length.

The effect on small batch sizes was dramatic, and best shown with a size of 1: exactly half of the routes were missing.

GoBGP with 500 routes:

$ ./gobgp global rib -a evpn | wc -l
501
$ ./gobgp global rib -a evpn -b 1 | wc -l
251
$ ./gobgp global rib -a evpn -b 2 | wc -l
335
$ ./gobgp global rib -a evpn -b 500 | wc -l
501

Note: the above example uses 500 routes, but the issue is present even with two routes.

This commit also adds a --batch-size to gobgp rib commands, to be able to use the tweak from the cli. It is very useful with a large number of routes to avoid GoBGP gobbling up gigabytes of memory.

With the append being gated by the length check, for each batch sent, we
were dropped the current path. Change to always append first, then check
the length.

The effect on small batch sizes was dramatic, and best shown with a size
of 1: exactly half of the routes were missing.

GoBGP with 500 routes:

    $ ./gobgp global rib -a evpn | wc -l
    501
    $ ./gobgp global rib -a evpn -b 1 | wc -l
    251
    $ ./gobgp global rib -a evpn -b 2 | wc -l
    335
    $ ./gobgp global rib -a evpn -b 500 | wc -l
    501

Note: the above example uses 500 routes, but the issue is present even
with two routes.

This commit also adds a --batch-size to gobgp rib commands, to be able
to use the tweak from the cli. It is very useful with a large number of
routes to avoid GoBGP gobbling up gigabytes of memory.
@fujita
Copy link
Member

fujita commented Dec 2, 2024

Pushed, thanks!

@fujita fujita closed this Dec 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants