-
Notifications
You must be signed in to change notification settings - Fork 24.9k
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
System indices auto-create for searchable-snapshots #66940
System indices auto-create for searchable-snapshots #66940
Conversation
Backport of elastic#66276. Part of elastic#61656. Use the system indices auto-creation infrastructure for the searchable snapshots plugin.
Pinging @elastic/es-distributed (Team:Distributed) |
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.
LGTM but I've left minor comments
); | ||
@Override | ||
public void onFailure(Exception e) { | ||
logger.debug(new ParameterizedMessage("failure in cache fill: [{}]", request.id()), e); |
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.
We can use debug(Supplier<?> msgSupplier, Throwable t)
}); | ||
}); | ||
} catch (Exception e) { | ||
logger.warn(new ParameterizedMessage("cache fill failure: [{}]", CachedBlob.generateId(repository, name, path, offset)), e); |
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.
We should use warn(Supplier<?> msgSupplier, Throwable t)
@tlrx thanks for the review. I won't change those logging methods as they are the same in |
Thanks. I think we can change them in master too. |
Backport of #66276.
Part of #61656. Use the system indices auto-creation infrastructure
for the searchable snapshots plugin.