-
Notifications
You must be signed in to change notification settings - Fork 314
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
Report indexing times per shard #514
Report indexing times per shard #514
Conversation
With this commit we do not only show the total indexing times (across all shards) but also report the indexing times per (primary) shard. Closes elastic#374
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! Left a suggestion to stick to append()
instead of +=
when appending items in lists.
@@ -665,19 +705,38 @@ def report_merge_part_times(self, baseline_stats, contender_stats): | |||
) | |||
|
|||
def report_total_times(self, baseline_stats, contender_stats): | |||
totals = [] | |||
totals += self.report_total_time("indexing time", |
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.
As discussed, append()
could be a better choice as +=
with lists may produce unexpected results if the input is a string vs a list.
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.
Yes, good suggestion! I'll merge this PR and do this separately.
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.
Addressed in 8d7a799.
With this commit we use a custom structure in the ES metrics store for per shard statistics instead of the generic `value.single` one. Relates #514
With this commit we specify that values should be stored as doubles instead of floats (which are implicitly assumed) in order to ensure that we can store larger values as well. Relates #514
With this commit we do not only show the total indexing times (across
all shards) but also report the indexing times per (primary) shard.
Closes #374