Skip to content

Watcher history index has too many indexed fields - (#71479) #117701

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
Merged
merged 9 commits into from
Dec 4, 2024
Merged

Watcher history index has too many indexed fields - (#71479) #117701

merged 9 commits into from
Dec 4, 2024

Conversation

lukewhiting
Copy link
Contributor

@lukewhiting lukewhiting commented Nov 28, 2024 •
edited
Loading

This pull request includes updates to the Watcher Index Template Registry and associated JSON templates to exclude the input chain fields from the .watcher-history index from being mapped. This prevents the field expansion matches too many fields, limit: 1024, got: 1406 error when the chain is complex.

Changes to Watcher Index Template Registry:

  • Updated INDEX_TEMPLATE_VERSION to 17 in WatcherIndexTemplateRegistryField to reflect the exclusion of the input chain from indexing and added comment for new version number

Changes to JSON templates:

  • Increased the priority of the watch-history-no-ilm.json template to 2147483647 and added a new dynamic template to disable indexing for result.input.chain. [1] [2]
  • Increased the priority of the watch-history.json template to 2147483648 and added a new dynamic template to disable indexing for result.input.chain. [1] [2]

Fixes #71479

@lukewhiting lukewhiting changed the title (#71479) - Watcher history index has too many indexed fields Watcher history index has too many indexed fields - (#71479) Nov 28, 2024
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-data-management (Team:Data Management)

@elasticsearchmachine elasticsearchmachine added the Team:Data Management Meta label for data/management team label Nov 28, 2024
@elasticsearchmachine
Copy link
Collaborator

Hi @lukewhiting, I've created a changelog YAML for you.

@lukewhiting lukewhiting force-pushed the 71479-watcher-history-too-many-fields branch from 5fa1921 to 10a0403 Compare November 28, 2024 10:55
@elasticsearchmachine
Copy link
Collaborator

Hi @lukewhiting, I've created a changelog YAML for you.

@joegallo joegallo requested a review from dakrone December 3, 2024 17:02
dakrone
dakrone approved these changes Dec 3, 2024
View reviewed changes
Copy link
Member

@dakrone dakrone left a comment

Choose a reason for hiding this comment

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

LGTM, I left two minor comments

x-pack/plugin/core/template-resources/src/main/resources/watch-history.json Outdated
"priority": 2147483647,
"priority": 2147483648,
Copy link
Member

Choose a reason for hiding this comment

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

Same here

Copy link
Contributor

Choose a reason for hiding this comment

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

Also 2147483647 was Integer.MAX_VALUE, right? I'm not sure anything bad happens if we exceed that, but it seems interesting.

Copy link
Contributor Author

@lukewhiting lukewhiting Dec 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

As above, reverted.

@joegallo

Also 2147483647 was Integer.MAX_VALUE, right? I'm not sure anything bad happens if we exceed that, but it seems interesting.

Just at of interest I was playing about with this, the code did seem to happily accept this priority value in a template and correctly store and return it without overflowing or erroring... That said, I would not be supprised if it overflows at some point when doing comparisons to other priorities and silently evaluates this as -2147483646.

@lukewhiting lukewhiting added the auto-backport Automatically create backport pull requests when merged label Dec 4, 2024
@lukewhiting lukewhiting enabled auto-merge (squash) December 4, 2024 09:48
@lukewhiting lukewhiting merged commit c09bdfb into elastic:main Dec 4, 2024
15 of 16 checks passed
lukewhiting added a commit to lukewhiting/elasticsearch that referenced this pull request Dec 4, 2024
@lukewhiting
Watcher history index has too many indexed fields - (elastic#71479) (e…
04b6c48 
…lastic#117701)

* Exclude result.input.chain from watcher history index mappings

* Update docs/changelog/117701.yaml

* Fixup text now fields are disabled higher up the chain

* Revert priority change
@lukewhiting lukewhiting mentioned this pull request Dec 4, 2024
Merged
@elasticsearchmachine
Copy link
Collaborator

💚 Backport successful

Status Branch Result
✅ 8.x

elasticsearchmachine pushed a commit that referenced this pull request Dec 4, 2024
@lukewhiting
Watcher history index has too many indexed fields - (#71479) (#117701) (
13b2961 
#117979)

* Exclude result.input.chain from watcher history index mappings

* Update docs/changelog/117701.yaml

* Fixup text now fields are disabled higher up the chain

* Revert priority change
@lukewhiting lukewhiting deleted the 71479-watcher-history-too-many-fields branch December 4, 2024 14:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joegallo joegallo joegallo left review comments

@dakrone dakrone dakrone approved these changes

Assignees
No one assigned
Labels
auto-backport Automatically create backport pull requests when merged >bug :Data Management/Watcher Team:Data Management Meta label for data/management team v8.18.0 v9.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Watcher history index has too many indexed fields
4 participants
@lukewhiting @elasticsearchmachine @dakrone @joegallo