Skip to content

Add warnings to setup.template.overwrite#4463

Closed
Leaf-Lin wants to merge 1 commit intomasterfrom
Leaf-Lin-patch-1
Closed

Add warnings to setup.template.overwrite#4463
Leaf-Lin wants to merge 1 commit intomasterfrom
Leaf-Lin-patch-1

Conversation

@Leaf-Lin
Copy link
Copy Markdown

Motivation/summary

Similar to elastic/beats#22357, setup.template.overwrite could potentially overload Elasticsearch with too many update requests.
On Elasticsearch side, it will be addressed in newer versions by introducing no-op updates which will be available from 7.11+ elastic/elasticsearch#64493

See also elastic/elasticsearch#57662

Thought having some warnings in the APM setting here should prevent users unwittingly leave template auto-creation on across large number of APM.

Checklist

I have considered changes for:

Similar to elastic/beats#22357, setup.template.overwrite could potentially overload Elasticsearch with too many update requests. 
On Elasticsearch side, it will be addressed in newer versions by introducing no-op updates which will be available from 7.11+ elastic/elasticsearch#64493

See also elastic/elasticsearch#57662

Thought having some warnings in the APM setting here should prevent users unwittingly leave template auto-creation on across large number of APM.
@ghost
Copy link
Copy Markdown

ghost commented Nov 27, 2020

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: Pull request #4463 opened

  • Start Time: 2020-11-27T04:15:51.170+0000

  • Duration: 41 min 32 sec

Test stats 🧪

Test Results
Failed 0
Passed 4754
Skipped 143
Total 4897

Steps errors 3

Expand to view the steps failures

Compress

  • Took 0 min 0 sec . View more details on here
  • Description: tar --exclude=coverage-files.tgz -czf coverage-files.tgz coverage

Compress

  • Took 0 min 0 sec . View more details on here
  • Description: tar --exclude=system-tests-linux-files.tgz -czf system-tests-linux-files.tgz system-tests

Test Sync

  • Took 3 min 8 sec . View more details on here
  • Description: ./.ci/scripts/sync.sh

@axw
Copy link
Copy Markdown
Member

axw commented Nov 30, 2020

Thanks @Leaf-Lin! I imagine that because APM Server is (currently) mostly deployed in smaller numbers, it wouldn't be such an issue. Once we move to running under Elastic Agent, on the edge, that would change; but we'll also be moving template management to Fleet then. Anyway, doesn't hurt to be cautious here.

@bmorelli25 I'm not sure how/when we normally update these copied-from-beats docs, so I'll leave it to you.

@axw axw requested a review from bmorelli25 November 30, 2020 01:10
@axw axw added the docs label Nov 30, 2020
@bmorelli25
Copy link
Copy Markdown
Member

Thanks, @Leaf-Lin!

This documentation is shared by both Beats and APM Server, with Beats being the source of truth. It looks like Naomi added this note to the Beats documentation recently, but we haven't updated the file on the APM side. Because APM Server programmatically updates documentation from Beats, committing this PR would only be temporary -- the contents would be overwritten on our next update.

I've opened elastic/beats#22804 to address a few typos in the original PR. Once that is merged, I'll pull the changes into APM Server. In the meantime, I'll close this PR. Thanks again :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants