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

fix: connection close hang #658

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

fix: connection close hang #658

wants to merge 2 commits into from

Commits on Feb 3, 2024

  1. fix: connection close hang

    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
    stevenh committed Feb 3, 2024
    Configuration menu
    Copy the full SHA
    6177dc9 View commit details
    Browse the repository at this point in the history

Commits on Feb 13, 2024

  1. fix: add support for newer state impacting command

    Add support for newer command which impact the state of a client
    connection hence need special handling when returning connections to the
    pool.
    
    Also add missing type assertions for ConnWithContext on connection types.
    stevenh committed Feb 13, 2024
    Configuration menu
    Copy the full SHA
    f5f94cf View commit details
    Browse the repository at this point in the history