Skip to content

FaultInjection: Fix random number generation#30623

Merged
yashykt merged 8 commits intogrpc:masterfrom
yashykt:FaultInjectionRand
Aug 18, 2022
Merged

FaultInjection: Fix random number generation#30623
yashykt merged 8 commits intogrpc:masterfrom
yashykt:FaultInjectionRand

Conversation

@yashykt
Copy link
Copy Markdown
Member

@yashykt yashykt commented Aug 17, 2022

rand() is not thread-safe, so instead use absl::InsecureBitGen and absl::Uniform to generate random numbers.

Copy link
Copy Markdown
Member

@markdroth markdroth left a comment

Choose a reason for hiding this comment

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

This looks really good! Just one comment to address.

TEST_P(FaultInjectionTest, XdsFaultInjectionPercentageDelay) {
CreateAndStartBackends(1);
const uint32_t kFixedDelayMilliseconds = 100000;
#ifndef NDEBUG
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm really not keen on putting #ifndefs in the middle of individual tests. Can we not just increase the timeout even for opt builds?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Done

@yashykt yashykt force-pushed the FaultInjectionRand branch from 77ff4c2 to 9b93d05 Compare August 18, 2022 19:56
Comment thread test/cpp/end2end/xds/xds_fault_injection_end2end_test.cc
Comment thread test/cpp/end2end/xds/xds_fault_injection_end2end_test.cc
Comment thread test/cpp/end2end/xds/xds_fault_injection_end2end_test.cc
@yashykt
Copy link
Copy Markdown
Member Author

yashykt commented Aug 18, 2022

Thanks for reviewing!

@yashykt yashykt merged commit 02df22f into grpc:master Aug 18, 2022
@copybara-service copybara-service Bot added the imported Specifies if the PR has been imported to the internal repository label Aug 19, 2022
@yashykt yashykt deleted the FaultInjectionRand branch May 18, 2023 20:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bloat/low imported Specifies if the PR has been imported to the internal repository lang/core per-call-memory/neutral per-channel-memory/neutral release notes: yes Indicates if PR needs to be in release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants