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.

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

Enabled MADV_DONTNEED in systemd and docker files #22786

Closed
4 of 5 tasks
urso opened this issue Nov 30, 2020 · 8 comments
Closed
4 of 5 tasks

Enabled MADV_DONTNEED in systemd and docker files #22786

urso opened this issue Nov 30, 2020 · 8 comments
Assignees
Labels
Team:Services (Deprecated) Label for the former Integrations-Services team

Comments

@urso
Copy link

urso commented Nov 30, 2020

The go runtime defaults to MADV_FREE by default. In that case pages are still assigned with the Beat, even after the pages have been "returned" to the OS. The kernel will eventually claim these pages, but RSS memory usage is reported as very high (which it is not necessarily). This seems to mess with the OOM killer at times.

As workaround one can tell the runtime to use MADV_DONTNEED via GODEBUG="madvdontneed=1". We should introduce this environment variable to the systemd file, init file, and docker file by default.

What to do

  • Add GODEBUG="madvdontneed=1" to docker images.
  • Add GODEBUG="madvdontneed=1" to systemd unit file.
  • Add GODEBUG="madvdontneed=1" to init file.
  • Add a note about this in known issues for 7.x maintained versions.
  • Backport the changes to 7.10 too.

More information

@botelastic botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Nov 30, 2020
@urso urso added the Team:Services (Deprecated) Label for the former Integrations-Services team label Nov 30, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/integrations-services (Team:Services)

@botelastic botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Nov 30, 2020
@fearful-symmetry
Copy link
Contributor

@urso The systemd unit files are pretty straight forward, and I assume we'll just want to add an ENV to the dockerfile. However, I'm not quite sure how the init shell scripts work. Can we just... export something here?

exec /usr/share/{{.BeatName}}/bin/{{.BeatName}} \
  --path.home /usr/share/{{.BeatName}} \
  --path.config /etc/{{.BeatName}} \
  --path.data /var/lib/{{.BeatName}} \
  --path.logs /var/log/{{.BeatName}} \
  "$@"

@urso
Copy link
Author

urso commented Dec 7, 2020

I assume we'll just want to add an ENV to the dockerfile.

Yes

Can we just... export something here?

This looks like a beat wrapper script for CLI use. Is it beatname.sh.tmpl?

The wrapper is ignored by the init file and by the systemd unit file. Modifying it will only affect users running Beats on command line, or those having custom scripts running the wrapper for whatever reason. Yet we should apply the change to the wraper as well. In the wrapper ensure that we do not overwrite GODEBUG if it is already set.

You find the debian init files in dev-tools/packaging/templates/deb/. The ones for the RPMs in dev-tools/packaging/templates/rpm/.

The init scripts are normal shell scripts that spawn sub-processes. In each script it is just a matter of adding export GODEBUG=${GODEBUG-"madvdontneed=1"} to the script. Maybe introduce a DEFAULT_GODEBUG="..." variable and write export GODEBUG=${GODEBUG-$DEFAULT_GODEBUG}. The - ensures that we do not modify GODEBUG even if it is empty. This allows users to "remove" our flags by running GODEBUG= filebeat.sh ...

@fearful-symmetry
Copy link
Contributor

Linked PR. Not sure how to test this beyond "the env var is in the build"

@fearful-symmetry
Copy link
Contributor

Merged, with backported changes. Closing this now.

@urso
Copy link
Author

urso commented Jan 11, 2021

@fearful-symmetry I reopened the issue. The task "Add a note about this in known issues for 7.x maintained versions." seems to be missing. Please add a "known issue" to our docs. The oldest affected version is 7.2 I think.

@urso
Copy link
Author

urso commented Jan 28, 2021

@fearful-symmetry Did we finish all tasks? Can we close the issue?

@fearful-symmetry
Copy link
Contributor

Yep.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team:Services (Deprecated) Label for the former Integrations-Services team
Projects
None yet
Development

No branches or pull requests

3 participants