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

Journald polling #380

Merged
merged 7 commits into from
Aug 18, 2021
Merged

Journald polling #380

merged 7 commits into from
Aug 18, 2021

Conversation

jsirianni
Copy link
Member

@jsirianni jsirianni commented Aug 16, 2021

Description of Changes

Instead of running journalctl as a long running process, this PR switches to a polling strategy similar to file input's design. On some platforms, journalctl has trouble following rotated files (and fails to close file handles to old files). This situation is very difficult to replicate. The band aid solution is to restart the agent, which is not ideal in a production environment such as OpenShift. By polling the journal, this situation should be avoided.

  • All wait group handling is in poll()
  • Added check for canceled context before reading output from journald in poll()
  • Removed goroutine for sync offsets, instead sync on defer when poll() is finished
  • Added poll duration parameter to operator, defaulting to 200ms (just like file input)
  • Added call to cmd.Wait(), which closes the subprocess after it has finished and all stdout has been read

Please check that the PR fulfills these requirements

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)
  • Add a changelog entry (for non-trivial bug fixes / features)
  • CI passes

@jsirianni jsirianni requested a review from djaglowski August 16, 2021 18:43
@jsirianni jsirianni marked this pull request as ready for review August 16, 2021 18:43
@codecov
Copy link

codecov bot commented Aug 16, 2021

Codecov Report

Merging #380 (9ce72a1) into master (69d0199) will decrease coverage by 0.10%.
The diff coverage is 71.11%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #380      +/-   ##
==========================================
- Coverage   73.28%   73.18%   -0.10%     
==========================================
  Files         124      124              
  Lines        7979     7994      +15     
==========================================
+ Hits         5847     5850       +3     
- Misses       1633     1645      +12     
  Partials      499      499              
Impacted Files Coverage Δ
operator/builtin/input/journald/journald.go 57.94% <71.11%> (+0.28%) ⬆️
operator/builtin/input/tcp/tcp.go 72.66% <0.00%> (-3.91%) ⬇️
operator/builtin/output/forward/forward.go 60.49% <0.00%> (-1.23%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 69d0199...9ce72a1. Read the comment docs.

@djaglowski
Copy link
Member

Log Files Logs / Second CPU Avg (%) CPU Avg Δ (%) Memory Avg (MB) Memory Avg Δ (MB)
1 1000 1.4827456 -0.1552341 126.90248 -2.336609
1 5000 5.2415705 -0.017201424 137.08379 -3.6473541
1 10000 10.965538 +0.6723442 150.46431 +4.6431885
1 50000 50.13796 -6.4859085 172.0163 -4.695038
1 100000 98.75975 -2.249977 225.27061 -14.469421
10 100 1.8965862 -0.36210036 131.91217 -1.532196
10 500 6.689655 +0.65503883 139.40355 +0.22520447
10 1000 11.965454 -0.086299896 145.17929 +0.39117432
10 5000 56.65749 +1.5536804 177.94289 -3.8060303
10 10000 106.67423 -0.63072205 215.33002 -6.930893

Copy link
Member

@djaglowski djaglowski left a comment

Choose a reason for hiding this comment

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

Couple minor things, but looks good.

operator/builtin/input/journald/journald.go Outdated Show resolved Hide resolved
operator/builtin/input/journald/journald.go Show resolved Hide resolved
@djaglowski
Copy link
Member

Log Files Logs / Second CPU Avg (%) CPU Avg Δ (%) Memory Avg (MB) Memory Avg Δ (MB)
1 1000 1.4310206 -0.20695913 128.24811 -0.99098206
1 5000 5.189793 -0.06897879 137.83675 -2.894394
1 10000 10.086128 -0.20706558 143.92403 -1.8970947
1 50000 52.467773 -4.1560936 173.71147 -2.9998627
1 100000 111.10532 +10.095596 235.6712 -4.0688324
10 100 1.9483167 -0.31036985 135.03555 +1.5911865
10 500 7.0691457 +1.0345297 141.87056 +2.692215
10 1000 12.516948 +0.46519375 146.43575 +1.6476288
10 5000 55.68668 +0.5828705 182.39345 +0.64453125
10 10000 119.29865 +11.993698 233.59267 +11.331757

@jsirianni jsirianni merged commit 7537876 into master Aug 18, 2021
@jsirianni jsirianni deleted the journald-polling branch August 18, 2021 02:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants