-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Search Index Optimizations #18649
Search Index Optimizations #18649
Conversation
Quality Gate passed for 'open-metadata-ingestion'Issues Measures |
for (int i = 0; i < response.getItems().length; i++) { | ||
BulkItemResponse itemResponse = response.getItems()[i]; | ||
if (itemResponse.isFailed()) { | ||
String failureMessage = itemResponse.getFailureMessage(); | ||
String entityData = requests.get(i).toString(); | ||
entityErrorList.add( | ||
new EntityError().withMessage(failureMessage).withEntity(entityData)); | ||
LOG.warn("Bulk item failed: {}", failureMessage); | ||
} | ||
} |
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.
What do you think of using a Stream here like this (caveat i have not tested it)
entityErrorList = response.getItems().length > 0 ? response.getItems()
.stream()
.filter(BulkItemResponse::isFailed)
.map(
item -> {
int idx = Arrays.asList(response.getItems()).indexOf(item);
String failureMessage = itemResponse.getFailureMessage()
EntityError().withMessage(itemResponse.getFailureMessage()).withEntity(request.get(idx).toString)
LOG.warn("Bulk item failed: {}", failureMessage);
}).toList() : entityErrorList;
Also do you know if List<EntityError> entityErrorList
will be mutable and how it will behave with the multithreading?
for (BulkItemResponse item : response.getItems()) { | ||
if (item.isFailed()) { | ||
failed++; | ||
} | ||
} |
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.
can we do something like
int failed = entityErrorList.size()
I believe this prevents us from looping twice over the response
for (int i = 0; i < response.getItems().length; i++) { | ||
BulkItemResponse itemResponse = response.getItems()[i]; | ||
if (itemResponse.isFailed()) { | ||
String failureMessage = itemResponse.getFailureMessage(); | ||
String entityData = bulkRequest.requests().get(i).toString(); | ||
entityErrorList.add( | ||
new EntityError().withMessage(failureMessage).withEntity(entityData)); | ||
LOG.warn("Bulk item failed on retry {}: {}", attempt, failureMessage); | ||
} | ||
} |
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.
Same question here https://github.com/open-metadata/OpenMetadata/pull/18649/files#r1846975047
for (BulkItemResponse item : response.getItems()) { | ||
if (item.isFailed()) { | ||
failed++; | ||
} | ||
} |
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.
same question here https://github.com/open-metadata/OpenMetadata/pull/18649/files#r1846981743
Describe your changes:
Fixes
I worked on ... because ...
Type of change:
Checklist:
Fixes <issue-number>: <short explanation>