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

Only log stopping rollback manager once #20041

Merged
merged 1 commit into from
Apr 10, 2023

Conversation

cipherboy
Copy link
Contributor

@cipherboy cipherboy commented Apr 7, 2023

When testing the Rollback Manager's one-time invocation in Enterprise, it was noticed that due to the channel being closed, we'd always hit this case and thus spam logs rather quickly with this message.

Switch to a boolean flip to log this once, as it is not executed in parallel and thus doesn't need a sync.Once.

This only affected anyone calling the test core's StopAutomaticRollbacks() helper.

This was introduced in #19748 though the enterprise counterpart had not merged yet.

@cipherboy cipherboy added bug Used to indicate a potential bug test pr/no-changelog backport/1.13.x labels Apr 7, 2023
@cipherboy cipherboy added this to the 1.14 milestone Apr 7, 2023
@cipherboy cipherboy requested review from a team April 7, 2023 12:21
When testing the Rollback Manager's one-time invocation in Enterprise,
it was noticed that due to the channel being closed, we'd always hit
this case and thus spam logs rather quickly with this message.

Switch to a boolean flip to log this once, as it is not executed in
parallel and thus doesn't need a sync.Once.

This only affected anyone calling the test core's
StopAutomaticRollbacks() helper.

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
@cipherboy cipherboy force-pushed the cipherboy-fix-rollback-manager-logging branch from b605792 to d6d1247 Compare April 7, 2023 12:36
Copy link
Contributor

@kitography kitography left a comment

Choose a reason for hiding this comment

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

Looks good, cleaner tests is nice :)

@cipherboy
Copy link
Contributor Author

Thanks @kitography!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Used to indicate a potential bug pr/no-changelog test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants