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

Add benchmark for filestream input #37317

Merged
merged 3 commits into from
Dec 7, 2023
Merged

Conversation

rdner
Copy link
Member

@rdner rdner commented Dec 6, 2023

Now we can quickly compare performance metrics when we make changes to the filestream implementation without running the whole Filebeat.

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.

How to test this PR locally

Default Filestream Configuration

cd ./filebeat/input/filestream
go test -run=none -bench='BenchmarkFilestream/filestream_default_throughput.*' -benchmem -benchtime=100x

On my machine I've got the following results:

BenchmarkFilestream/filestream_default_throughput-10
100          10703028 ns/op        13576607 B/op     181225 allocs/op

def-cpu-before
def-mem-before

Fingerprint File Identity

cd ./filebeat/input/filestream
go test -run=none -bench='BenchmarkFilestream/filestream_fingerprint_throughput.*' -benchmem -benchtime=100x
BenchmarkFilestream/filestream_fingerprint_throughput-10
100          11664752 ns/op        13746616 B/op     191417 allocs/op

fp-cpu-before
fp-mem-before

Related issues

@rdner rdner added enhancement Team:Elastic-Agent-Data-Plane Label for the Agent Data Plane team backport-7.17 Automated backport to the 7.17 branch with mergify labels Dec 6, 2023
@rdner rdner self-assigned this Dec 6, 2023
@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 Dec 6, 2023
@rdner rdner force-pushed the filestream-benchmark branch from 0c78985 to 0ea5a7c Compare December 6, 2023 17:33
@elasticmachine
Copy link
Collaborator

❕ Build Aborted

Either there was a build timeout or someone aborted the build.

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Duration: 31 min 32 sec

🤖 GitHub comments

Expand to view the GitHub comments

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

  • /test : Re-trigger the build.

  • /package : Generate the packages and run the E2E tests.

  • /beats-tester : Run the installation tests with beats-tester.

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

@rdner rdner force-pushed the filestream-benchmark branch from 0ea5a7c to 65ade18 Compare December 6, 2023 17:56
@elasticmachine
Copy link
Collaborator

❕ Build Aborted

Either there was a build timeout or someone aborted the build.

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Duration: 31 min 46 sec

🤖 GitHub comments

Expand to view the GitHub comments

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

  • /test : Re-trigger the build.

  • /package : Generate the packages and run the E2E tests.

  • /beats-tester : Run the installation tests with beats-tester.

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

@rdner rdner marked this pull request as ready for review December 6, 2023 18:06
@rdner rdner requested a review from a team as a code owner December 6, 2023 18:06
Now we can quickly compare performance metrics when we make changes to the
filestream implementation without running the whole Filebeat.
@rdner rdner force-pushed the filestream-benchmark branch from 65ade18 to db5e526 Compare December 6, 2023 18:11
@rdner rdner enabled auto-merge (squash) December 6, 2023 18:17
@elasticmachine
Copy link
Collaborator

❕ Build Aborted

Either there was a build timeout or someone aborted the build.

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Duration: 33 min 56 sec

🤖 GitHub comments

Expand to view the GitHub comments

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

  • /test : Re-trigger the build.

  • /package : Generate the packages and run the E2E tests.

  • /beats-tester : Run the installation tests with beats-tester.

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

@elasticmachine
Copy link
Collaborator

💚 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: 2023-12-06T18:11:54.687+0000

  • Duration: 133 min 36 sec

Test stats 🧪

Test Results
Failed 0
Passed 8315
Skipped 755
Total 9070

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

Expand to view the GitHub comments

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

  • /test : Re-trigger the build.

  • /package : Generate the packages and run the E2E tests.

  • /beats-tester : Run the installation tests with beats-tester.

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

@elasticmachine
Copy link
Collaborator

❕ Build Aborted

Either there was a build timeout or someone aborted the build.

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Duration: 8 min 10 sec

🤖 GitHub comments

Expand to view the GitHub comments

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

  • /test : Re-trigger the build.

  • /package : Generate the packages and run the E2E tests.

  • /beats-tester : Run the installation tests with beats-tester.

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

@elasticmachine
Copy link
Collaborator

💚 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: 2023-12-06T21:45:05.332+0000

  • Duration: 131 min 51 sec

Test stats 🧪

Test Results
Failed 0
Passed 8315
Skipped 755
Total 9070

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

Expand to view the GitHub comments

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

  • /test : Re-trigger the build.

  • /package : Generate the packages and run the E2E tests.

  • /beats-tester : Run the installation tests with beats-tester.

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

Copy link
Contributor

@leehinman leehinman left a comment

Choose a reason for hiding this comment

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

LGTM

one optional nit/comment

connector, eventsDone := newTestPipeline(expEventCount)
done := make(chan struct{})
go func() {
err := input.Run(context, connector)
Copy link
Contributor

Choose a reason for hiding this comment

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

nit. Right now the benchmark is capturing the time spent with the config, setting up manager etc. Might be worth using StopTimer and StartTimer to limit the reporting the time spent in input.Run. I'm sure the setup time is roughly constant so not a huge deal, but it might make it easier to see small changes

@rdner rdner merged commit f2cf95c into elastic:main Dec 7, 2023
29 checks passed
mergify bot pushed a commit that referenced this pull request Dec 7, 2023
Now we can quickly compare performance metrics when we make changes to the
filestream implementation without running the whole Filebeat.

(cherry picked from commit f2cf95c)
@rdner rdner added the backport-skip Skip notification from the automated backport with mergify label Dec 7, 2023
@rdner rdner removed the backport-7.17 Automated backport to the 7.17 branch with mergify label Dec 7, 2023
@rdner rdner deleted the filestream-benchmark branch December 7, 2023 09:40
@alexsapran
Copy link
Contributor

awesome work, this will make a big impact especially when we start running and reporting the status of those benchmarks per PR basis.

Scholar-Li pushed a commit to Scholar-Li/beats that referenced this pull request Feb 5, 2024
Now we can quickly compare performance metrics when we make changes to the
filestream implementation without running the whole Filebeat.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-skip Skip notification from the automated backport with mergify enhancement Team:Elastic-Agent-Data-Plane Label for the Agent Data Plane team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants