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

Check that the data dir exists #15821

Merged

Conversation

travisdowns
Copy link
Member

@travisdowns travisdowns commented Dec 20, 2023

If you start up Redpanda point to a data dir that doesn't exist, the error is fairly obscure: a std::system_error mentioning failed to open but no indication of what we were trying to open.

This has tripped up several people, including externally (and yours truly) so let's just fix it.

Backports Required

  • none - not a bug fix
  • none - this is a backport
  • none - issue does not exist in previous branches
  • none - papercut/not impactful enough to backport
  • v23.3.x
  • v23.2.x
  • v23.1.x

Release Notes

Improvements

  • A clearer error message when the data directory does not exist or is the wrong type

rockwotj
rockwotj previously approved these changes Dec 21, 2023
@travisdowns travisdowns marked this pull request as draft December 22, 2023 03:02
@travisdowns
Copy link
Member Author

On hold since there is a complication with the rp test fixture which seems to be often used with a non-existent data dir (and yet we don't fail on the pid file creation I guess because the fixture disables that).

@travisdowns travisdowns marked this pull request as ready for review January 15, 2024 15:33
If you start up Redpanda point to a data dir that doesn't exist, the
error is fairly obscure: a std::system_error mentioning failed to open
but no indication of _what_ we were trying to open.

This has tripped up several people, including externally so let's
just fix it. We apply a check that the directory exists immediately
before creating the pidfile, which is the place that will fail first
in practice if the directory does not exist.

Pidfiles are enabled by default, but are disabled in some places, e.g.,
our fixture tests: if we *don't* create a pidfile we do not require
the directory to exist, it will be created automatically, so we don't
check existence in that case.
@travisdowns
Copy link
Member Author

travisdowns commented Jan 15, 2024

It seems like at the introduction of pifiles ~4 years ago the Redpanda behavior flipped from creating the data dir if it didn't exist, to failing if it didn't exist, perhaps by accident as we do the pidfile creation very early (as we should) which happens to be before the place we create the directories. At least, that's how the code looks currently: if no pidfile is specified we explicitly create the data dir (and any required path components leading up to it) later on in initialization (and the fixture tests I mention rely on this behavior).

So arguably there might be a "bug" here since it's already the well-established behavior by now that we fail to start up with pidfile + missing data dir, I'm just going to keep it that way, but I move the data dir check into the if (making pidfile) branch so we fail with the right error message instead.

@vbotbuildovich
Copy link
Collaborator

@travisdowns travisdowns merged commit 9dbb710 into redpanda-data:dev Jan 15, 2024
19 checks passed
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.

None yet

4 participants