-
Notifications
You must be signed in to change notification settings - Fork 195
Distinguish+expand Hot Spotting from Task Backlog #4657
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
base: main
Are you sure you want to change the base?
Conversation
Vale Linting ResultsSummary: 2 warnings, 6 suggestions found
|
| File | Line | Rule | Message |
|---|---|---|---|
| troubleshoot/elasticsearch/hotspotting.md | 30 | Elastic.Latinisms | Latin terms and abbreviations are a common source of confusion. Use 'using' instead of 'via'. |
| troubleshoot/elasticsearch/task-queue-backlog.md | 128 | Elastic.BritishSpellings | Use American English spelling 'behavior' instead of British English 'behaviour'. |
💡 Suggestions (6)
| File | Line | Rule | Message |
|---|---|---|---|
| troubleshoot/elasticsearch/hotspotting.md | 30 | Elastic.WordChoice | Consider using 'can, might' instead of 'may', unless the term is in the UI. |
| troubleshoot/elasticsearch/hotspotting.md | 185 | Elastic.FutureTense | 'will most' might be in future tense. Write in the present tense to describe the state of the product as it is now. |
| troubleshoot/elasticsearch/hotspotting.md | 185 | Elastic.FutureTense | 'will surface' might be in future tense. Write in the present tense to describe the state of the product as it is now. |
| troubleshoot/elasticsearch/task-queue-backlog.md | 15 | Elastic.Wordiness | Consider using 'many' instead of 'a large number of'. |
| troubleshoot/elasticsearch/task-queue-backlog.md | 97 | Elastic.FutureTense | 'will contain' might be in future tense. Write in the present tense to describe the state of the product as it is now. |
| troubleshoot/elasticsearch/task-queue-backlog.md | 130 | Elastic.Wordiness | Consider using 'sometimes' instead of 'In some cases'. |
The Vale linter checks documentation changes against the Elastic Docs style guide.
To use Vale locally or report issues, refer to Elastic style guide for Vale.
🔍 Preview links for changed docs |
| * [Check the thread pool status](#diagnose-task-queue-thread-pool) | ||
| * [Inspect hot threads on each node](#diagnose-task-queue-hot-thread) | ||
| * [Check thread pool status](#diagnose-task-queue-thread-pool) | ||
| * [Inspect node hot threads](#diagnose-task-queue-hot-thread) |
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 actually prefer the original here, but maybe they're not technically accurate.
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.
That's fair. I was trying to get the TOC to stop line-overflowing & then afterwards was trying to surface that you care if any threads are caught regardless of if individual node related. But TBF I don't feel overly strongly, I probably just got caught away with edits.
|
Hi @stefnestor, thanks for another really nice add to our docs! I've added a bunch of comments, but overall LGTM! 🎸 |
Co-authored-by: David Kilfoyle <[email protected]>
kilfoyle
left a comment
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.
LGTM! 🏎️
|
Hello team, My suggestions are below. Apologies for adding them as a comment instead of editing the page directly—since there is already a proposed revised version in progress, I didn’t want to introduce conflicting changes. Please consider these suggestions as additions on top of the proposed updates referenced above. 1) Reorder sections for a more logical flow Since this document focuses on task queue backlog, I suggest reordering the sections so the two that rely on the Task Management API are grouped together. Proposed order:
2) Clarify the interpretation of The current text says:
I noticed there were some updates on the proposed change, and the wording has improved since it was somewhat vague. 3) Replace the “For example…” sentence under I suggest removing:
…and replacing it with the following, which is clearer and provides a concrete example:
4) Broaden the phrasing under “Recommendations” to avoid implying only two remediation options Current sentence:
This reads as if there are only two levers (“add resources” or “cancel”), while now there are more (prior to the modifications, there were only those 2). Suggested replacement:
|
|
@rodrigomadalozzo your suggestions look really good to me. If you don't mind adding them into the PR I'll be happy to re-review / approve. Thanks! |
Summary
Follow-up to #4592, when I originally wrote hot spotting (elastic/elasticsearch#95429), we didn't have task queue backlog so some of its kind of content ended up over there.
Undoes that and adds in content related to
Generative AI disclosure