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

Resolve journald argument re-use #389

Merged
merged 2 commits into from
Aug 19, 2021
Merged

Resolve journald argument re-use #389

merged 2 commits into from
Aug 19, 2021

Conversation

jsirianni
Copy link
Member

@jsirianni jsirianni commented Aug 18, 2021

Description of Changes

Now that journald polls for logs (instead of a single long running process), the way arguments are handled needs to be modified. Right now, every poll cycle will append --after-cursor to the args []string value. These parameters are preserved, resulting in many --after-cursor arguments being passed to journald.

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

@djaglowski
Copy link
Member

Log Files Logs / Second CPU Avg (%) CPU Avg Δ (%) Memory Avg (MB) Memory Avg Δ (MB)
1 1000 1.5689743 +0.08622837 129.73761 -0.45742798
1 5000 5.2414584 +0.034559727 137.77344 -0.5580597
1 10000 11.241461 +0.86194515 145.16986 -0.25726318
1 50000 51.000954 +1.6067657 175.78569 +2.4690094
1 100000 108.9007 +8.437637 248.86772 +8.924301
10 100 1.9482888 -0.017226577 133.16595 -0.15840149
10 500 5.982695 +0.051532745 139.9988 +1.3216705
10 1000 11.638024 +0.22399235 144.53233 -2.370407
10 5000 55.61294 +2.4057465 177.94518 -2.0328674
10 10000 114.47258 +4.2478104 238.57584 +10.594696

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

codecov bot commented Aug 18, 2021

Codecov Report

Merging #389 (6e54576) into master (74fc7a1) will decrease coverage by 0.02%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #389      +/-   ##
==========================================
- Coverage   73.25%   73.23%   -0.02%     
==========================================
  Files         124      124              
  Lines        7994     7995       +1     
==========================================
- Hits         5856     5855       -1     
- Misses       1640     1646       +6     
+ Partials      498      494       -4     
Impacted Files Coverage Δ
operator/builtin/input/journald/journald.go 57.48% <0.00%> (-0.46%) ⬇️
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 74fc7a1...6e54576. Read the comment docs.

djaglowski
djaglowski previously approved these changes Aug 18, 2021
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.

A couple very minor optional points. Looks good though.

operator/builtin/input/journald/journald.go Outdated Show resolved Hide resolved
operator/builtin/input/journald/journald.go Outdated 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.5000101 +0.017264247 130.47467 +0.27963257
1 5000 5.6209 +0.41400146 135.13133 -3.2001648
1 10000 10.32766 -0.05185604 144.79553 -0.6315918
1 50000 50.725185 +1.3309975 178.05402 +4.737335
1 100000 97.62879 -2.8342743 229.12944 -10.81398
10 100 1.9828467 +0.017331362 132.23195 -1.0924072
10 500 5.9828043 +0.05164194 137.92268 -0.7544403
10 1000 11.98264 +0.5686083 147.7733 +0.8705597
10 5000 53.638985 +0.4317894 179.94194 -0.036102295
10 10000 110.33655 +0.11177826 236.23155 +8.250412

@jsirianni jsirianni merged commit 80c8406 into master Aug 19, 2021
@jsirianni jsirianni deleted the journald-args branch August 19, 2021 01:49
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