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

Ulimit debian #849

Closed
wants to merge 5 commits into from
Closed

Ulimit debian #849

wants to merge 5 commits into from

Conversation

ph
Copy link
Contributor

@ph ph commented Aug 3, 2022

Port of elastic/beats#32580

What does this PR do?

After a considerable amount of googling and strace yesterday, I learned that there's a bug in certain older versions of apt-get in debian, where it'll try to iterate through all possible FDs as set by internal linux system limits. This results in some amusing strace output:

fcntl(337334262, F_SETFD, FD_CLOEXEC)    = -1 EBADF (Bad file descriptor)

This only happens on certain docker installs where the docker daemon won't set the NOFILE ulimit in its containers.

This fix just adds a ulimit flag to the docker invocation when needed.

Why is it important?

On systems that hit this bug, mage package on filebeat can take up to an hour, purely due to the apt-get update operation.

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 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.

ph added 4 commits March 22, 2022 16:57
@ph
Add templates for handling issues and pull request
0526076 
We were missing the new issues and the new pr templates.
@ph
Merge remote-tracking branch 'origin/main'
d513cfc
@ph
Merge remote-tracking branch 'upstream/main'
fd1a95a
@ph
add ulimit to debian container
4fc4a82 
Porting patch from @fearful-symmetry elastic/beats#32580

> There's a bug on certain debian versions:
> https://discuss.linuxcontainers.org/t/debian-jessie-containers-have-extremely-low-performance/1272
> basically, apt-get has a bug where will try to iterate through every possible FD as set by the NOFILE ulimit.
> On certain docker installs, docker will set the ulimit to a value > 10^9, which means apt-get will take >1 hour.
> This runs across all possible debian platforms, since there's no real harm in it.
@ph ph added backport-v8.3.0 Automated backport with mergify backport-v8.4.0 Automated backport with mergify backport-v8.5.0 Automated backport with mergify labels Aug 3, 2022
@ph ph requested a review from a team as a code owner August 3, 2022 13:46
@ph ph self-assigned this Aug 3, 2022
@ph ph requested review from aleksmaus and michalpristas and removed request for a team August 3, 2022 13:46
@ph ph added the ci:end-to-end Enable the e2e-testing label Aug 3, 2022
@ph ph mentioned this pull request Aug 3, 2022
Merged
5 tasks
@ph ph requested a review from AndersonQ August 3, 2022 13:47
@ph
Copy link
Contributor Author

ph commented Aug 3, 2022

@AndersonQ Can you check this one?

@cmacknz
Copy link
Member

cmacknz commented Aug 3, 2022

I think you'll also want this follow up fix included: https://github.com/elastic/beats/pull/32582/files

@ph
Copy link
Contributor Author

ph commented Aug 3, 2022

@cmacknz thanks, I was still waiting for the local build :(

@elasticmachine
Copy link
Contributor

elasticmachine commented Aug 3, 2022 •
edited
Loading

💔 Build Failed

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: 2022-08-03T14:14:38.218+0000

  • Duration: 98 min 58 sec

Test stats 🧪

Test Results
Failed 0
Passed 6134
Skipped 23
Total 6157

Steps errors 1

Expand to view the steps failures

Building e2e-tests » End-2-End Tests Pipeline helper » main
  • Took 63 min 46 sec . View more details here
  • Description: null

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

  • /package : Generate the packages.

  • run integration tests : Run the Elastic Agent Integration tests.

  • run end-to-end tests : Generate the packages and run the E2E Tests.

  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

@elasticmachine
Copy link
Contributor

elasticmachine commented Aug 3, 2022 •
edited
Loading

🌐 Coverage report

Name Metrics % (covered/total) Diff
Packages 97.26% (71/73) 👍
Files 67.857% (152/224) 👍
Classes 68.161% (304/446) 👍
Methods 52.057% (835/1604) 👍
Lines 38.466% (9098/23652) 👍 0.025
Conditionals 100.0% (0/0) 💚

@ph ph mentioned this pull request Aug 9, 2022
Merged
6 tasks
@ph
Copy link
Contributor Author

ph commented Aug 9, 2022

fixed in #856

@ph ph closed this Aug 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AndersonQ AndersonQ AndersonQ approved these changes

@aleksmaus aleksmaus Awaiting requested review from aleksmaus aleksmaus is a code owner automatically assigned from elastic/elastic-agent-control-plane

@michalpristas michalpristas Awaiting requested review from michalpristas michalpristas is a code owner automatically assigned from elastic/elastic-agent-control-plane

Assignees

@ph ph

Labels
backport-v8.3.0 Automated backport with mergify backport-v8.4.0 Automated backport with mergify backport-v8.5.0 Automated backport with mergify ci:end-to-end Enable the e2e-testing
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ph @cmacknz @elasticmachine @AndersonQ