[Monitoring] Using primary average shard size #96177

igoristic · 2021-04-02T21:06:09Z

Resolves: #96145

Instead of using index_stats.primaries.store.size_in_bytes raw value we are calculating shard average size:

index_stats.primaries.store.size_in_bytes / index_stats.shards.primaries

elasticmachine · 2021-04-02T21:06:11Z

Pinging @elastic/stack-monitoring (Team:Monitoring)

…arge_shard_size

jasonrhodes

Just a few questions, thanks!

docs/user/monitoring/kibana-alerts.asciidoc

x-pack/plugins/monitoring/common/constants.ts

x-pack/plugins/monitoring/server/alerts/large_shard_size_alert.ts

x-pack/plugins/monitoring/server/lib/alerts/fetch_index_shard_size.ts

jasonrhodes · 2021-04-08T14:33:20Z

x-pack/plugins/monitoring/server/lib/alerts/fetch_index_shard_size.ts

      const { name: nodeName, uuid: nodeId } = sourceNode;
-      const shardSize = +(shardSizeBytes! / gbMultiplier).toFixed(2);
+      const avgShardSize = primaryShardSizeBytes ? primaryShardSizeBytes / totalPrimaryShards : 0;


We're already defaulting this value to 0 on line 145, right? 0 / totalPrimaryShards will be 0 so do we need this ternary?

Also, do we ever expect totalPrimaryShards to be 0?

Right, the ternary actually checks to make sure it's not 0 (which can be either from the response or our default), since I don't want to divide the 0 (like you mentioned). Maybe scoping would make it more readable? eg:

avgShardSize = primaryShardSizeBytes ? (primaryShardSizeBytes / totalPrimaryShards) : 0

...do we ever expect totalPrimaryShards to be 0

Yes, it's possible to have zero primaries (mainly during the allocation of shards), in which case the cluster status goes red (though in most cases it's temporary since not all shards are yet assigned). This article explains it very well

I guess I don't even need this check, since I'm already doing this right before:

if (!primaryShardSizeBytes) { continue; }

Exactly, that's what I was thinking. I asked about totalPrimaryShards because we're currently not checking that value to make sure it's not 0, so I think we can still end up in a divide by 0 scenario here, right?

No, because the condition if (primaryShardSizeBytes) would not pass because 0 is actually considered falsy in javascript/ts. So, we don't need to explicitly say if (primaryShardSizeBytes === 0)

Heh, yeah I know about 0 being falsy ;) I'm asking about the denominator, totalPrimaryShards.

Eh, I see your point now, sorry for the derp moment 🙃

Added that to the check as well, so should be good now

jasonrhodes · 2021-04-08T14:35:26Z

@ravikesarwani / @igoristic I requested we simplify the language here a bit and just say "average shard size" because that's what we're trying to calculate by dividing primary size by primary count.

If you all think that we definitely need to specify "primary" everywhere, can we say "average primary shard size" rather than "(primary average) shard size"?

ravikesarwani · 2021-04-08T18:42:23Z

From the user perspective "average shard size" is good enough.
In code I would assume we should say in the comments why we choose to use primary shard for the calculation.

…arge_shard_size

jasonrhodes

A few more questions/changes.

x-pack/plugins/monitoring/common/constants.ts

x-pack/plugins/monitoring/server/lib/alerts/fetch_index_shard_size.ts

…arge_shard_size

igoristic · 2021-04-13T14:42:07Z

@elasticmachine merge upstream

jasonrhodes

LGTM

kibanamachine · 2021-04-13T16:46:47Z

💚 Build Succeeded

Metrics [docs]

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`monitoring`	45.4KB	45.5KB	+38.0B

History

💔 Build #119050 failed 567fec3
💚 Build #118832 succeeded 0d78959
💛 Build #118809 was flaky 019e9c9
💚 Build #117430 succeeded 290259a
💚 Build #117118 succeeded 9e62f40

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

* Using shard size avg instead of primary total * Added ui text * Changed to primary average instead of total * Addressed cr feedback * Added zero check * Fixed threshold checking * Changed description Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

* Using shard size avg instead of primary total * Added ui text * Changed to primary average instead of total * Addressed cr feedback * Added zero check * Fixed threshold checking * Changed description Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

igoristic · 2021-04-14T02:17:36Z

Backport:
7.x: cc27608
7.12: 9ddd575

…ax_primary_shard_size * 'master' of github.com:elastic/kibana: (99 commits) added missing optional chain for bracket notation (elastic#96939) [Discover][DocViewer] Fix toggle columns from doc viewer table tab (elastic#95748) [TSVB] Fix per-request caching of index patterns (elastic#97043) [Datatable] Fix filter cell flakiness (elastic#96934) Unskip heatmap suite and fixes flakiness (elastic#96941) [Fleet] Improve performance of data stream API (elastic#97058) [ML] Data Frame Analytics: remove beta badge (elastic#96977) [App Search] Migrate expanded rows for meta engines table in Engines Overview (elastic#96251) Instances latency distribution chart tooltips and axis fixes (elastic#95577) [Monitoring] Using primary average shard size (elastic#96177) [Workplace Search] Hide Kibana chrome on 3rd party connector redirects (elastic#97028) ## [Security Solution] Fixes `Exit full screen` and `Copy to cliboard` styling issues (elastic#96676) Index pattern field editor - Add warning on name or type change (elastic#95528) [App Search] Add small engine breadcrumb utility helper (elastic#96917) Copy esArchiver commands from ./reassign.ts to fix tests (elastic#97012) [Security Solution][Detections] Updates MITRE Tactics, Techniques, and Subtechniques for 7.13 (elastic#97011) Index patterns server - throw correct error on field caps 404 (elastic#95879) Use `EuiThemeProvider` in lists plugin tests and stories (elastic#96129) [npm] upgrade caniuse database (elastic#97002) chore(NA): moving @kbn/apm-utils into bazel (elastic#96227) ... # Conflicts: # x-pack/plugins/index_lifecycle_management/__jest__/client_integration/edit_policy/serialization/policy_serialization.test.ts # x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/schema.ts

…to-metrics-tab * 'master' of github.com:elastic/kibana: (61 commits) [Usage collection] Usage counters (elastic#96696) UI actions readme (elastic#96925) [TSVB] Enable brush for visualizations created with no index patterns (elastic#96727) [Data telemetry] Add Async Search to the tests (elastic#96693) added missing optional chain for bracket notation (elastic#96939) [Discover][DocViewer] Fix toggle columns from doc viewer table tab (elastic#95748) [TSVB] Fix per-request caching of index patterns (elastic#97043) [Datatable] Fix filter cell flakiness (elastic#96934) Unskip heatmap suite and fixes flakiness (elastic#96941) [Fleet] Improve performance of data stream API (elastic#97058) [ML] Data Frame Analytics: remove beta badge (elastic#96977) [App Search] Migrate expanded rows for meta engines table in Engines Overview (elastic#96251) Instances latency distribution chart tooltips and axis fixes (elastic#95577) [Monitoring] Using primary average shard size (elastic#96177) [Workplace Search] Hide Kibana chrome on 3rd party connector redirects (elastic#97028) ## [Security Solution] Fixes `Exit full screen` and `Copy to cliboard` styling issues (elastic#96676) Index pattern field editor - Add warning on name or type change (elastic#95528) [App Search] Add small engine breadcrumb utility helper (elastic#96917) Copy esArchiver commands from ./reassign.ts to fix tests (elastic#97012) [Security Solution][Detections] Updates MITRE Tactics, Techniques, and Subtechniques for 7.13 (elastic#97011) ...

igoristic added 2 commits April 2, 2021 16:53

Using shard size avg instead of primary total

dbb21eb

Added ui text

9e62f40

igoristic added Team:Monitoring Stack Monitoring team v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.13.0 v7.12.1 labels Apr 2, 2021

igoristic requested a review from a team April 2, 2021 21:06

igoristic added 2 commits April 5, 2021 21:46

Merge branch 'master' of https://github.com/elastic/kibana into fix_l…

febe3f9

…arge_shard_size

Changed to primary average instead of total

290259a

igoristic changed the title ~~[Monitoring] Using average shard size~~ [Monitoring] Using primary average shard size Apr 6, 2021

jasonrhodes requested changes Apr 8, 2021

View reviewed changes

igoristic added 2 commits April 12, 2021 10:41

Merge branch 'master' of https://github.com/elastic/kibana into fix_l…

3be20e9

…arge_shard_size

Addressed cr feedback

019e9c9

igoristic requested a review from jasonrhodes April 12, 2021 16:05

Added zero check

0d78959

jasonrhodes requested changes Apr 12, 2021

View reviewed changes

x-pack/plugins/monitoring/common/constants.ts Outdated Show resolved Hide resolved

x-pack/plugins/monitoring/server/lib/alerts/fetch_index_shard_size.ts Outdated Show resolved Hide resolved

igoristic added 3 commits April 12, 2021 20:02

Fixed threshold checking

56d8a0d

Merge branch 'master' of https://github.com/elastic/kibana into fix_l…

90f8872

…arge_shard_size

Changed description

567fec3

Merge branch 'master' into fix_large_shard_size

9ffcfa2

igoristic requested a review from jasonrhodes April 13, 2021 14:42

jasonrhodes approved these changes Apr 13, 2021

View reviewed changes

igoristic merged commit 355c949 into elastic:master Apr 13, 2021

igoristic deleted the fix_large_shard_size branch April 13, 2021 21:01

This was referenced Apr 13, 2021

[7.x] [Monitoring] Using primary average shard size (#96177) #97048

Merged

[7.12] [Monitoring] Using primary average shard size (#96177) #97049

Merged

igoristic mentioned this pull request May 4, 2021

[Monitoring] Fixed parsing error in shard size query #99159

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Monitoring] Using primary average shard size #96177

[Monitoring] Using primary average shard size #96177

igoristic commented Apr 2, 2021 •

edited

Loading

elasticmachine commented Apr 2, 2021

jasonrhodes left a comment

jasonrhodes Apr 8, 2021

igoristic Apr 12, 2021

igoristic Apr 12, 2021

jasonrhodes Apr 12, 2021

igoristic Apr 12, 2021

jasonrhodes Apr 12, 2021

igoristic Apr 12, 2021

jasonrhodes commented Apr 8, 2021

ravikesarwani commented Apr 8, 2021

jasonrhodes left a comment

igoristic commented Apr 13, 2021

jasonrhodes left a comment

kibanamachine commented Apr 13, 2021

igoristic commented Apr 14, 2021

[Monitoring] Using primary average shard size #96177

[Monitoring] Using primary average shard size #96177

Conversation

igoristic commented Apr 2, 2021 • edited Loading

elasticmachine commented Apr 2, 2021

jasonrhodes left a comment

Choose a reason for hiding this comment

jasonrhodes Apr 8, 2021

Choose a reason for hiding this comment

igoristic Apr 12, 2021

Choose a reason for hiding this comment

igoristic Apr 12, 2021

Choose a reason for hiding this comment

jasonrhodes Apr 12, 2021

Choose a reason for hiding this comment

igoristic Apr 12, 2021

Choose a reason for hiding this comment

jasonrhodes Apr 12, 2021

Choose a reason for hiding this comment

igoristic Apr 12, 2021

Choose a reason for hiding this comment

jasonrhodes commented Apr 8, 2021

ravikesarwani commented Apr 8, 2021

jasonrhodes left a comment

Choose a reason for hiding this comment

igoristic commented Apr 13, 2021

jasonrhodes left a comment

Choose a reason for hiding this comment

kibanamachine commented Apr 13, 2021

💚 Build Succeeded

Metrics [docs]

Page load bundle

History

igoristic commented Apr 14, 2021

igoristic commented Apr 2, 2021 •

edited

Loading