Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[Index Management] Filter system indices from cat API request #104155

Merged
merged 4 commits into from
Jul 2, 2021
Merged

[Index Management] Filter system indices from cat API request #104155

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b4d1c18
filter system indices from cat API request
alisonelizabeth Jul 1, 2021
bd58d40
refactor indices test and unskip
alisonelizabeth Jul 1, 2021
a646d2a
remove old comment
alisonelizabeth Jul 1, 2021
3681f57
Use reduce to retain original number of loops and vars.
cjcenizal Jul 1, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions x-pack/plugins/index_management/server/lib/fetch_indices.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,12 @@ async function fetchIndicesCall(
query: catQuery,
});

// The two responses should be equal in the number of indices returned
return catHits.map((hit) => {
// System indices may show up in _cat APIs, as these APIs are primarily used for troubleshooting
// For now, we filter them out and only return index information for the indices we have
// In the future, we should migrate away from using cat APIs (https://github.com/elastic/kibana/issues/57286)
const filteredCatHits = catHits.filter((hit) => typeof indices[hit.index] !== 'undefined');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might be a bit silly, but I'm wondering if performance could be a concern. We know users might have hundreds of thousands (maybe millions) of indices. In this case we are doubling memory consumption by creating the filteredCatHits variable, and also doubling the number of iterations by adding another filter loop. There's a good chance that testing could reveal this to be a non-issue. But we could also just make the code immune to these concerns by sticking with a single loop and doing our undefined check within the loop:

return catHits.reduce((decoratedIndices, hit) => {
  const index = indices[hit.index];
  
  if (typeof index !== 'undefined') {
    const aliases = Object.keys(index.aliases);

    decoratedIndices.push({
      health: hit.health,
      status: hit.status,
      name: hit.index,
      uuid: hit.uuid,
      primary: hit.pri,
      replica: hit.rep,
      documents: hit['docs.count'],
      size: hit['store.size'],
      isFrozen: hit.sth === 'true', // sth value coming back as a string from ES
      aliases: aliases.length ? aliases : 'none',
      hidden: index.settings.index.hidden === 'true',
      data_stream: index.data_stream,
    });
  }
  
  return decoratedIndices;
}, []);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 great point. Thanks for fixing!


return filteredCatHits.map((hit) => {
const index = indices[hit.index];
const aliases = Object.keys(index.aliases);

Expand Down
16 changes: 12 additions & 4 deletions x-pack/test/api_integration/apis/management/index_management/indices.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,12 +177,19 @@ export default function ({ getService }) {
});
});

// FLAKY: https://github.com/elastic/kibana/issues/64473
describe.skip('list', function () {
describe('list', function () {
this.tags(['skipCloud']);

it('should list all the indices with the expected properties and data enrichers', async function () {
const { body } = await list().expect(200);
// Create an index that we can assert against
await createIndex('test_index');

// Verify indices request
const { body: indices } = await list().expect(200);

// Find the "test_index" created to verify expected keys
const indexCreated = indices.find((index) => index.name === 'test_index');

const expectedKeys = [
'health',
'hidden',
Expand All @@ -203,7 +210,8 @@ export default function ({ getService }) {
// We need to sort the keys before comparing then, because race conditions
// can cause enrichers to register in non-deterministic order.
const sortedExpectedKeys = expectedKeys.sort();
const sortedReceivedKeys = Object.keys(body[0]).sort();
const sortedReceivedKeys = Object.keys(indexCreated).sort();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm hoping that by asserting against a specific index this test will be more stable. It's possible for an index to also have a date_stream property (which is not included in the expectedKeys), so this may have been occasionally failing because the index return from body[0] had a date stream associated with. Related comment: #64473 (comment).


expect(sortedReceivedKeys).to.eql(sortedExpectedKeys);
});
});
Expand Down