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

Expose Config.TestOnly configuration option for disabling staggered start #414

Merged
merged 2 commits into from
Jul 3, 2024

Conversation

brandur
Copy link
Contributor

@brandur brandur commented Jul 3, 2024

This one's a continuation of #385. Maintenance services have a staggered
start feature that causes them to sleep for a random amount of jittered
time on startup so they don't all try to work simultaneously. This is
useful in production, but somewhat harmful in tests because it makes start
and stop slower and thereby integration test cases slower.

River has an internal flag that allows staggered start to be disabled in
its own test suite, but external users of River have no way to access this
functionality.

Here, introduce Config.TestOnly that can be provided to client
configuration in test suites regardless of whether the caller is internal
or not.

This differs slightly from #385 in that it provides only a boolean, with
the idea being that if we find it useful to disable other features for
tests in the future, a boolean keeps third party code for forwards
compatible in that they get these disabled automatically. In case it does
become important to distinguish between individual features at some later
time, I figure we can add an additional TestOnlyConfig property that
allows full configuration beyond defaults.

bgentry and others added 2 commits July 2, 2024 19:26
… start

This one's a continuation of #385. Maintenance services have a staggered
start feature that causes them to sleep for a random amount of jittered
time on startup so they don't all try to work simultaneously. This is
useful in production, but somewhat harmful in tests because it makes
start and stop slower and thereby integration test cases slower.

River has an internal flag that allows staggered start to be disabled in
its own test suite, but external users of River have no way to access
this functionality.

Here, introduce `Config.TestOnly` that can be provided to client
configuration in test suites regardless of whether the caller is
internal or not.

This differs slightly from #385 in that it provides only a boolean, with
the idea being that if we find it useful to disable other features for
tests in the future, a boolean keeps third party code for forwards
compatible in that they get these disabled automatically. In case it
does become important to distinguish between individual features at some
later time, I figure we can add an additional `TestOnlyConfig` property
that allows full configuration beyond defaults.
@brandur brandur force-pushed the brandur-test-only branch from 9fe31da to 70a34c8 Compare July 3, 2024 02:46
@brandur
Copy link
Contributor Author

brandur commented Jul 3, 2024

Damn, intermittency in TestPeriodicJobEnqueuer/AddManyAfterStart is absolutely awful. I'll try to get that fixed up.

Copy link
Contributor

@bgentry bgentry left a comment

Choose a reason for hiding this comment

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

Looks solid! Thanks for picking this up.

@brandur
Copy link
Contributor Author

brandur commented Jul 3, 2024

Awesome. NP!

@brandur brandur merged commit c2a2383 into master Jul 3, 2024
10 checks passed
@brandur brandur deleted the brandur-test-only branch July 3, 2024 14:39
brandur added a commit that referenced this pull request Jul 9, 2024
I was looking into what was making the example tests so slow (some like
the periodic job enqueuer one often take 3 seconds to run), and it turns
out that the lion's share of it is from from service start up jitter.

Example tests aren't in the main `river` package (they're in
`river_test`) so they didn't previously have a way of interacting with
the client in such a way as to disable stagger. Luckily, we do have a
way now. Take advantage of the recently introduced `TestOnly` client
property from #414 to make all the examples faster.
brandur added a commit that referenced this pull request Jul 9, 2024
I was looking into what was making the example tests so slow (some like
the periodic job enqueuer one often take 3 seconds to run), and it turns
out that the lion's share of it is from from service start up jitter.

Example tests aren't in the main `river` package (they're in
`river_test`) so they didn't previously have a way of interacting with
the client in such a way as to disable stagger. Luckily, we do have a
way now. Take advantage of the recently introduced `TestOnly` client
property from #414 to make all the examples faster.
brandur added a commit that referenced this pull request Jul 9, 2024
I was looking into what was making the example tests so slow (some like
the periodic job enqueuer one often take 3 seconds to run), and it turns
out that the lion's share of it is from from service start up jitter.

Example tests aren't in the main `river` package (they're in
`river_test`) so they didn't previously have a way of interacting with
the client in such a way as to disable stagger. Luckily, we do have a
way now. Take advantage of the recently introduced `TestOnly` client
property from #414 to make all the examples faster.
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