Skip to content

[CCR] Do not unnecessarily wrap fetch exception in a ElasticSearch exception and#33777

Merged
martijnvg merged 3 commits intoelastic:masterfrom
martijnvg:ccr_fetch_exception_reporting
Sep 17, 2018
Merged

[CCR] Do not unnecessarily wrap fetch exception in a ElasticSearch exception and#33777
martijnvg merged 3 commits intoelastic:masterfrom
martijnvg:ccr_fetch_exception_reporting

Conversation

@martijnvg
Copy link
Member

properly map fetch_exception.exception field as object.

The extra caused_by level is not necessary here:

"fetch_exceptions": [
              {
                "from_seq_no": 1,
                "retries": 106,
                "exception": {
                  "type": "exception",
                  "reason": "[index1] IndexNotFoundException[no such index]",
                  "caused_by": {
                    "type": "index_not_found_exception",
                    "reason": "no such index",
                    "index_uuid": "_na_",
                    "index": "index1"
                  }
                }
              }
            ],

…ception and

properly map fetch_exception.exception field as object.

The extra caused by level is not necessary here:

```
"fetch_exceptions": [
              {
                "from_seq_no": 1,
                "retries": 106,
                "exception": {
                  "type": "exception",
                  "reason": "[index1] IndexNotFoundException[no such index]",
                  "caused_by": {
                    "type": "index_not_found_exception",
                    "reason": "no such index",
                    "index_uuid": "_na_",
                    "index": "index1"
                  }
                }
              }
            ],
```
@martijnvg martijnvg added review :Distributed/CCR Issues around the Cross Cluster State Replication features labels Sep 17, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed

@martijnvg
Copy link
Member Author

Just changed exception field mapping to be a type nested by request from @ycombinator

Copy link
Member

@jasontedor jasontedor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

},
"exception": {
"type": "text"
"type": "nested",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @martijnvg, I was actually wondering if fetch_exceptions (not exception) should be nested as it contains an array of objects. Thoughts?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ycombinator I pushed: 1797b73

Copy link
Contributor

@ycombinator ycombinator left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The mapping change for fetch_exceptions (not the main intent of this PR) LGTM.

@martijnvg martijnvg merged commit 15f30d6 into elastic:master Sep 17, 2018
martijnvg added a commit that referenced this pull request Sep 17, 2018
…ception and (#33777)

* [CCR] Do not unnecessarily wrap fetch exception in a ElasticSearch exception and
properly map fetch_exception.exception field as object.

The extra caused by level is not necessary here:

```
"fetch_exceptions": [
              {
                "from_seq_no": 1,
                "retries": 106,
                "exception": {
                  "type": "exception",
                  "reason": "[index1] IndexNotFoundException[no such index]",
                  "caused_by": {
                    "type": "index_not_found_exception",
                    "reason": "no such index",
                    "index_uuid": "_na_",
                    "index": "index1"
                  }
                }
              }
            ],
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Distributed/CCR Issues around the Cross Cluster State Replication features

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants