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

[BUG] Improper status codes for snapshot executions #4687

Open
Bukhtawar opened this issue Oct 5, 2022 · 8 comments
Open

[BUG] Improper status codes for snapshot executions #4687

Bukhtawar opened this issue Oct 5, 2022 · 8 comments
Assignees
Labels
bug Something isn't working CCI College Contributor Initiative related issues and PRs distributed framework good first issue Good for newcomers hacktoberfest Global event that encourages people to contribute to open-source.

Comments

@Bukhtawar
Copy link
Collaborator

Bukhtawar commented Oct 5, 2022

Describe the bug
A missing S3 bucket for the snapshot results on a 500 its possible that the bucket has been deleted after association.

{
    "error": {
        "root_cause": [
            {
                "type": "repository_exception",
                "reason": "[snapshot-name] could not read repository data from index blob"
            }
        ],
        "type": "repository_exception",
        "reason": "[snapshot-name] could not read repository data from index blob",
        "caused_by": {
            "type": "i_o_exception",
            "reason": "Exception when listing blobs by prefix [index-name]",
            "caused_by": {
                "type": "amazon_s3_exception",
                "reason": "amazon_s3_exception: The specified bucket does not exist (Service: Amazon S3; Status Code: 404; Error Code: NoSuchBucket; Request ID: V3DXMMJXXXXXX; S3 Extended Request ID: D7W3ThAkomdyqtKj0msy1g1Q/qpU0h9XXXXXXXX)"
            }
        }
    },
    "status": 500
}

Then a concurrent snapshot exception is a 503

public class ConcurrentSnapshotExecutionException extends SnapshotException {
public ConcurrentSnapshotExecutionException(final String repositoryName, final String snapshotName, final String msg) {
super(repositoryName, snapshotName, msg);
}
public ConcurrentSnapshotExecutionException(final Snapshot snapshot, final String msg) {
super(snapshot, msg);
}
public ConcurrentSnapshotExecutionException(StreamInput in) throws IOException {
super(in);
}
@Override
public RestStatus status() {
return RestStatus.SERVICE_UNAVAILABLE;
}

Expected behavior
Errors which are client side misconfiguration or client side operations which aren't supported based on the system's concurrency specification should be a 4xx

@Bukhtawar Bukhtawar added bug Something isn't working untriaged hacktoberfest Global event that encourages people to contribute to open-source. labels Oct 5, 2022
@andrross andrross added good first issue Good for newcomers and removed untriaged labels Oct 5, 2022
@p1729
Copy link

p1729 commented Oct 7, 2022

I would like to work on this @Bukhtawar. Will the status code always be a 400 or must it be based on error context e.g 404 in the above example? // CC @andrross

@Bukhtawar
Copy link
Collaborator Author

@p1729 Thanks for offering to contribute. Below error codes should do.
ConcurrentSnapshotException -> 429
RepositoryException(bucket not found) -> 404

p1729 added a commit to p1729/OpenSearch that referenced this issue Oct 7, 2022
Signed-off-by: p1729 <pankajkumar1729@gmail.com>
p1729 added a commit to p1729/OpenSearch that referenced this issue Oct 7, 2022
Signed-off-by: p1729 <pankajkumar1729@gmail.com>
@p1729 p1729 mentioned this issue Oct 7, 2022
6 tasks
@baba-devv
Copy link
Contributor

Hi @Bukhtawar, would like to work on this, I was going through the code & maybe I could resolve this.
Also, I've found some unused things, if you allow I can improve that.
Thanks in advance. cc: @andrross

@Bukhtawar
Copy link
Collaborator Author

Hi @Bukhtawar, would like to work on this, I was going through the code & maybe I could resolve this. Also, I've found some unused things, if you allow I can improve that. Thanks in advance. cc: @andrross

Thanks @baba-devv you could pick this up if @p1729 isn't still on it?

@andrross
Copy link
Member

andrross commented Jan 9, 2023

Also, I've found some unused things, if you allow I can improve that.

@baba-devv By all means feel free to contribute! Please do submit separate PRs though, as it usually best not to group unrelated things together. If it is a small refactoring or cleanup feel free to submit a PR directly. If it is larger or worth some discussion, then start with an issue describing what you plan to do.

@baba-devv
Copy link
Contributor

@andrross understood, will try to keep things sorted, thanks.

@JalODan
Copy link

JalODan commented Apr 2, 2023

Hi @Bukhtawar, I would like to work on this issue. I see @baba-devv have resolved partially. I can take the remaining part namely RepositoryException(bucket not found) -> 404. Thanks in advance. cc: @andrross

@Rishikesh1159 Rishikesh1159 added the CCI College Contributor Initiative related issues and PRs label Apr 5, 2023
@Ayan07
Copy link

Ayan07 commented Jun 20, 2023

Hi @Bukhtawar , I'm interested to take up this issue.
Can you please assign this to me?

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working CCI College Contributor Initiative related issues and PRs distributed framework good first issue Good for newcomers hacktoberfest Global event that encourages people to contribute to open-source.
Projects
None yet
Development

No branches or pull requests

8 participants