-
Notifications
You must be signed in to change notification settings - Fork 955
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
Fix - Handle ES missing index edge case (bug fix) #2193
Merged
ozandogrultan
merged 9 commits into
amundsen-io:main
from
deliveryhero:handle-es-missing-index-edge-case
Sep 22, 2023
Merged
Fix - Handle ES missing index edge case (bug fix) #2193
ozandogrultan
merged 9 commits into
amundsen-io:main
from
deliveryhero:handle-es-missing-index-edge-case
Sep 22, 2023
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ces for any resource type Signed-off-by: Muhammad Mikaal S. Anwar <mikaalanwar@gmail.com>
Signed-off-by: Muhammad Mikaal S. Anwar <mikaalanwar@gmail.com>
Signed-off-by: Muhammad Mikaal S. Anwar <mikaalanwar@gmail.com>
Signed-off-by: Muhammad Mikaal S. Anwar <mikaalanwar@gmail.com>
Signed-off-by: Muhammad Mikaal S. Anwar <mikaalanwar@gmail.com>
mikaalanwar
requested review from
feng-tao,
jinhyukchang,
allisonsuarez,
verdan,
bolkedebruin,
mgorsk1,
youngyjd,
dechoma,
sewardgw,
dkunitsk,
kristenarmes,
ozandogrultan,
B-T-D and
a team
as code owners
September 21, 2023 23:04
Signed-off-by: Muhammad Mikaal S. Anwar <mikaalanwar@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please bump up the version in setup.py
Signed-off-by: Muhammad Mikaal S. Anwar <mikaalanwar@gmail.com>
ozandogrultan
approved these changes
Sep 22, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This pull request handles an edge case with basic search. If more than one
resource_types
is configured for amundsen (e.g Tables and Dashboards) and one of them have missing indices (i.e. no data got ingested for either of them in an initial run), it will crash the basic search from working for allresource_types
. In a clean deployment (blank neo4j and elasticsearch), it is possible to not have data for all configured resource types at once. In such a case the search should not return any results for the resource_type with missing data instead of crashing basic search altogether. This PR addresses this problem and ensures that we trigger an elasticsearch "search" against only thoseresource_types
which have a corresponding index/alias present in the elasticsearch database.Motivation and Context
For a fresh deployment of amundsen, in case any of the configured resource types is missing indices in elasticsearch, it will prevent the search from running altogether. Also, the error statement tends to be a bit confusing:
[ERROR] es_proxy_v2_1.execute_multisearch_query:349 (1:Thread-48) - Failed to execute ES search queries. TransportError(N/A, 'index_not_found_exception')
As it does not mention which particular index (or alias) was missing. In my case, dashboards were missing but I was under the impression that maybe the search service was not looking in the right place because there were some indices in my elasticsearch instance already, which were searchable using other debugging tools (such as elasticvue). I came across the aforementioned issue and wanted to push a fix for this. There wasn't a corresponding open issue against this already, but there is a reference to a similar issue here.
How Has This Been Tested?
I prepared a clean environment locally by deleting all neo4j and elasticsearch data. Next, I ran an ingestion run for only one resource type (without knowing that I'd have to run it for all
resource_types
to actually get it working). In my case, I ingested tables only, without ingesting dashboards. And then I noticed that the advanced search (sidebar) was working but the main, basic search bar wasn't. Upon investigation, I narrowed it down and implemented this fix. Now, the main/basic search works fine even if I don't ingest dashboards.Documentation
No updates to the documentation were needed.
CheckList