opensearch-project / OpenSearch

🔎 Open source distributed and RESTful search engine.
https://opensearch.org/docs/latest/opensearch/index/
Apache License 2.0
9.04k stars 1.67k forks source link

[BUG] Improper status codes for snapshot executions #4687

Open Bukhtawar opened 1 year ago

Bukhtawar commented 1 year ago

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

https://github.com/opensearch-project/OpenSearch/blob/15161a86491569d94642f3a88174e1389218b303/server/src/main/java/org/opensearch/snapshots/ConcurrentSnapshotExecutionException.java#L45-L62

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

p1729 commented 1 year ago

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 commented 1 year ago

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

baba-devv commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

JalODan commented 1 year ago

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

Ayan07 commented 1 year ago

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

Thanks!