Skip to content

Commit

Permalink
fix: connection close hang
Browse files Browse the repository at this point in the history
Fix connection Close hanging if CLIENT REPLY is set to OFF or SKIP.

This includes a rework of how command actions are handled to use a
single map lookup based on generated permutations to improve performance
and allow it to be used in both activeConn and conn. Replication of this
lookup is need as its impossible to share information between the
different Conn interface implementations due to returning interface
instead of concrete type.

The performance improvements, 33-89% sec/op and eliminating all memory
allocations, help to mitigate the impact of double lookup.

This also expands the benchmark coverage to allow for wider validation
of this change.

Fixes #361

go test -run=^$ -bench=BenchmarkLookupCommand -count=10 -benchmem > orig.log
benchstat orig.log permute.log
goos: linux
goarch: amd64
pkg: github.com/gomodule/redigo/redis
cpu: 11th Gen Intel(R) Core(TM) i9-11900H @ 2.50GHz
                                │   orig.log   │             permute.log             │
                                │    sec/op    │   sec/op     vs base                │
LookupCommandInfoCorrectCase-16    55.89n ± 7%   37.24n ± 2%  -33.37% (p=0.000 n=10)
LookupCommandInfoMixedCase-16     359.85n ± 9%   38.79n ± 2%  -89.22% (p=0.000 n=10)
LookupCommandInfoNoMatch-16       262.45n ± 1%   36.98n ± 4%  -85.91% (p=0.000 n=10)
geomean                            174.1n        37.66n       -78.37%

                                │   orig.log   │               permute.log               │
                                │     B/op     │    B/op     vs base                     │
LookupCommandInfoCorrectCase-16   0.000 ± 0%     0.000 ± 0%         ~ (p=1.000 n=10) ¹
LookupCommandInfoMixedCase-16     32.00 ± 0%      0.00 ± 0%  -100.00% (p=0.000 n=10)
LookupCommandInfoNoMatch-16       8.000 ± 0%     0.000 ± 0%  -100.00% (p=0.000 n=10)
geomean                                      ²               ?                       ² ³
¹ all samples are equal
² summaries must be >0 to compute geomean
³ ratios must be >0 to compute geomean

                                │   orig.log   │               permute.log               │
                                │  allocs/op   │ allocs/op   vs base                     │
LookupCommandInfoCorrectCase-16   0.000 ± 0%     0.000 ± 0%         ~ (p=1.000 n=10) ¹
LookupCommandInfoMixedCase-16     4.000 ± 0%     0.000 ± 0%  -100.00% (p=0.000 n=10)
LookupCommandInfoNoMatch-16       2.000 ± 0%     0.000 ± 0%  -100.00% (p=0.000 n=10)
geomean                                      ²               ?                       ² ³
¹ all samples are equal
² summaries must be >0 to compute geomean
³ ratios must be >0 to compute geomean
  • Loading branch information
stevenh committed Feb 2, 2024
1 parent 9129745 commit ad4982a
Show file tree
Hide file tree
Showing 12 changed files with 2,955 additions and 183 deletions.
3 changes: 2 additions & 1 deletion .github/workflows/golangci-lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,10 @@ jobs:
go-version: '1.21'
cache: false # Handled by golangci-lint.

- name: Validate go mod
- name: Validate go mod / generate
run: |
go mod tidy
go generate ./...
git --no-pager diff && [[ 0 -eq $(git status --porcelain | wc -l) ]]
- name: golangci-lint
Expand Down
Loading

0 comments on commit ad4982a

Please sign in to comment.