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

Increase rate limits during tests #1463

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

nirs
Copy link
Member

@nirs nirs commented Jun 18, 2024

We used the defaults which may be too low. In VRG controller we allow up to 10 events per seconds, with burst of up to 100 events per seconds. This may be enough for the tests, but since we use very low minimum internal, we may be blocked by the rate limiter and fail after 1 second timeout. To avoid this, use 5 times higher limits.

Local test logs:

@nirs nirs force-pushed the tests-rate-limit branch 2 times, most recently from 7417a54 to 49e70bb Compare June 19, 2024 15:06
nirs added 2 commits June 19, 2024 20:22
In some cases it is useful to be able to pass -ginkgo arguments
(e.g. -ginkgo.no-color). With this change we can run:

    make test GINKGO_ARGS="-ginkgo.no-color"

This is useful writing collecting test results to file avoiding the
color escape codes in the output.

Signed-off-by: Nir Soffer <nsoffer@redhat.com>
We used the defaults which may be too low. In VRG controller we allow up
to 10 events per seconds, with burst of up to 100 events per seconds.
This may be enough for the tests, but since we use very low minimum
internal, we may be blocked by the rate limiter and fail after 1 second
timeout. To avoid this, use 5 times higher limits.

I tested this locally by running 100 `make test` runs before and after
the change. The number of failures was similar (88/100 before, 89/100
after), but the failed tests are different.

With this change:

    $ grep '\[FAILED\] in' test-stress-100.log | awk '{print $5}' | sort | uniq -c | sort -rn
      2 /home/nsoffer/src/ramen/controllers/vrg_volrep_test.go:398
      2 /home/nsoffer/src/ramen/controllers/vrg_volrep_test.go:1411
      2 /home/nsoffer/src/ramen/controllers/protectedvolumereplicationgrouplist_controller_test.go:98
      2 /home/nsoffer/src/ramen/controllers/drpolicy_controller_test.go:56
      2 /home/nsoffer/src/ramen/controllers/drplacementcontrol_controller_test.go:1524
      2 /home/nsoffer/src/ramen/controllers/drcluster_controller_test.go:337
      2 /home/nsoffer/src/ramen/controllers/controllers_utils_test.go:66
      1 /home/nsoffer/src/ramen/controllers/drpolicy_controller_test.go:124
      1 /home/nsoffer/src/ramen/controllers/drcluster_controller_test.go:411
      1 /home/nsoffer/src/ramen/controllers/drcluster_controller_test.go:379
      1 /home/nsoffer/src/ramen/controllers/controllers_utils_test.go:50

Before this change:

    $ grep '\[FAILED\] in' test-stress-100-before.log | awk '{print $5}' | sort | uniq -c | sort -rn
      4 /home/nsoffer/src/ramen/controllers/protectedvolumereplicationgrouplist_controller_test.go:98
      2 /home/nsoffer/src/ramen/controllers/vrg_volrep_test.go:1411
      2 /home/nsoffer/src/ramen/controllers/controllers_utils_test.go:28
      1 /home/nsoffer/src/ramen/controllers/vrg_volrep_test.go:398
      1 /home/nsoffer/src/ramen/controllers/volsync/vshandler_test.go:1342
      1 /home/nsoffer/src/ramen/controllers/volsync/vshandler_test.go:1219
      1 /home/nsoffer/src/ramen/controllers/drplacementcontrol_controller_test.go:962
      1 /home/nsoffer/src/ramen/controllers/drplacementcontrol_controller_test.go:935
      1 /home/nsoffer/src/ramen/controllers/drplacementcontrol_controller_test.go:801
      1 /home/nsoffer/src/ramen/controllers/drplacementcontrol_controller_test.go:742
      1 /home/nsoffer/src/ramen/controllers/drplacementcontrol_controller_test.go:1956
      1 /home/nsoffer/src/ramen/controllers/drplacementcontrol_controller_test.go:1372
      1 /home/nsoffer/src/ramen/controllers/controllers_utils_test.go:66

Not sure that the change helps.

Signed-off-by: Nir Soffer <nsoffer@redhat.com>
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.

None yet

2 participants