Skip to content

ECH: Move nodes off allocator doc updated #1619

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

Open
wants to merge 38 commits into
base: main
Choose a base branch
from

Conversation

eedugon
Copy link
Contributor

@eedugon eedugon commented Jun 5, 2025

As described in #1527, this PR is promoting a knowledge article into our existing doc, per @kunisen and support team request.

Preview:

Changes:

  • Title updated
  • Email notifications section fixed (it wasn't valid).
  • Content of mentioned KB integrated into the doc.

Links to existing KB:

Closes #1527

@eedugon eedugon marked this pull request as ready for review June 5, 2025 10:09
@eedugon eedugon requested review from a team as code owners June 5, 2025 10:09
@kunisen

This comment was marked as outdated.

@eedugon
Copy link
Contributor Author

eedugon commented Jun 5, 2025

@kunisen , about the comment you have shared:

But one thing I just noticed, is maybe we could add a "Frequently Asked Questions (FAQs)" sub heading in the page so that readers can understand we included a bunch of FAQs.

The headings are in "Q&A" format style already, but that's something I wasn't sure if it was the right approach, and I wanted to double check that with other docs folks.

I agree if the headings are kept in this Q&A format, then a "Frequently Asked Questions" heading would make all sense, but maybe we rewrite the headers to be in a different format.

cc: @shainaraskas , what would you say?

Copy link
Contributor

@jakommo jakommo 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 small comments, but other than that LGTM!

@shainaraskas
Copy link
Collaborator

shainaraskas commented Jun 5, 2025

The headings are in "Q&A" format style already, but that's something I wasn't sure if it was the right approach, and I wanted to double check that with other docs folks.

I agree if the headings are kept in this Q&A format, then a "Frequently Asked Questions" heading would make all sense, but maybe we rewrite the headers to be in a different format.

this isn't really in our style (reasoning) and could be reworked

a couple of them should be removed (e.g. the support CTA), or integrated into the doc ("Could such a system maintenance be avoided or skipped?" should just be introductory information about why this happens and its inevitability)

some could be pulled into an "Availability during system maintenance" section and perhaps "Data loss risk for non-HA deployments"

some of them could be reworded ("How can I be notified when a node is changed?" > "Notifications for moved or changed nodes" [more task-based]).

I do think that if we want to keep these together, they do need a heading of their own so they're not nested below "Possible causes and impact"

@eedugon
Copy link
Contributor Author

eedugon commented Jun 5, 2025

@shainaraskas : I'll do some rework on this to avoid the FAQ style while keeping all the key points we want to communicate to the users. Thanks a lot for your feedback!

@kunisen
Copy link
Contributor

kunisen commented Jun 6, 2025

Thanks for being patient and all the help! 🙏

[1]

I made a bunch of updates based on internal ticket comments - https://github.com/elastic/support-tech-lead/issues/1576#issuecomment-2948156720.

Here's the preview:
https://docs-v3-preview.elastic.dev/elastic/docs-content/pull/1619/troubleshoot/monitoring/node-moves-outages

[2]

@eedugon I totally get what you and @shainaraskas said above #1619 (comment). Please feel free to make any updates from docs perspective based on your writing standard.

I still added FAQ heading because if we don't have this, it's logically unbalanced and not ready for being merged.
I know it's not clearing the docs criteria, but in case it takes long or great effort to reorganize the wordings, it's technically and logically ready for merge, which means we could do the merge first, and then think about the wording improvement next.

Again, please feel free to make your change even including the removal of that one.

[3]

Also, I believe it's technically clear now so no longer need to discuss anything further internally. But if still anything is technically unclear or regarding the expectation, let's still discuss it internally ha :)

@kunisen
Copy link
Contributor

kunisen commented Jun 19, 2025

Also @shainaraskas could you help solve this please?

image

https://github.com/elastic/docs-content/actions/runs/15755256766

'connectors-kibana/email-action-type.md' is not a valid link in the 'kibana' cross link index: https://elastic-docs-link-index.s3.us-east-2.amazonaws.com/elastic/kibana/main/links.json

I have no idea about how to refer to a kibana doc in cloud docs...

@eedugon
Copy link
Contributor Author

eedugon commented Jun 19, 2025

Thanks @kunisen ! It feels and reads well to me, let's see if @shainaraskas can do a final review hopefully next week.

I've only updated one minor thing:

  • I've added the error message in a code block, as it's what we are doing in the majority of documents, and visually feels much better.

  • Also the screenshot in this case doesn't add almost any value. If the reader is interested on how the activity page looks like they have the link right there that takes you to the activity page where there's a nice screenshot available. In the KB article I agree the screenshot might have more sense, but in the web and with the context of the Activity page link, I don't feel it adds much value (and we try to minimize the amount of screenshots to maintain).

I hope this is ok for you too! Let us know otherwise, and we can see how to introduce also the screenshot view.

Copy link
Contributor

@alaudazzi alaudazzi left a comment

Choose a reason for hiding this comment

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

I left a few minor editing suggestions, otherwise LGTM.

@kunisen
Copy link
Contributor

kunisen commented Jun 21, 2025

Thanks @alaudazzi !!
Accepted all the suggestions and enabled auto merge.

it seems like we have a problem deploying to preview. Do we know how to fix this?

@kunisen
Copy link
Contributor

kunisen commented Jun 21, 2025

I’m on mobile and can’t fix now but the error seems to be caused by email anchor missing due to the review comment.
could you help me fix it and commit it so that we could merge this please?

Error: email does not exist in troubleshoot/monitoring/node-moves-outages.md.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ECH - Promote KB about allocator moving nodes due to essential system maintenance
6 participants