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.

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

[ILM] Data tiers for 7.10 #76126

Merged
merged 62 commits into from
Sep 18, 2020
Merged

Conversation

jloleysens
Copy link
Contributor

@jloleysens jloleysens commented Aug 27, 2020

Summary

Adds support for data tiers in ILM.

TODO

Screenshots

Screenshots

Select expanded

Screenshot 2020-09-10 at 17 24 37

Default state (warm tier)

Screenshot 2020-09-10 at 17 07 49

Custom data tier allocation activated

Screenshot 2020-09-10 at 17 07 56

Data tier allocation off (migration { enabled: false } } )

Screenshot 2020-09-10 at 17 08 03

No node roles found warning

Screenshot 2020-09-10 at 17 09 48

No node attrs found warning (pre-existing)

Screenshot 2020-09-10 at 17 14 08

How to test

  1. Start Kibana with yarn es snapshot -E node.attr.rack_id=rack_one (this will enable you to use the custom allocation selector
  2. Navigate to the ILM policy UI and either edit or create a new policy
  3. In the create/edit UI make sure to use Warm/Cold/Frozen phase as this is where the new form field was added for selecting data tier allocation
  4. Confirm that selecting "Default" does not add any allocate action to the phase in question
  5. Confirm that selecting "Off" adds migrate: { enabled: false } to the policy actions section
  6. Confirm that selecting "Custom" enables you to select the node attribute you started the cluster with

Note: the "Off" option should only start working once elastic/elasticsearch#61377 is merged.

Checklist

Delete any items that are not applicable to this PR.

For maintainers

This reverts commit 54b6f7ff3ec8b0f57b150ab2276d617686da9fb5.
This reverts commit 63868b44ec60d7431c3a0189b16aeece1db2d38e.
- also moved node attr and node allocation component to inside
  new folder that contains all allocation components.
- only focussed on updating warm phase for now
- The described form group now has a switch for showing
  controls on the left.
- Refactored DataTierAllocation to CustomDataTierAllocation
- Removed 'node-roles' option
- Updated copy
- Moved JSX around a bit in the edit policy section to make logic
  simpler
- Still only implemented for warm, cold and frozen are still
  under way
@jloleysens jloleysens added release_note:enhancement Feature:ILM v8.0.0 Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more v7.10.0 labels Aug 27, 2020
@jloleysens
Copy link
Contributor Author

@elasticmachine merge upstream

@elasticmachine
Copy link
Contributor

merge conflict between base and head

@jloleysens
Copy link
Contributor Author

@elasticmachine merge upstream

@elasticmachine
Copy link
Contributor

merge conflict between base and head

…s-for-710

* 'master' of github.com:elastic/kibana: (43 commits)
  [APM] Chart units don't update when toggling the chart legends (elastic#74931)
  [ILM] Add support for frozen phase in UI (elastic#75968)
  Hides advanced json for count metric (elastic#74636)
  add client-side feature usage API (elastic#75486)
  [Maps] add drilldown support map embeddable (elastic#75598)
  [Enterprise Search] Request handler refactors/enhancements + update existing routes to use shared handler (elastic#76106)
  [Resolver] model `location.search` in redux (elastic#76140)
  [APM] Prevent imports of public in server code (elastic#75979)
  fix eslint issue
  skip flaky suite (elastic#76223)
  [APM] Transaction duration anomaly alerting integration (elastic#75719)
  [Transforms] Avoid using "Are you sure" (elastic#75932)
  [Security Solution][Exceptions] - Fix bug of alerts not updating after closure from exceptions modal (elastic#76145)
  [plugin-helpers] improve 3rd party KP plugin support (elastic#75019)
  [docs/getting-started] link to yarn v1 specifically (elastic#76169)
  [Security_Solution][Resolver] Resolver loading and error state (elastic#75600)
  Fixes App Search documentation links (elastic#76133)
  Fix alerts unable to create / update when the name has trailing whitepace(s) (elastic#76079)
  [Resolver] Fix useSelector usage (elastic#76129)
  [Enterprise Search] Migrate util and components from ent-search (elastic#76051)
  ...

# Conflicts:
#	x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/data_tier_allocation/node_allocation.tsx
#	x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/edit_policy.tsx
#	x-pack/plugins/index_lifecycle_management/public/application/services/policies/types.ts
#	x-pack/plugins/index_lifecycle_management/public/application/services/policies/warm_phase.ts
- Made types more explicit 'default', 'custom' and 'none'
- Fixed issue introduced by use useCallback on state setter -
  need to use the function setter pattern to not have stale data
  being set.
node roles.

- Refactored way we get node data to a provider component so that
  phases still have flexibility in how they render. Currently
  this also means that we fetch node stats data for each phase
  we render
- Created a callout for when there is no node role to which data
  can be allocated for the default setting
- Also updated the behaviour to render the entire form even when
  we cannot fetch node data for some reason. It is not ideal to
  not have node data, but we should not block the entire form.
@jloleysens
Copy link
Contributor Author

@elasticmachine merge upstream

Copy link
Contributor

@mdefazio mdefazio left a comment

Choose a reason for hiding this comment

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

I think this looks good on the design side of things! There could perhaps be more description on the left side regarding data allocation with a doc link, but I know there have been several additional comments for copy already.

Copy link
Contributor

@alisonelizabeth alisonelizabeth left a comment

Choose a reason for hiding this comment

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

Nice work @jloleysens! Tested locally. Everything LGTM. Left a few non-blocking comments. Thanks for adding tests!

@@ -4,4 +4,10 @@
* you may not use this file except in compliance with the Elastic License.
*/

export * from './api';

export type DataTierNodeRole = 'data' | 'data_hot' | 'data_warm' | 'data_cold' | 'data_frozen';
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need both NodeRole and DataTierNodeRole (looks like they contain the same values)?

@@ -0,0 +1,41 @@
/*
Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't look like this file is being used anywhere?

@@ -100,6 +103,9 @@ export interface AllocateAction {
require?: {
[attribute: string]: string;
};
migrate?: {
enabled: false;
Copy link
Contributor

Choose a reason for hiding this comment

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

Was setting false here intentional?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes it was, :). The default will be for enabled to be true so the only reason to ever specify this at the moment is to turn migration off. I will add a comment to that effect 👍

@debadair
Copy link
Contributor

@cjcenizal has some good points. I know what we're talking about, and "Data tier allocation" is hard for me to parse. On its own, "allocation" is a fairly understandable term, but "Data tier allocation" definitely sounds like jargon.

I think it would be more approachable if we just use "Data allocation", and talk in terms of moving data instead of reallocating shards:

Data allocation
Move warm data to nodes optimized for read-only access.

Similarly, the default option can focus on the node role, rather than the style of allocation, and the descriptions can use the "move" terminology instead of reallocate:

Use warm nodes (recommended)
Move data to nodes in the warm tier.

Custom
Move data based on node attributes.

None
Do not move data in the warm phase.

In the selected state, I think it would be better to use the same language, and introduce the allocation types in the context of the "learn more" links:

Use warm nodes (recommended)
Move data to nodes in the warm tier.
Learn more about role-based shard allocation.

Custom
Move data based on node attributes.
Learn more about attribute-based shard allocation.

None
Do not move data in the warm phase.
Learn more about shard allocation.

I think the imperative form for the error messages works better, too:

No nodes assigned to the warm tier
Assign at least one node to the warm tier
to use role-based allocation. The policy
will fail to complete allocation if there are no warm nodes.

No node attributes configured
Define custom node attributes in elasticsearch.yml to use
attribute-based allocation. Warm nodes
will be used instead.

…s-for-710

* 'master' of github.com:elastic/kibana: (95 commits)
  log request body in new ES client (elastic#77150)
  use `navigateToUrl` to navigate to recent nav links (elastic#77446)
  Move core config service to `kbn/config` package (elastic#76874)
  [UBI] Copy license to /licenses folder (elastic#77563)
  Skip flaky Events Viewer Cypress test
  [Lens] Remove dynamic names in telemetry fields (elastic#76988)
  [Maps] Add DynamicStyleProperty#getMbPropertyName and DynamicStyleProperty#getMbPropertyValue (elastic#77366)
  [Enterprise Search] Add flag to restrict width of layout (elastic#77539)
  [Security Solutions][Cases - Timeline] Fix bug when adding a timeline to a case (elastic#76967)
  [Security Solution][Detections] Integration test for Editing a Rule (elastic#77090)
  [Ingest pipelines] Polish pipeline debugging workflow (elastic#76058)
  [@kbn/utils] Adds missing dependency (elastic#77536)
  Add the Enterprise Search logo to the Overview search result (elastic#77514)
  [Logs UI] [Metrics UI] Remove saved object field mappings (elastic#75482)
  [Security Solution][Exceptions] Exception modal bulk close option only closes alerts generated by same rule (elastic#77402)
  Adds @kbn/utils package (elastic#76518)
  Map layout changes (elastic#77132)
  [Security Solution] [Detections] EQL Rule Creation (elastic#76831)
  Adding test user to maps tests under documents source folder  (elastic#77245)
  Suppress error logs when clients connect over HTTP instead of HTTPS (elastic#77397)
  ...

# Conflicts:
#	x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/index.ts
#	x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/phases/warm_phase.tsx
Also deleted unused component "AdvancedSection" for now
@jloleysens
Copy link
Contributor Author

@debadair

Thanks for the copy review. I have addressed your suggestions in 50a9651.

Data allocation
Move warm data to nodes optimized for read-only access.

I think it would still be really nice to mention something about cost of hardware for nodes in the different tiers. So I took your suggestion and landed on for the phase data allocation description:

"Move data to data nodes optimized for less frequent, read-only access. Cold data can be stored on inexpensive hardware."

Let me know what you think.

Use warm nodes (recommended)

I also like the use of "recommended" in the title to emphasise this option over the other options.

Would you mind taking another look at reviewing the changes?

@jloleysens
Copy link
Contributor Author

@alisonelizabeth Thanks for the review! I believe I addressed all of your feedback in the last series of commits (d695e95, 36e4a45)

Copy link
Contributor

@debadair debadair left a comment

Choose a reason for hiding this comment

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

Left a few additional suggestions, but I think this looks good.

'xpack.indexLifecycleMgmt.coldPhase.dataTier.defaultAllocationNotAvailableBody',
{
defaultMessage:
'This policy will not complete allocation because there are no cold nodes. Assign at least one node to the cold tier.',
Copy link
Contributor

Choose a reason for hiding this comment

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

Apply the same changes for the cold tier:

Suggested change
'This policy will not complete allocation because there are no cold nodes. Assign at least one node to the cold tier.',
'Assign at least one node to the cold tier to use role-based allocation. The policy will fail to complete allocation if there are no cold nodes.,

'xpack.indexLifecycleMgmt.frozenPhase.dataTier.defaultAllocationNotAvailableBody',
{
defaultMessage:
'This policy will not complete allocation because there are no frozen nodes. Assign at least one node to the frozen tier.',
Copy link
Contributor

Choose a reason for hiding this comment

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

Apply same changes to the frozen tier:

Suggested change
'This policy will not complete allocation because there are no frozen nodes. Assign at least one node to the frozen tier.',
'Assign at least one node to the frozen tier to use role-based allocation. The policy will fail to complete allocation if there are no frozen nodes.',


const i18nTexts = {
title: i18n.translate('xpack.indexLifecycleMgmt.editPolicy.nodeAttributesMissingLabel', {
defaultMessage: 'No node attributes configured in elasticsearch.yml',
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
defaultMessage: 'No node attributes configured in elasticsearch.yml',
defaultMessage: 'No custom node attributes configured',

dataTierAllocation: {
description: i18n.translate('xpack.indexLifecycleMgmt.coldPhase.dataTier.description', {
defaultMessage:
'Move data to data nodes optimized for less frequent, read-only access. Cold data can be stored on inexpensive hardware.',
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
'Move data to data nodes optimized for less frequent, read-only access. Cold data can be stored on inexpensive hardware.',
'Move data to data nodes optimized for less frequent, read-only access. Store colder data on less-expensive hardware.',

dataTierAllocation: {
description: i18n.translate('xpack.indexLifecycleMgmt.frozenPhase.dataTier.description', {
defaultMessage:
'Move data to data nodes optimized for infrequent, read-only access. Frozen data can be stored on the least expensive hardware.',
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
'Move data to data nodes optimized for infrequent, read-only access. Frozen data can be stored on the least expensive hardware.',
'Move data to data nodes optimized for infrequent, read-only access. Store frozen data on the least-expensive hardware.',

dataTierAllocation: {
description: i18n.translate('xpack.indexLifecycleMgmt.warmPhase.dataTier.description', {
defaultMessage:
'Move warm data to nodes optimized for read-only access. Warm data can be stored on less expensive hardware.',
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
'Move warm data to nodes optimized for read-only access. Warm data can be stored on less expensive hardware.',
'Move warm data to nodes optimized for read-only access. Store warm data on less-expensive hardware.',

@jloleysens
Copy link
Contributor Author

@elasticmachine merge upstream

@jloleysens
Copy link
Contributor Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

@kbn/optimizer bundle module count

id value diff baseline
indexLifecycleManagement 207 +134 73

async chunks size

id value diff baseline
indexLifecycleManagement 242.4KB +55.8KB 186.6KB

page load bundle size

id value diff baseline
indexLifecycleManagement 232.5KB +176.0B 232.4KB

distributable file count

id value diff baseline
default 45915 +6 45909

History

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

@jloleysens jloleysens merged commit 63bb3bf into elastic:master Sep 18, 2020
@jloleysens jloleysens deleted the ilm/data-tiers-for-710 branch September 18, 2020 10:14
jloleysens added a commit to jloleysens/kibana that referenced this pull request Sep 18, 2020
* wip

* Revert "wip"

This reverts commit 54b6f7ff3ec8b0f57b150ab2276d617686da9fb5.

* Revert "Revert "wip""

This reverts commit 63868b44ec60d7431c3a0189b16aeece1db2d38e.

* Refactor to using EUI button group component

- also moved node attr and node allocation component to inside
  new folder that contains all allocation components.
- only focussed on updating warm phase for now

* WIP: moved form UX more in line with EUI

- The described form group now has a switch for showing
  controls on the left.
- Refactored DataTierAllocation to CustomDataTierAllocation
- Removed 'node-roles' option
- Updated copy
- Moved JSX around a bit in the edit policy section to make logic
  simpler

* Refactor UI to reflect custom-ness of "Custom" and "None" options

- Still only implemented for warm, cold and frozen are still
  under way

* server side changes for getting node data

* double opt-in

* Refactored data tier allocation type

- Made types more explicit 'default', 'custom' and 'none'
- Fixed issue introduced by use useCallback on state setter -
  need to use the function setter pattern to not have stale data
  being set.

* Some refacoring, but main point is to add warning detection for
node roles.

- Refactored way we get node data to a provider component so that
  phases still have flexibility in how they render. Currently
  this also means that we fetch node stats data for each phase
  we render
- Created a callout for when there is no node role to which data
  can be allocated for the default setting
- Also updated the behaviour to render the entire form even when
  we cannot fetch node data for some reason. It is not ideal to
  not have node data, but we should not block the entire form.

* fix i18n

* fix type issue with deafult policies missing allocation type

* remove "undefined" as option for setting phase data

* Create referentially stable data setter for all phases - prevent infini-update

* fix type issue

* refactor data -> nodesData

* refactor cold phase for data tiers

* refactor frozen phase for data tiers

* fixed existing tests for warm section

* restored existing test coverage for cold phase and added test coverage for frozen

* fix api integration test

* remove unused translations

* slight UX update to turning on custom attribute allocation

* added scss file for data tier advanced section and other style
updates

* added tests for new warning

* fix types

* added correct copy for cold and frozen phases

* fix types and i18n

* implement copy feedback

* added spacer after the enable data tier allocation switch

* refactor to super select

* fix replicas copy

* update phase serialization for cold and frozen

* Refactor so that logic determining warnings lives together

- also factor out the warning of the node allocation component
- revisit copy for the allocation warning

* tier -> phase

* Added some much needed policy serialization test coverage

- also factored out policy allocation action serialization

* fix import paths and added required file header

* fix existing test coverage

* refine copy for data tier allocation recommended option

* fix showing warning for no node attrs

* fix inverted warning logic 🤦🏼‍♂️

* fix typing

* implement CJs copy feedback

* fix i18n

* remove unused or invalid translations

* provide ability to not alter original policy

* do not alter the original policy in the serilalization process

* fix jest tests

* Remove duplicate type and refactor NodeRole to NodeDataRole

Also deleted unused component "AdvancedSection" for now

* added comment to "false" typing

* revised and refactored copy based on feedback

* address copy feedback

* update kibana schema to allow migrate: { enabled: false }

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
jloleysens added a commit that referenced this pull request Sep 18, 2020
* wip

* Revert "wip"

This reverts commit 54b6f7ff3ec8b0f57b150ab2276d617686da9fb5.

* Revert "Revert "wip""

This reverts commit 63868b44ec60d7431c3a0189b16aeece1db2d38e.

* Refactor to using EUI button group component

- also moved node attr and node allocation component to inside
  new folder that contains all allocation components.
- only focussed on updating warm phase for now

* WIP: moved form UX more in line with EUI

- The described form group now has a switch for showing
  controls on the left.
- Refactored DataTierAllocation to CustomDataTierAllocation
- Removed 'node-roles' option
- Updated copy
- Moved JSX around a bit in the edit policy section to make logic
  simpler

* Refactor UI to reflect custom-ness of "Custom" and "None" options

- Still only implemented for warm, cold and frozen are still
  under way

* server side changes for getting node data

* double opt-in

* Refactored data tier allocation type

- Made types more explicit 'default', 'custom' and 'none'
- Fixed issue introduced by use useCallback on state setter -
  need to use the function setter pattern to not have stale data
  being set.

* Some refacoring, but main point is to add warning detection for
node roles.

- Refactored way we get node data to a provider component so that
  phases still have flexibility in how they render. Currently
  this also means that we fetch node stats data for each phase
  we render
- Created a callout for when there is no node role to which data
  can be allocated for the default setting
- Also updated the behaviour to render the entire form even when
  we cannot fetch node data for some reason. It is not ideal to
  not have node data, but we should not block the entire form.

* fix i18n

* fix type issue with deafult policies missing allocation type

* remove "undefined" as option for setting phase data

* Create referentially stable data setter for all phases - prevent infini-update

* fix type issue

* refactor data -> nodesData

* refactor cold phase for data tiers

* refactor frozen phase for data tiers

* fixed existing tests for warm section

* restored existing test coverage for cold phase and added test coverage for frozen

* fix api integration test

* remove unused translations

* slight UX update to turning on custom attribute allocation

* added scss file for data tier advanced section and other style
updates

* added tests for new warning

* fix types

* added correct copy for cold and frozen phases

* fix types and i18n

* implement copy feedback

* added spacer after the enable data tier allocation switch

* refactor to super select

* fix replicas copy

* update phase serialization for cold and frozen

* Refactor so that logic determining warnings lives together

- also factor out the warning of the node allocation component
- revisit copy for the allocation warning

* tier -> phase

* Added some much needed policy serialization test coverage

- also factored out policy allocation action serialization

* fix import paths and added required file header

* fix existing test coverage

* refine copy for data tier allocation recommended option

* fix showing warning for no node attrs

* fix inverted warning logic 🤦🏼‍♂️

* fix typing

* implement CJs copy feedback

* fix i18n

* remove unused or invalid translations

* provide ability to not alter original policy

* do not alter the original policy in the serilalization process

* fix jest tests

* Remove duplicate type and refactor NodeRole to NodeDataRole

Also deleted unused component "AdvancedSection" for now

* added comment to "false" typing

* revised and refactored copy based on feedback

* address copy feedback

* update kibana schema to allow migrate: { enabled: false }

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
gmmorris added a commit to gmmorris/kibana that referenced this pull request Sep 22, 2020
* master: (92 commits)
  [ILM] Data tiers for 7.10 (elastic#76126)
  [ML] Transforms: Fixes styling of preview grid pagination in summary step (elastic#77789)
  [Drilldowns] Beta badge support. Mark URL Drilldown as Beta (elastic#75654)
  Re-enable session lifespan, idle timeout api integration tests and use unique names for the security test reports. (elastic#77746)
  [Alerting] renames code in alerting RBAC exemption to make it easier to maintain (elastic#77598)
  [Alerting & Actions] Overwrite SOs when updating instead of partially updating (elastic#73688)
  fixed react warning in Suspense in alert flyout (elastic#77777)
  [APM] Track usage of Gold+ features (elastic#77630)
  Visualize: Bad request when working with histogram aggregation (elastic#77684)
  remove legacy ES plugin (elastic#77703)
  [Lens] change name of custom query to filters (elastic#77725)
  skip flaky suite (elastic#76239)
  remove visual aspects of baseline job (elastic#77815)
  skip flaky suite (elastic#77835)
  Fixes typo in data recognizer text (elastic#77691)
  management/update trusted_apps jest snapshot
  [build] Use Elastic hosted UBI minimal base image (elastic#77776)
  [APM] Add transaction error rate alert (elastic#76933)
  [Security Solution] [Detections] Remove file validation on import route (elastic#77770)
  [Enterprise Search][tech debt] Add Kea logic paths for easier debugging/defaults (elastic#77698)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:ILM release_note:enhancement Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more v7.10.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants