Skip to content

Commit

Permalink
[8.6] [Stack Monitoring] Use doc ID to deduplicate shard allocations (#…
Browse files Browse the repository at this point in the history 
…143963) (#145520)

# Backport

This will backport the following commits from `main` to `8.6`:
- [[Stack Monitoring] Use doc ID to deduplicate shard allocations
(#143963)](#143963)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Milton
Hultgren","email":"[email protected]"},"sourceCommit":{"committedDate":"2022-11-17T09:44:58Z","message":"[Stack
Monitoring] Use doc ID to deduplicate shard allocations (#143963)\n\n##
Summary\r\n\r\nThis PR depends on Metricbeat changes introduced in this
PR\r\nhttps://github.com/elastic/beats/pull/33457 or the Internal
collection\r\nchanges introduced
in\r\nhttps://github.com/elastic/elasticsearch/pull/91153.\r\n\r\nThe
above PR fixes a bug that makes Metricbeat properly report on all\r\nthe
shards in an index, but our shard legend had the same kind of bug,\r\nit
only displayed the first replica because it didn't account for
the\r\nreplica \"id\" in the function that tries to deduplicate the
results.\r\n\r\nSince the above PR ensures we report each shard with a
unique document\r\nID, we change the Kibana code to use the document ID
instead of trying\r\nto regenerate a unique ID.\r\n\r\n### How to
test\r\nGet the changes: `git fetch
[email protected]:miltonhultgren/kibana.git\r\nsm-shard-allocations:sm-shard-allocations
&& git switch\r\nsm-shard-allocations`\r\n\r\nStart Elasticsearch,
create an index like this:\r\n```\r\nPUT /some-index\r\n{\r\n
\"settings\": {\r\n \"index\": {\r\n \"number_of_shards\": 3, \r\n
\"number_of_replicas\": 3\r\n }\r\n }\r\n}\r\n```\r\n\r\nRun Metricbeat
with `xpack.enabled: true`. \r\n\r\nVisit the Index details page for
`some-index` in the Stack Monitoring\r\napp and verify that on a single
node cluster you have 12 shards, 9 of\r\nwhich are
unassigned.\r\n\r\n<img width=\"753\" alt=\"Screenshot 2022-10-25 at 17
19
25\"\r\nsrc=\"https://user-images.githubusercontent.com/2564140/197819368-8ea45e1c-7472-4e15-9267-3b5d73378f2a.png\">\r\n\r\nCo-authored-by:
Kibana Machine
<[email protected]>","sha":"b7debc839ff6923923fe9a41355bd0318e04cdca","branchLabelMapping":{"^v8.7.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Team:Infra
Monitoring UI","release_note:skip","Feature:Stack
Monitoring","backport:prev-minor","v8.7.0"],"number":143963,"url":"https://github.com/elastic/kibana/pull/143963","mergeCommit":{"message":"[Stack
Monitoring] Use doc ID to deduplicate shard allocations (#143963)\n\n##
Summary\r\n\r\nThis PR depends on Metricbeat changes introduced in this
PR\r\nhttps://github.com/elastic/beats/pull/33457 or the Internal
collection\r\nchanges introduced
in\r\nhttps://github.com/elastic/elasticsearch/pull/91153.\r\n\r\nThe
above PR fixes a bug that makes Metricbeat properly report on all\r\nthe
shards in an index, but our shard legend had the same kind of bug,\r\nit
only displayed the first replica because it didn't account for
the\r\nreplica \"id\" in the function that tries to deduplicate the
results.\r\n\r\nSince the above PR ensures we report each shard with a
unique document\r\nID, we change the Kibana code to use the document ID
instead of trying\r\nto regenerate a unique ID.\r\n\r\n### How to
test\r\nGet the changes: `git fetch
[email protected]:miltonhultgren/kibana.git\r\nsm-shard-allocations:sm-shard-allocations
&& git switch\r\nsm-shard-allocations`\r\n\r\nStart Elasticsearch,
create an index like this:\r\n```\r\nPUT /some-index\r\n{\r\n
\"settings\": {\r\n \"index\": {\r\n \"number_of_shards\": 3, \r\n
\"number_of_replicas\": 3\r\n }\r\n }\r\n}\r\n```\r\n\r\nRun Metricbeat
with `xpack.enabled: true`. \r\n\r\nVisit the Index details page for
`some-index` in the Stack Monitoring\r\napp and verify that on a single
node cluster you have 12 shards, 9 of\r\nwhich are
unassigned.\r\n\r\n<img width=\"753\" alt=\"Screenshot 2022-10-25 at 17
19
25\"\r\nsrc=\"https://user-images.githubusercontent.com/2564140/197819368-8ea45e1c-7472-4e15-9267-3b5d73378f2a.png\">\r\n\r\nCo-authored-by:
Kibana Machine
<[email protected]>","sha":"b7debc839ff6923923fe9a41355bd0318e04cdca"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v8.7.0","labelRegex":"^v8.7.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/143963","number":143963,"mergeCommit":{"message":"[Stack
Monitoring] Use doc ID to deduplicate shard allocations (#143963)\n\n##
Summary\r\n\r\nThis PR depends on Metricbeat changes introduced in this
PR\r\nhttps://github.com/elastic/beats/pull/33457 or the Internal
collection\r\nchanges introduced
in\r\nhttps://github.com/elastic/elasticsearch/pull/91153.\r\n\r\nThe
above PR fixes a bug that makes Metricbeat properly report on all\r\nthe
shards in an index, but our shard legend had the same kind of bug,\r\nit
only displayed the first replica because it didn't account for
the\r\nreplica \"id\" in the function that tries to deduplicate the
results.\r\n\r\nSince the above PR ensures we report each shard with a
unique document\r\nID, we change the Kibana code to use the document ID
instead of trying\r\nto regenerate a unique ID.\r\n\r\n### How to
test\r\nGet the changes: `git fetch
[email protected]:miltonhultgren/kibana.git\r\nsm-shard-allocations:sm-shard-allocations
&& git switch\r\nsm-shard-allocations`\r\n\r\nStart Elasticsearch,
create an index like this:\r\n```\r\nPUT /some-index\r\n{\r\n
\"settings\": {\r\n \"index\": {\r\n \"number_of_shards\": 3, \r\n
\"number_of_replicas\": 3\r\n }\r\n }\r\n}\r\n```\r\n\r\nRun Metricbeat
with `xpack.enabled: true`. \r\n\r\nVisit the Index details page for
`some-index` in the Stack Monitoring\r\napp and verify that on a single
node cluster you have 12 shards, 9 of\r\nwhich are
unassigned.\r\n\r\n<img width=\"753\" alt=\"Screenshot 2022-10-25 at 17
19
25\"\r\nsrc=\"https://user-images.githubusercontent.com/2564140/197819368-8ea45e1c-7472-4e15-9267-3b5d73378f2a.png\">\r\n\r\nCo-authored-by:
Kibana Machine
<[email protected]>","sha":"b7debc839ff6923923fe9a41355bd0318e04cdca"}}]}]
BACKPORT-->

Co-authored-by: Milton Hultgren <[email protected]>
  • Loading branch information
@kibanamachine @miltonhultgren
kibanamachine and miltonhultgren authored Nov 17, 2022
1 parent 490e0a5 commit cd4e770
Show file tree
Hide file tree
Showing 4 changed files with 8 additions and 4 deletions.
1 change: 1 addition & 0 deletions x-pack/plugins/monitoring/common/types/es.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ export interface ElasticsearchResponse {
}

export interface ElasticsearchResponseHit {
_id: string;
_index: string;
_source: ElasticsearchSource;
inner_hits?: {
Expand Down
5 changes: 4 additions & 1 deletion x-pack/plugins/monitoring/server/lib/elasticsearch/shards/get_shard_allocation.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,11 @@ describe('get_shard_allocation', () => {
describe('handleResponse', () => {
it('deduplicates shards', () => {
const nextTimestamp = '2018-07-06T00:00:01.259Z';
const hits = shards.map((shard) => {
const hits = shards.map((shard, index) => {
return {
_id: `STATE_UUID:${shard.node}:${shard.index}:s${shard.shard}:${
shard.primary ? 'p' : `r${index}`
}`,
_source: {
...exampleShardSource,
shard,
Expand Down
5 changes: 2 additions & 3 deletions x-pack/plugins/monitoring/server/lib/elasticsearch/shards/get_shard_allocation.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,8 @@ export function handleResponse(response: ElasticsearchResponse) {
mbShard?.shard?.relocating_node?.id ?? legacyShard?.relocating_node ?? null;
const node = mbShard?.node?.id ?? legacyShard?.node;
// note: if the request is for a node, then it's enough to deduplicate without primary, but for indices it displays both
const shardId = `${index}-${shardNumber}-${primary}-${relocatingNode}-${node}`;

if (!uniqueShards.has(shardId)) {
if (!uniqueShards.has(hit._id)) {
// @ts-ignore
shards.push({
index,
Expand All @@ -48,7 +47,7 @@ export function handleResponse(response: ElasticsearchResponse) {
shard: shardNumber,
state: legacyShard?.state ?? mbShard?.shard?.state,
});
uniqueShards.add(shardId);
uniqueShards.add(hit._id);
}
}

Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/monitoring/server/lib/logstash/get_node_info.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ jest.mock('../../static_globals', () => ({
// deletes, adds, or updates the properties based on a default object
function createResponseObjHit(params?: HitParams[]): ElasticsearchResponseHit {
const defaultResponseObj: ElasticsearchResponseHit = {
_id: '123123a',
_index: 'index',
_source: {
cluster_uuid: '123',
Expand Down

0 comments on commit cd4e770

Please sign in to comment.