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

[3.5] add more debug info for opening WAL files failure #14864

Closed

Conversation

ahrtr
Copy link
Member

@ahrtr ahrtr commented Nov 28, 2022

Backport #14862 to 3.5.

Related issue: #14851

Signed-off-by: Benjamin Wang wachao@vmware.com

Please read https://github.com/etcd-io/etcd/blob/main/CONTRIBUTING.md#contribution-flow.

@ahrtr ahrtr force-pushed the add_log_open_wal_failure_3.5_20221128 branch 2 times, most recently from 9ab785f to b1136dc Compare November 28, 2022 08:25
@ahrtr
Copy link
Member Author

ahrtr commented Nov 28, 2022

cc @serathius

@serathius
Copy link
Member

Not a big fan of backporting a feature, especially one that breaks backward compatibility. Returned error will no longer be ErrFileNotFound, so all the checks err == ErrFileNotFound need to be changed to error.Is(err, ErrFileNotFound).

In my opinion the benefits don't outweigh the risk of something breaking.

Signed-off-by: Benjamin Wang <wachao@vmware.com>
@ahrtr ahrtr force-pushed the add_log_open_wal_failure_3.5_20221128 branch from b1136dc to fc65d9b Compare November 28, 2022 11:01
@ahrtr
Copy link
Member Author

ahrtr commented Nov 28, 2022

all the checks err == ErrFileNotFound need to be changed to error.Is(err, ErrFileNotFound)

Already updated.

Not a big fan of backporting a feature, especially one that breaks backward compatibility.

Note that the reason why I submitted this PR is that it's hard to tell where is the error coming from in #14851. Since we will maintain 3.5 for a long time, it's definitely worthwhile to add such debug info.

In my opinion the benefits don't outweigh the risk of something breaking.

I don't get what's the risk you are talking about. It only affects unit test.

@ahrtr ahrtr closed this Feb 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants