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

[DOCS] Adds important admonition to handling delayed data page #97753

Merged
merged 8 commits into from
Jul 19, 2023
Merged

[DOCS] Adds important admonition to handling delayed data page #97753

merged 8 commits into from
Jul 19, 2023

Conversation

szabosteve
Copy link
Contributor

@szabosteve szabosteve commented Jul 18, 2023 •
edited
Loading

Overview

This PR:

  • adds an IMPORTANT admonition to the Handling delayed data page that hints that missed documents could be the results of an error with the ingestion process,
  • adjusts the ES error message to include that the source of error could be the ingestion process.

@github-actions
Copy link
Contributor

Documentation preview:

@szabosteve szabosteve added >docs General docs changes :ml Machine learning Team:Docs Meta label for docs team auto-backport Automatically create backport pull requests when merged v8.9.0 v8.8.3 labels Jul 18, 2023
@szabosteve szabosteve marked this pull request as ready for review July 18, 2023 14:12
@elasticsearchmachine elasticsearchmachine added the Team:ML Meta label for the ML team label Jul 18, 2023
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/ml-core (Team:ML)

@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-docs (Team:Docs)

benwtrent
benwtrent reviewed Jul 18, 2023
View reviewed changes
x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/messages/Messages.java Outdated
+ " Consider increasing query_delay";
+ " Check if your ingest pipeline delays ingesting. If the pipeline works as expected, consider increasing query_delay.";
Copy link
Member

Choose a reason for hiding this comment

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

I have no strong feelings about this. But IDK how common it is for folks to have ingest pipelines.

Would a user be confused if they have no ingest pipelines?

If y'all think this is fine, I am cool with it. @droberts195

Copy link
Contributor

Choose a reason for hiding this comment

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

I think there's been a misunderstanding at some point. "Ingest pipeline" here really means "end to end ingestion process". So that includes everything from:

  • Originating application creating message
  • Data collector reading the message
  • Data collector processing
  • Data collector sending to Elasticsearch
  • Elasticsearch processing
  • Elasticsearch indexing
  • Elasticsearch making searchable

Using dictionary definitions of words that could be described as an "ingest pipeline", but we shouldn't use the term here because it has a very specific meaning in Elasticsearch.

So basically "ingest pipeline" should be replaced with something else throughout this PR. Maybe "end to end ingestion process", "end to end ingestion flow", "ingestion flow from source to index", or something better.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the input, I rephrased the text via af50d7f.

@szabosteve szabosteve requested a review from droberts195 July 18, 2023 14:49
droberts195
droberts195 approved these changes Jul 18, 2023
View reviewed changes
Copy link
Contributor

@droberts195 droberts195 left a comment

Choose a reason for hiding this comment

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

LGTM

valeriy42
valeriy42 reviewed Jul 19, 2023
View reviewed changes
Copy link
Contributor

@valeriy42 valeriy42 left a comment

Choose a reason for hiding this comment

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

Thank you for looking at this. I added some details to the texts while keeping the size similar.

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/job/messages/Messages.java Outdated Show resolved Hide resolved
docs/reference/ml/anomaly-detection/ml-delayed-data-detection.asciidoc Outdated Show resolved Hide resolved
@szabosteve szabosteve requested a review from valeriy42 July 19, 2023 08:54
valeriy42
valeriy42 approved these changes Jul 19, 2023
View reviewed changes
Copy link
Contributor

@valeriy42 valeriy42 left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you 🙏

@szabosteve szabosteve merged commit 3fd9467 into elastic:main Jul 19, 2023
@szabosteve szabosteve deleted the expand-delayed-data branch July 19, 2023 11:32
@szabosteve szabosteve mentioned this pull request Jul 19, 2023
Merged
szabosteve added a commit to szabosteve/elasticsearch that referenced this pull request Jul 19, 2023
@szabosteve
[DOCS] Adds important admonition to handling delayed data page (elast…
363482a 
…ic#97753)
@szabosteve szabosteve mentioned this pull request Jul 19, 2023
Merged
@elasticsearchmachine
Copy link
Collaborator

💚 Backport successful

Status Branch Result
✅ 8.9
✅ 8.8

szabosteve added a commit to szabosteve/elasticsearch that referenced this pull request Jul 19, 2023
@szabosteve
[DOCS] Adds important admonition to handling delayed data page (elast…
cf2bc00 
…ic#97753)
szabosteve added a commit that referenced this pull request Jul 19, 2023
@szabosteve
[DOCS] Adds important admonition to handling delayed data page (#97753)…
c7bdd6f 
… (#97806)
szabosteve added a commit that referenced this pull request Jul 19, 2023
@szabosteve
[DOCS] Adds important admonition to handling delayed data page (#97753)…
9e281d6 
… (#97805)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@benwtrent benwtrent benwtrent left review comments

@droberts195 droberts195 droberts195 approved these changes

@valeriy42 valeriy42 valeriy42 approved these changes

Assignees
No one assigned
Labels
auto-backport Automatically create backport pull requests when merged >docs General docs changes :ml Machine learning Team:Docs Meta label for docs team Team:ML Meta label for the ML team v8.8.3 v8.9.0 v8.10.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@szabosteve @elasticsearchmachine @valeriy42 @benwtrent @droberts195