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

roachtest: delete synctest #106917

Merged
merged 1 commit into from
Jul 17, 2023
Merged

roachtest: delete synctest #106917

merged 1 commit into from
Jul 17, 2023

Conversation

sumeerbhola
Copy link
Collaborator

This test has been skipped since May 2020, because Pebble code is written to crash on an error when writing to the MANIFEST and the WAL, since it is not viable to continue. Additionally, there were problems with hangs due to charybdefs returning EAGAIN (which have since been fixed).

We are deleting this now since:

  • The purpose of this test is to test that Pebble does not lose data due to filesystem errors. It does so in a cruder manner than Pebble's strict MemFS based testing (which came later). Given the existence of the latter, there is no good reason to retain this test.

  • Working around charybdefs flakiness (see roachtest: remove the FUSE-variants of the disk-stall roachtest #102492), and Pebble's failstop-on-error behavior is not worth the initial and ongoing maintenance effort.

Epic: none

Fixes: #48603

Release note: None

@sumeerbhola sumeerbhola requested review from jbowens and a team July 17, 2023 15:58
@sumeerbhola sumeerbhola requested review from a team as code owners July 17, 2023 15:58
@sumeerbhola sumeerbhola requested review from smg260 and renatolabs and removed request for a team July 17, 2023 15:58
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Member

@RaduBerinde RaduBerinde left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @jbowens, @renatolabs, and @smg260)

This test has been skipped since May 2020, because Pebble code is written
to crash on an error when writing to the MANIFEST and the WAL, since it
is not viable to continue. Additionally, there were problems with hangs
due to charybdefs returning EAGAIN (which have since been fixed).

We are deleting this now since:
- The purpose of this test is to test that Pebble does not lose data due
  to filesystem errors. It does so in a cruder manner than Pebble's
  strict MemFS based testing (which came later). Given the existence of
  the latter, there is no good reason to retain this test.

- Working around charybdefs flakiness (see cockroachdb#102492), and Pebble's
  failstop-on-error behavior is not worth the initial and ongoing
  maintenance effort.

Epic: none

Fixes: cockroachdb#48603

Release note: None
Copy link
Contributor

@itsbilal itsbilal left a comment

Choose a reason for hiding this comment

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

:lgtm_strong:

Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (and 1 stale) (waiting on @jbowens, @renatolabs, and @smg260)

@sumeerbhola
Copy link
Collaborator Author

TFTR!

@sumeerbhola
Copy link
Collaborator Author

flaky tests

@sumeerbhola
Copy link
Collaborator Author

bors r=RaduBerinde,itsbilal

@craig
Copy link
Contributor

craig bot commented Jul 17, 2023

Build succeeded:

@craig craig bot merged commit 5ff1c3f into cockroachdb:master Jul 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

roachtest: synctest failed on Pebble
4 participants