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

Revert "libroach: use FlushWAL instead of SyncWAL" #32605

Merged
merged 1 commit into from Nov 27, 2018

Conversation

Projects
None yet
3 participants
@petermattis
Copy link
Contributor

petermattis commented Nov 26, 2018

This reverts commit a321424.

We have a report with credible testing that using FlushWAL instead of
SyncWAL causes data loss in disk full situations. Presumably there is
some error that is not being propagated correctly.
Possibly related to #31948.

See #25173. Possibly unrelated, but the symptom is the same.

Release note (bug fix): Fix a node data loss bug that occurs when a disk
becomes temporarily full.

Revert "libroach: use FlushWAL instead of SyncWAL"
This reverts commit a321424.

We have a report with credible testing that using FlushWAL instead of
SyncWAL causes data loss in disk full situations. Presumably there is
some error that is not being propagated correctly.
Possibly related to #31948.

See #25173. Possibly unrelated, but the symptom is the same.

Release note (bug fix): Fix a node data loss bug that occurs when a disk
becomes temporarily full.

@petermattis petermattis requested a review from cockroachdb/core-prs as a code owner Nov 26, 2018

@cockroach-teamcity

This comment has been minimized.

Copy link
Member

cockroach-teamcity commented Nov 26, 2018

This change is Reviewable

@petermattis petermattis requested review from bdarnell and tbg Nov 26, 2018

@bdarnell

This comment has been minimized.

Copy link
Member

bdarnell commented Nov 26, 2018

File a new issue to track the investigation into the underlying bug (and turning FlushWAL back on assuming the performance gains are worth it)

@petermattis

This comment has been minimized.

Copy link
Contributor

petermattis commented Nov 27, 2018

Will do on filing an issue.

bors r=bdarnell

craig bot pushed a commit that referenced this pull request Nov 27, 2018

Merge #32605
32605: Revert "libroach: use FlushWAL instead of SyncWAL" r=bdarnell a=petermattis

This reverts commit a321424.

We have a report with credible testing that using FlushWAL instead of
SyncWAL causes data loss in disk full situations. Presumably there is
some error that is not being propagated correctly.
Possibly related to #31948.

See #25173. Possibly unrelated, but the symptom is the same.

Release note (bug fix): Fix a node data loss bug that occurs when a disk
becomes temporarily full.

Co-authored-by: Peter Mattis <petermattis@gmail.com>
@craig

This comment has been minimized.

Copy link

craig bot commented Nov 27, 2018

Build succeeded

@craig craig bot merged commit a485597 into cockroachdb:master Nov 27, 2018

3 checks passed

GitHub CI (Cockroach) TeamCity build finished
Details
bors Build succeeded
Details
license/cla Contributor License Agreement is signed.
Details

@petermattis petermattis deleted the petermattis:pmattis/disable-flush-wal branch Nov 27, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment