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 fine-grained locking for file audit sink. #7022

Closed
wants to merge 1 commit into from
Closed

Add fine-grained locking for file audit sink. #7022

wants to merge 1 commit into from

Conversation

bradbl
Copy link
Contributor

@bradbl bradbl commented Jun 30, 2019

This replaces the exclusive mutex that protected the logging of
requests and responses. This limited the overall throughput when
file auditng is enabled as only a single goroutine at a time could
be marshalling request/response data.

This creates a new mutex which only protects write access to the
the file. The lock is only taken after the data has marshalled and
is ready for writing to the file.

Resolves #7014

Before change:

$ go test -bench=.
goos: darwin
goarch: amd64
pkg: github.com/hashicorp/vault/builtin/audit/file
BenchmarkAuditFile_request-8   	   30000	     54506 ns/op
PASS
ok  	github.com/hashicorp/vault/builtin/audit/file	2.288s

After change:

$ go test -bench=.
goos: darwin
goarch: amd64
pkg: github.com/hashicorp/vault/builtin/audit/file
BenchmarkAuditFile_request-8   	  100000	     15289 ns/op
PASS
ok  	github.com/hashicorp/vault/builtin/audit/file	1.767s

This replaces the exclusive mutex that protected the logging of
requests and responses. This limited the overall throughput when
file auditng is enabled as only a single goroutine at a time could
be marshalling request/response data.

This creates a new mutex which only protects write access to the
the file. The lock is only taken after the data has marshalled and
is ready for writing to the file.
@hashicorp-cla
Copy link

hashicorp-cla commented Jun 30, 2019

CLA assistant check
All committers have signed the CLA.

jefferai added a commit that referenced this pull request Jun 30, 2019
This was inspired by #7022 but has the advantage of avoiding
double-locking and needing to perform lock upgrades while also
simplifying the logic and being faster.

Original, #7022, this:

goos: linux
goarch: amd64
pkg: github.com/hashicorp/vault/builtin/audit/file
BenchmarkAuditFile_request-4       30000             60734 ns/op
PASS
ok      github.com/hashicorp/vault/builtin/audit/file   2.428s

goos: linux
goarch: amd64
pkg: github.com/hashicorp/vault/builtin/audit/file
BenchmarkAuditFile_request-4       50000             34772 ns/op
PASS
ok      github.com/hashicorp/vault/builtin/audit/file   2.086s

goos: linux
goarch: amd64
pkg: github.com/hashicorp/vault/builtin/audit/file
BenchmarkAuditFile_request-4       50000             25302 ns/op
PASS
ok      github.com/hashicorp/vault/builtin/audit/file   1.542s

Fixes #7014
Closes #7022
@jefferai
Copy link
Member

Closing for now in favor of #7024

@jefferai jefferai closed this Jun 30, 2019
jefferai added a commit that referenced this pull request Jul 1, 2019
This was inspired by #7022 but has the advantage of avoiding
double-locking and needing to perform lock upgrades while also
simplifying the logic and being faster.

Original, #7022, this:

goos: linux
goarch: amd64
pkg: github.com/hashicorp/vault/builtin/audit/file
BenchmarkAuditFile_request-4       30000             60734 ns/op
PASS
ok      github.com/hashicorp/vault/builtin/audit/file   2.428s

goos: linux
goarch: amd64
pkg: github.com/hashicorp/vault/builtin/audit/file
BenchmarkAuditFile_request-4       50000             34772 ns/op
PASS
ok      github.com/hashicorp/vault/builtin/audit/file   2.086s

goos: linux
goarch: amd64
pkg: github.com/hashicorp/vault/builtin/audit/file
BenchmarkAuditFile_request-4       50000             25302 ns/op
PASS
ok      github.com/hashicorp/vault/builtin/audit/file   1.542s

Fixes #7014
Closes #7022
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.

Performance Bottlecheck when Auditing to File
3 participants