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

Fix data race in WAL #9580

Merged
merged 1 commit into from
Mar 15, 2018
Merged

Fix data race in WAL #9580

merged 1 commit into from
Mar 15, 2018

Conversation

e-dard
Copy link
Contributor

@e-dard e-dard commented Mar 15, 2018

Fixes #9577.

This PR fixes a data race in the WAL, which can occur when writes and deletes are being executed concurrently. The WAL uses a buffer pool of []byte when reading from the log. WAL entries are unmarshaled into these buffers and passed along to the relevant methods handling the different
types of entry (write, delete etc).

In the case of deletes, the keys that need to be deleted were being stored for later processing, however these keys were part of the backing array of initial buffer from the pool. As such, those keys could be written to at a future time when handling other parts of the WAL.

The original bug can be triggered as follows:

$  go test -c -race github.com/influxdata/influxdb/tsdb
$ repeat 100 ./tsdb.test -test.run TestStore_Sketches

In master the race should appear after a few iterations. I've ran a few hundred iterations on this PR and it's 🍏 📗 💚

This commit fixes a data race in the WAL, which can occur when writes
and deletes are being executed concurrently. The WAL uses a buffer pool
of `[]byte` when reading the WAL. WAL entries are unmarshaled into these
buffers and passed along to the relevant methods handling the different
types of entry (write, delete etc).

In the case of deletes, the keys that need to be deleted were being
stored for later processing, however these keys were part of the backing
array of initial buffer from the pool. As such, those keys could be
written to at a future time when handling other parts of the WAL.
@e-dard e-dard added this to the 1.5.1 milestone Mar 15, 2018
@ghost ghost assigned e-dard Mar 15, 2018
@ghost ghost added the review label Mar 15, 2018
Copy link
Contributor

@stuartcarnie stuartcarnie left a comment

Choose a reason for hiding this comment

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

👏

@mark-rushakoff
Copy link
Contributor

.hooks/pre-commit 100755 → 100644

Removing the executable bit looks unintentional and unrelated to the fix.

@e-dard e-dard merged commit 8ab8b5a into master Mar 15, 2018
@e-dard e-dard deleted the er-wal-race branch March 15, 2018 17:59
@ghost ghost removed the review label Mar 15, 2018
@e-dard e-dard mentioned this pull request Mar 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Data race in TSDB store
4 participants