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.

Sign up for GitHub

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

Jump to bottom

Show concrete error when enrich index not exist rather than NPE #99604

Merged
merged 4 commits into from Sep 29, 2023
Merged

Show concrete error when enrich index not exist rather than NPE #99604

merged 4 commits into from Sep 29, 2023

Conversation

puppylpg
Copy link
Contributor

@puppylpg puppylpg commented Sep 15, 2023 •
edited

There should be NullPointerException check and throw index not found exception to the response so the user can understand what happens with the enrich index, rather than just "Caused by: java.lang.NullPointerException: Cannot invoke "org.elasticsearch.cluster.metadata.IndexAbstraction.getIndices()" because "ia" is null", which is confusing for user.

To solve #96122

@elasticsearchmachine elasticsearchmachine added v8.11.0 external-contributor Pull request authored by a developer outside the Elasticsearch team needs:triage Requires assignment of a team area label labels Sep 15, 2023
@pgomulka pgomulka added the :Data Management/Ingest Node Execution or management of Ingest Pipelines including GeoIP label Sep 18, 2023
@elasticsearchmachine elasticsearchmachine added Team:Data Management Meta label for data/management team and removed needs:triage Requires assignment of a team area label labels Sep 18, 2023
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-data-management (Team:Data Management)

@pgomulka pgomulka added the needs:triage Requires assignment of a team area label label Sep 18, 2023
@elasticsearchmachine elasticsearchmachine removed the needs:triage Requires assignment of a team area label label Sep 18, 2023
@jbaiera jbaiera self-requested a review September 22, 2023 17:47
@jbaiera jbaiera self-assigned this Sep 28, 2023
@jbaiera
Copy link
Member

jbaiera commented Sep 28, 2023

Thanks for contributing this! I don't know if this is for sure related to #96122 or if it just simply has the same error message, so I've left the issue unlinked in the change log entry.

@jbaiera
Copy link
Member

jbaiera commented Sep 28, 2023

@elasticmachine ok to test

@jbaiera
Copy link
Member

jbaiera commented Sep 28, 2023

@elasticmachine update branch

@puppylpg
Copy link
Contributor Author

puppylpg commented Sep 29, 2023 •
edited

Thanks for contributing this! I don't know if this is for sure related to #96122 or if it just simply has the same error message, so I've left the issue unlinked in the change log entry.

I'm sure that's the same problem because that's a colleague of mine. 😸 He met the issue during a test but have no controll of that cluster(7.17.7), then after a few months I encounter the same problem in data center migration(7.12.0 -> 7.17.6) and get the detailed error log.

Plus, after upgrading to latest 7.17.13, the problem is solved indeed. Thanks for your effort.

@jbaiera
Copy link
Member

jbaiera commented Sep 29, 2023

I'm sure that's the same problem because that's a colleague of mine. 😸

Oh nice, didn't realize. Consider my previous suspicion invalidated 😄

It looks like there's just a couple of check style issues that I should be able to clear on my own. The change LGTM, I can merge it after we get a green CI build.

jbaiera
jbaiera approved these changes Sep 29, 2023
View reviewed changes
Copy link
Member

@jbaiera jbaiera left a comment

Choose a reason for hiding this comment

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

LGTM pending green CI

@jbaiera jbaiera added auto-backport-and-merge Automatically create backport pull requests and merge when ready v7.17.14 v8.10.3 labels Sep 29, 2023
@jbaiera jbaiera merged commit ccc896d into elastic:main Sep 29, 2023
14 checks passed
@jbaiera jbaiera mentioned this pull request Sep 29, 2023
Merged
@elasticsearchmachine
Copy link
Collaborator

💔 Backport failed

Status Branch Result
❌ 7.17 Commit could not be cherrypicked due to conflicts
✅ 8.10

You can use sqren/backport to manually backport by running backport --upstream elastic/elasticsearch --pr 99604

jbaiera added a commit to jbaiera/elasticsearch that referenced this pull request Sep 29, 2023
@puppylpg @jbaiera @elasticmachine
Show concrete error when enrich index not exist rather than NPE (elas…
4968776 
…tic#99604)

There should be NullPointerException check and throw index not found exception to the response 
so the user can understand what happens with the enrich index

---------

Co-authored-by: James Baiera <james.baiera@gmail.com>
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
elasticsearchmachine pushed a commit that referenced this pull request Sep 29, 2023
@jbaiera @puppylpg @elasticmachine
Show concrete error when enrich index not exist rather than NPE (#99604…
4ac2db5 
…) (#100107)

There should be NullPointerException check and throw index not found exception to the response 
so the user can understand what happens with the enrich index

---------

Co-authored-by: puppylpg <shininglhb@163.com>
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
piergm pushed a commit to piergm/elasticsearch that referenced this pull request Oct 2, 2023
@puppylpg @jbaiera
@elasticmachine @piergm
Show concrete error when enrich index not exist rather than NPE (elas…
b2912be 
…tic#99604)

There should be NullPointerException check and throw index not found exception to the response 
so the user can understand what happens with the enrich index

---------

Co-authored-by: James Baiera <james.baiera@gmail.com>
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
@jbaiera jbaiera mentioned this pull request Oct 2, 2023
Merged
@jbaiera
Copy link
Member

jbaiera commented Oct 2, 2023

💚 All backports created successfully

Status Branch Result
✅ 7.17

Questions ?

Please refer to the Backport tool documentation

jbaiera pushed a commit to jbaiera/elasticsearch that referenced this pull request Oct 2, 2023
@puppylpg @jbaiera
Show concrete error when enrich index not exist rather than NPE (elas…
854e1a1 
…tic#99604)

There should be NullPointerException check and throw index not found exception to the response
so the user can understand what happens with the enrich index

---------

Co-authored-by: James Baiera <james.baiera@gmail.com>
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
(cherry picked from commit ccc896d)

# Conflicts:
#	x-pack/plugin/enrich/src/main/java/org/elasticsearch/xpack/enrich/EnrichCache.java
#	x-pack/plugin/enrich/src/test/java/org/elasticsearch/xpack/enrich/EnrichCacheTests.java
elasticsearchmachine pushed a commit that referenced this pull request Oct 2, 2023
@jbaiera @puppylpg
Show concrete error when enrich index not exist rather than NPE (#99604…
364c340 
…) (#100155)

There should be NullPointerException check and throw index not found exception to the response
so the user can understand what happens with the enrich index

---------

Co-authored-by: James Baiera <james.baiera@gmail.com>
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
(cherry picked from commit ccc896d)

# Conflicts:
#	x-pack/plugin/enrich/src/main/java/org/elasticsearch/xpack/enrich/EnrichCache.java
#	x-pack/plugin/enrich/src/test/java/org/elasticsearch/xpack/enrich/EnrichCacheTests.java

Co-authored-by: puppylpg <shininglhb@163.com>
jakelandis pushed a commit to jakelandis/elasticsearch that referenced this pull request Oct 2, 2023
@puppylpg @jbaiera
@elasticmachine @jakelandis
Show concrete error when enrich index not exist rather than NPE (elas…
b2c40ba 
…tic#99604)

There should be NullPointerException check and throw index not found exception to the response 
so the user can understand what happens with the enrich index

---------

Co-authored-by: James Baiera <james.baiera@gmail.com>
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jbaiera jbaiera jbaiera approved these changes

Assignees

@jbaiera jbaiera

Labels
auto-backport-and-merge Automatically create backport pull requests and merge when ready :Data Management/Ingest Node Execution or management of Ingest Pipelines including GeoIP >enhancement external-contributor Pull request authored by a developer outside the Elasticsearch team Team:Data Management Meta label for data/management team v7.17.14 v8.10.3 v8.11.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@puppylpg @elasticsearchmachine @jbaiera @masseyke @pgomulka @elasticmachine