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

Fix exporter benchmark failures #2994

Merged
merged 1 commit into from
Nov 15, 2021
Merged

Fix exporter benchmark failures #2994

merged 1 commit into from
Nov 15, 2021
+44 −11

Conversation

heanlan
Copy link
Contributor

@heanlan heanlan commented Nov 5, 2021 •
edited
Loading

Add a dedicated initialization function for testing purpose - NewFlowExporterForTest.

Fixes: #2984

Signed-off-by: heanlan hanlan@vmware.com

@heanlan
Copy link
Contributor Author

heanlan commented Nov 5, 2021 •
edited
Loading

Tested on local env:

go test -test.v -run=BenchmarkExport -test.benchmem -bench=BenchmarkExportConntrackConns -benchtime=10x -memprofile memprofile.out -cpuprofile profile.out
goos: linux
goarch: amd64
pkg: antrea.io/antrea/pkg/agent/flowexporter/exporter
cpu: Intel(R) Core(TM) i7-1068NG7 CPU @ 2.30GHz
BenchmarkExportConntrackConns
    exporter_perf_test.go:97: 
        Summary:
        Number of conntrack connections: 20000
        Number of dying conntrack connections: 2000
        Total connections received: 19227
    exporter_perf_test.go:97: 
        Summary:
        Number of conntrack connections: 20000
        Number of dying conntrack connections: 2000
        Total connections received: 19369
BenchmarkExportConntrackConns-2   	      10	  25775772 ns/op	 3223556 B/op	   32004 allocs/op
PASS
ok  	antrea.io/antrea/pkg/agent/flowexporter/exporter	1.040s

go test -test.v -run=BenchmarkExport -test.benchmem -bench=BenchmarkExportDenyConns -benchtime=10x -memprofile memprofile.out -cpuprofile profile.out
goos: linux
goarch: amd64
pkg: antrea.io/antrea/pkg/agent/flowexporter/exporter
cpu: Intel(R) Core(TM) i7-1068NG7 CPU @ 2.30GHz
BenchmarkExportDenyConns
    exporter_perf_test.go:145: 
        Summary:
        Number of deny connections: 20000
        Number of idle deny connections: 2000
        Total connections received: 19000
    exporter_perf_test.go:145: 
        Summary:
        Number of deny connections: 20000
        Number of idle deny connections: 2000
        Total connections received: 19219
BenchmarkExportDenyConns-2   	      10	  33160579 ns/op	 3159456 B/op	   33989 allocs/op
PASS
ok  	antrea.io/antrea/pkg/agent/flowexporter/exporter	1.137s

go test -test.v -run=BenchmarkPoll -test.benchmem -benchtime=10x -bench=. -memprofile memprofile.out -cpuprofile profile.out
goos: linux
goarch: amd64
pkg: antrea.io/antrea/pkg/agent/flowexporter/connections
cpu: Intel(R) Core(TM) i7-1068NG7 CPU @ 2.30GHz
BenchmarkPoll
    conntrack_connections_perf_test.go:75: 
        Summary:
        Number of initial connections: 10000
        Number of new connections/poll: 1000
        Number of deleted connections/poll: 1000
    conntrack_connections_perf_test.go:75: 
        Summary:
        Number of initial connections: 10000
        Number of new connections/poll: 1000
        Number of deleted connections/poll: 1000
BenchmarkPoll-2   	      10	  15449828 ns/op	 2345543 B/op	   62117 allocs/op
PASS
ok  	antrea.io/antrea/pkg/agent/flowexporter/connections	0.534s

@heanlan heanlan requested a review from antoninbas November 5, 2021 20:09
@heanlan heanlan marked this pull request as draft November 5, 2021 20:28
@heanlan heanlan marked this pull request as ready for review November 5, 2021 20:34
@codecov-commenter
Copy link

codecov-commenter commented Nov 5, 2021 •
edited
Loading

Codecov Report

Merging #2994 (60b7980) into main (5db792d) will decrease coverage by 19.72%.
The diff coverage is 0.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##             main    #2994       +/-   ##
===========================================
- Coverage   59.84%   40.12%   -19.73%     
===========================================
  Files         289      166      -123     
  Lines       24551    20737     -3814     
===========================================
- Hits        14693     8320     -6373     
- Misses       8250    11605     +3355     
+ Partials     1608      812      -796
Flag Coverage Δ
kind-e2e-tests ?
unit-tests 40.12% <0.00%> (-0.09%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
pkg/agent/flowexporter/exporter/exporter.go 47.77% <0.00%> (-33.70%) ⬇️
pkg/ovs/openflow/default.go 0.00% <0.00%> (-100.00%) ⬇️
pkg/util/runtime/runtime.go 0.00% <0.00%> (-100.00%) ⬇️
pkg/agent/cniserver/pod_configuration_linux.go 0.00% <0.00%> (-100.00%) ⬇️
pkg/ovs/openflow/logs.go 9.52% <0.00%> (-90.48%) ⬇️
pkg/apis/controlplane/register.go 0.00% <0.00%> (-90.00%) ⬇️
pkg/agent/nodeportlocal/k8s/annotations.go 0.00% <0.00%> (-83.88%) ⬇️
pkg/agent/client.go 0.00% <0.00%> (-77.42%) ⬇️
pkg/ovs/ovsconfig/ovs_client_linux.go 0.00% <0.00%> (-76.93%) ⬇️
pkg/flowaggregator/certificate.go 0.00% <0.00%> (-76.58%) ⬇️
... and 235 more

@heanlan heanlan force-pushed the fix-exporter-perf-test branch 2 times, most recently from 60b7980 to 81dc498 Compare November 8, 2021 17:41
antoninbas
antoninbas reviewed Nov 8, 2021
View reviewed changes
pkg/agent/flowexporter/exporter/exporter_perf_test.go Outdated Show resolved Hide resolved
cmd/antrea-agent/agent.go Outdated Show resolved Hide resolved
@heanlan heanlan force-pushed the fix-exporter-perf-test branch from 81dc498 to c34dacb Compare November 8, 2021 19:18
@heanlan heanlan force-pushed the fix-exporter-perf-test branch 3 times, most recently from 2a5cb79 to 57dfeb2 Compare November 8, 2021 20:13
antoninbas
antoninbas previously approved these changes Nov 9, 2021
View reviewed changes
Copy link
Contributor

@antoninbas antoninbas left a comment

Choose a reason for hiding this comment

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

LGTM

@antoninbas
Copy link
Contributor

/test-all

@heanlan
Copy link
Contributor Author

heanlan commented Nov 9, 2021

/test-integration

tnqn
tnqn previously approved these changes Nov 12, 2021
View reviewed changes
Copy link
Member

@tnqn tnqn left a comment

Choose a reason for hiding this comment

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

LGTM, a minor comment

pkg/agent/flowexporter/exporter/exporter_perf_test.go Outdated Show resolved Hide resolved
@heanlan heanlan dismissed stale reviews from tnqn and antoninbas via 8318e9b November 12, 2021 20:32
@heanlan heanlan force-pushed the fix-exporter-perf-test branch from 57dfeb2 to 8318e9b Compare November 12, 2021 20:32
@heanlan
Copy link
Contributor Author

heanlan commented Nov 12, 2021

/test-all

tnqn
tnqn reviewed Nov 15, 2021
View reviewed changes
.golangci.yml Outdated Show resolved Hide resolved
@heanlan
Add a dedicated initialization function of FlowExporter for testing p…
a20d5fc 
…urpose

Signed-off-by: heanlan <hanlan@vmware.com>
@heanlan heanlan force-pushed the fix-exporter-perf-test branch from 8318e9b to a20d5fc Compare November 15, 2021 06:59
@heanlan
Copy link
Contributor Author

heanlan commented Nov 15, 2021

/test-all
/test-integration

@tnqn
Copy link
Member

tnqn commented Nov 15, 2021

/test-integration

@tnqn tnqn merged commit ce63b96 into antrea-io:main Nov 15, 2021
@heanlan heanlan deleted the fix-exporter-perf-test branch November 15, 2021 19:07
@wenqiq
Copy link
Contributor

wenqiq commented Nov 16, 2021

https://github.com/antrea-io/antrea/runs/4213784411?check_suite_focus=true
Go benchmark check compared with HEAD~1 will fail, the next PR will go back to normal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tnqn tnqn tnqn approved these changes

@antoninbas antoninbas antoninbas left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

BenchmarkExportConntrackConns panics
5 participants
@heanlan @codecov-commenter @antoninbas @tnqn @wenqiq