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.

Sign up for GitHub

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

Jump to bottom

[docdb] Add TSAN suppression for GFLAGs #5800

Closed
bmatican opened this issue Sep 23, 2020 · 0 comments
Closed

[docdb] Add TSAN suppression for GFLAGs #5800

bmatican opened this issue Sep 23, 2020 · 0 comments
Assignees
@bmatican
Labels
area/docdb YugabyteDB core features kind/failing-test Tests and testing infra

Comments

@bmatican
Copy link
Contributor

We seem to get some gflag races across several tests now and then, because we're introducing races with tests writing to flags, while the mini clusters have already started.

I was talking to @spolitov and it might make sense to just ignore all of these. One note Sergei did make is that races on strings might still be worth keeping, as it would make debugging these in tests easier. However, the generic suppression list does not have a good way to filter out the the datatype of gflags...
@bmatican bmatican added kind/failing-test Tests and testing infra area/docdb YugabyteDB core features labels Sep 23, 2020
@bmatican bmatican self-assigned this Sep 23, 2020
@bmatican bmatican mentioned this issue Oct 1, 2020
Open
bmatican added a commit that referenced this issue Oct 1, 2020
@bmatican
[#5800] Add TSAN suppression for GFLAG races
7568877 
Summary:
Since we should never write to gflags from the main code, we might as well ignore all test
race conditions on gflags.

Currently, the main DB code should not be updating gflags, so all race conditions we find in testing should be strictly due to tests writing, while the server side code does reads. We should be safe to ignore those!

There are two caveats to this:
- Despite test-only, race conditions on updating `string` gflags can still cause issues, and technically, a TSAN report on those races would be easier to figure out, than say, a SEGV
- For production code, while the DB itself does not normally update gflags, we do have a `yb-ts-cli set_flag` command, through which we can runtime update flags, in memory. We frequently use that, for modifying various settings, without restarting servers. While the RPC itself uses `SetAtomic`, the rest of the code does not, so that use case can generate are legitimate races! Have filed #5892 for fixing that in the future.

Test Plan:
jenkins and also tried out on a test that would give races at a 1/100 rate:
`ybd tsan --cxx-test client_backup-txn-test --gtest_filter BackupTxnTest.DeleteTable -n 500`

Reviewers: sergei, timur, mikhail

Reviewed By: mikhail

Subscribers: ybase

Differential Revision: https://phabricator.dev.yugabyte.com/D9454
@bmatican bmatican closed this as completed Oct 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@bmatican bmatican

Labels
area/docdb YugabyteDB core features kind/failing-test Tests and testing infra
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@bmatican