-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[ILM] Data tiers for 7.10 #76126
Conversation
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
@elasticmachine merge upstream |
merge conflict between base and head |
@elasticmachine merge upstream |
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.
@elasticmachine merge upstream |
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.
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.
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.
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'; |
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.
Do we need both NodeRole
and DataTierNodeRole
(looks like they contain the same values)?
@@ -0,0 +1,41 @@ | |||
/* |
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.
Doesn't look like this file is being used anywhere?
@@ -100,6 +103,9 @@ export interface AllocateAction { | |||
require?: { | |||
[attribute: string]: string; | |||
}; | |||
migrate?: { | |||
enabled: false; |
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.
Was setting false
here intentional?
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 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 👍
@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:
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:
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:
I think the imperative form for the error messages works better, too:
|
…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
Thanks for the copy review. I have addressed your suggestions in 50a9651.
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.
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? |
@alisonelizabeth Thanks for the review! I believe I addressed all of your feedback in the last series of commits (d695e95, 36e4a45) |
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.
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.', |
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.
Apply the same changes for the cold tier:
'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.', |
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.
Apply same changes to the frozen tier:
'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', |
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.
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.', |
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.
'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.', |
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.
'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.', |
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.
'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.', |
@elasticmachine merge upstream |
@elasticmachine merge upstream |
💚 Build SucceededBuild metrics@kbn/optimizer bundle module count
async chunks size
page load bundle size
distributable file count
History
To update your PR or re-run it, just comment with: |
* 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>
* 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>
* 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) ...
Summary
Adds support for data tiers in ILM.
TODO
Screenshots
Screenshots
Select expanded
Default state (warm tier)
Custom data tier allocation activated
Data tier allocation off (migration { enabled: false } } )
No node roles found warning
No node attrs found warning (pre-existing)
How to test
yarn es snapshot -E node.attr.rack_id=rack_one
(this will enable you to use the custom allocation selectorallocate
action to the phase in questionmigrate: { enabled: false }
to the policyactions
sectionNote: 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