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

[Elastic Agent] Fix lazy acker to only add new actions to the batch #27981

Merged
merged 3 commits into from
Sep 22, 2021
Merged

[Elastic Agent] Fix lazy acker to only add new actions to the batch #27981

merged 3 commits into from
Sep 22, 2021

Conversation

blakerouse
Copy link
Contributor

@blakerouse blakerouse commented Sep 16, 2021 •
edited
Loading

What does this PR do?

This modifies the lazy.Acker to only add a new action to the batch (aka. queue). This ensures that if the same action is added to the batch it is not duplicated.

Why is it important?

Because of the way the Fleet mode of the Elastic Agent is designed it will keep adding the same actions over and over again to the lazy.Acker because the underlying Acker fails to ack the action.

This causes the queue to grow indefinitely that causes the process to consume all the memory of the system.

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • [ ] I have made corresponding changes to the documentation
  • [ ] I have made corresponding change to the default configuration files
  • I have added tests that prove my fix is effective or that my feature works
  • I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

Related issues

@blakerouse blakerouse added Team:Elastic-Agent Label for the Agent team backport-v7.14.0 Automated backport with mergify backport-v7.15.0 Automated backport with mergify backport-v7.16.0 Automated backport with mergify labels Sep 16, 2021
@blakerouse blakerouse self-assigned this Sep 16, 2021
@botelastic botelastic bot added needs_team Indicates that the issue/PR needs a Team:* label and removed needs_team Indicates that the issue/PR needs a Team:* label labels Sep 16, 2021
@blakerouse blakerouse marked this pull request as ready for review September 16, 2021 18:42
@elasticmachine
Copy link
Collaborator

Pinging @elastic/agent (Team:Agent)

@elasticmachine
Copy link
Collaborator

elasticmachine commented Sep 16, 2021 •
edited by jenkins-beats-ci bot
Loading

💚 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 preview

Expand to view the summary

Build stats

  • Start Time: 2021-09-20T14:36:22.633+0000

  • Duration: 78 min 25 sec

  • Commit: 8818d7f

Test stats 🧪

Test Results
Failed 0
Passed 7108
Skipped 16
Total 7124

Trends 🧪

Image of Build Times

Image of Tests

💚 Flaky test report

Tests succeeded.

Expand to view the summary

Test stats 🧪

Test Results
Failed 0
Passed 7108
Skipped 16
Total 7124

@mergify
Copy link
Contributor

mergify bot commented Sep 20, 2021

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b agent-fix-lazy-ack upstream/agent-fix-lazy-ack
git merge upstream/master
git push upstream agent-fix-lazy-ack

michalpristas
michalpristas approved these changes Sep 20, 2021
View reviewed changes
Copy link
Contributor

@michalpristas michalpristas left a comment

Choose a reason for hiding this comment

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

change lgtm

@blakerouse
Copy link
Contributor Author

/package

@blakerouse blakerouse merged commit bdec63a into elastic:master Sep 22, 2021
@blakerouse blakerouse deleted the agent-fix-lazy-ack branch September 22, 2021 14:07
mergify bot pushed a commit that referenced this pull request Sep 22, 2021
@blakerouse
[Elastic Agent] Fix lazy acker to only add new actions to the batch (#…
86c4ac0 
…27981)

* Fix lazy acker.

* Add changelog.

(cherry picked from commit bdec63a)
@mergify mergify bot mentioned this pull request Sep 22, 2021
Merged
mergify bot pushed a commit that referenced this pull request Sep 22, 2021
@blakerouse
[Elastic Agent] Fix lazy acker to only add new actions to the batch (#…
aa3afdc 
…27981)

* Fix lazy acker.

* Add changelog.

(cherry picked from commit bdec63a)
@mergify mergify bot mentioned this pull request Sep 22, 2021
Merged
mergify bot pushed a commit that referenced this pull request Sep 22, 2021
@blakerouse
[Elastic Agent] Fix lazy acker to only add new actions to the batch (#…
c70dc8f 
…27981)

* Fix lazy acker.

* Add changelog.

(cherry picked from commit bdec63a)
@mergify mergify bot mentioned this pull request Sep 22, 2021
Merged
blakerouse added a commit that referenced this pull request Sep 22, 2021
@mergify @blakerouse
[Elastic Agent] Fix lazy acker to only add new actions to the batch (#…
5134380 
…27981) (#28060)

* Fix lazy acker.

* Add changelog.

(cherry picked from commit bdec63a)

Co-authored-by: Blake Rouse <[email protected]>
blakerouse added a commit that referenced this pull request Sep 22, 2021
@mergify @blakerouse
[Elastic Agent] Fix lazy acker to only add new actions to the batch (#…
dbaaa41 
…27981) (#28061)

* Fix lazy acker.

* Add changelog.

(cherry picked from commit bdec63a)

Co-authored-by: Blake Rouse <[email protected]>
blakerouse added a commit that referenced this pull request Sep 22, 2021
@mergify @blakerouse
[Elastic Agent] Fix lazy acker to only add new actions to the batch (#…
9e8d87b 
…27981) (#28062)

* Fix lazy acker.

* Add changelog.

(cherry picked from commit bdec63a)

Co-authored-by: Blake Rouse <[email protected]>
Icedroid pushed a commit to Icedroid/beats that referenced this pull request Nov 1, 2021
@blakerouse
[Elastic Agent] Fix lazy acker to only add new actions to the batch (e…
1cc7d92 
…lastic#27981)

* Fix lazy acker.

* Add changelog.
leweafan pushed a commit to leweafan/beats that referenced this pull request Apr 28, 2023
@mergify @blakerouse
[Elastic Agent] Fix lazy acker to only add new actions to the batch (e…
5901e36 
…lastic#27981) (elastic#28061)

* Fix lazy acker.

* Add changelog.

(cherry picked from commit 1fcb57e)

Co-authored-by: Blake Rouse <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@michalpristas michalpristas michalpristas approved these changes

Assignees

@blakerouse blakerouse

Labels
backport-v7.14.0 Automated backport with mergify backport-v7.15.0 Automated backport with mergify backport-v7.16.0 Automated backport with mergify Team:Elastic-Agent Label for the Agent team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Elastic Agent] Lazy Acker causes actions queue to grow indefinitely when underlying acker fails
3 participants
@blakerouse @elasticmachine @michalpristas