-
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
Move CompletionStats into the Engine #33847
Move CompletionStats into the Engine #33847
Conversation
By moving CompletionStats into the engine we can easily cache the stats for read-only engines if necessary. It also moves the responsibiltiy out of IndexShard which has quiet some complexity already. Relates to elastic#33835
Pinging @elastic/es-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, thanks. I left a minor comment
if (terms instanceof CompletionTerms) { | ||
// TODO: currently we load up the suggester for reporting its size | ||
long fstSize = ((CompletionTerms) terms).suggester().ramBytesUsed(); | ||
if (fieldNamePatterns != null && fieldNamePatterns.length > 0 && Regex.simpleMatch(fieldNamePatterns, info.name)) { |
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.
Nit: we don't need to check for non null non empty string array here as Regex.simpleMatch(fieldNamePatterns, info.name)
takes care of it
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.
By moving CompletionStats into the engine we can easily cache the stats for read-only engines if necessary. It also moves the responsibiltiy out of IndexShard which has quiet some complexity already. Relates to #33835
Completion and DocStats are pulled from internal readers instead of external since elastic#33835 and elastic#33847 which doesn't require us to refresh after a stats call since refreshes will happen internally anyhow and that will cause updated stats on ongoing indexing.
Completion and DocStats are pulled from internal readers instead of external since elastic#33835 and elastic#33847 which doesn't require us to refresh after a stats call since refreshes will happen internally anyhow and that will cause updated stats on ongoing indexing.
By moving CompletionStats into the engine we can easily cache the stats for
read-only engines if necessary. It also moves the responsibiltiy out of IndexShard
which has quiet some complexity already.
Relates to #33835