Skip to content

Fail-closed S3 tests race for the port they just freed #411

Description

@BryanFRD

Three tests that guard fail-closed behaviour in the S3 backend get their "unreachable" store by binding 127.0.0.1:0 and dropping the listener, then connecting to the port that was just freed:

  • storage::s3::tests::an_index_that_cannot_be_read_answers_that_somebody_still_holds_the_object
  • storage::s3::probe::tests::a_store_that_cannot_be_asked_is_not_trusted
  • storage::s3::probe::tests::a_store_that_cannot_be_asked_about_conditions_is_not_trusted_either

Freeing the port does not reserve it. Eight tests in the same binary bind stub listeners to 127.0.0.1:0 and run in parallel, and on Linux the kernel will hand a freshly released ephemeral port to the next listener that asks. When that happens the test is no longer talking to an unreachable store, it is talking to another test's stub, and it gets a real answer.

It happened on #403 (run 35587174316): the first of the three failed with an unreachable store must never be read as permission to delete. The same commit and lockfile pass when rerun, pass 15/15 in isolation and 25/25 alongside the rest of the storage suite locally, and it is the only failure of that test in the last 60 CI runs. A race, not the dependency bump that PR carried.

These are the wrong tests to have flake. The first one is what stops garbage collection from reading an unreachable index as "nobody holds this object" and deleting bytes somebody still references. A safety test that fails at random gets rerun until green, and the time it fails for a real reason it gets rerun the same way.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions