-
Notifications
You must be signed in to change notification settings - Fork 2.5k
fix(conn): conn to have state machine #3559
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
Merged
+3,221
−569
Merged
Changes from 8 commits
Commits
Show all changes
78 commits
Select commit
Hold shift + click to select a range
27591cd
wip
ndyakov 606264e
wip, used and unusable states
ndyakov 0a75466
Merge remote-tracking branch 'origin/master' into ndyakov/state-machi…
ndyakov 5721512
polish state machine
ndyakov 663a60e
correct handling OnPut
ndyakov 7526e67
better errors for tests, hook should work now
ndyakov 92433e6
fix linter
ndyakov 21bd243
improve reauth state management. fix tests
ndyakov 3f29463
Update internal/pool/conn.go
ndyakov de2f8ba
Update internal/pool/conn.go
ndyakov 94fa920
better timeouts
ndyakov cfcf37d
empty endpoint handoff case
ndyakov 3a53e1b
fix handoff state when queued for handoff
ndyakov c4ed467
try to detect the deadlock
ndyakov 9ad6288
try to detect the deadlock x2
ndyakov 03b0003
delete should be called
ndyakov 84e856e
improve tests
ndyakov a2c7a25
fix mark on uninitialized connection
ndyakov 23d0e0f
Merge branch 'master' into ndyakov/state-machine-conn
ndyakov ffbe1e5
Update internal/pool/conn_state_test.go
ndyakov 65a6ece
Update internal/pool/conn_state_test.go
ndyakov 0964dcc
Update internal/pool/pool.go
ndyakov bc42307
Update internal/pool/conn_state.go
ndyakov 33696fb
Update internal/pool/conn.go
ndyakov 13a4b3f
fix error from copilot
ndyakov 07e665f
address copilot comment
ndyakov 080a33c
fix(pool): pool performance (#3565)
ndyakov 9448059
initConn sets IDLE state
ndyakov b862bf5
Merge remote-tracking branch 'origin/master' into ndyakov/state-machi…
ndyakov d5db534
fix precision of time cache and usedAt
ndyakov dcd8f9c
allow e2e tests to run longer
ndyakov f1c8884
Merge branch 'master' into ndyakov/state-machine-conn
ndyakov 0752aec
Fix broken initialization of idle connections
ndyakov 54281d6
optimize push notif
ndyakov 600dfe2
100ms -> 50ms
ndyakov dccf01f
use correct timer for last health check
ndyakov 7201275
verify pass auth on conn creation
ndyakov 62eecaa
fix assertion
ndyakov 43eeae7
fix unsafe test
ndyakov 2965e3d
fix benchmark test
ndyakov 59da35b
improve remove conn
ndyakov 09a2f07
re doesn't support requirepass
ndyakov fc2da24
wait more in e2e test
ndyakov 5f0b58b
Merge branch 'master' into ndyakov/state-machine-conn
ndyakov d207749
flaky test
ndyakov ef3e06f
Merge remote-tracking branch 'origin/master' into ndyakov/state-machi…
ndyakov 5fa97c8
add missed method in interface
ndyakov d91800d
fix test assertions
ndyakov c5ca81d
Merge branch 'master' into ndyakov/state-machine-conn
ndyakov a39dd4c
silence logs and faster hooks manager
ndyakov c3dbc8c
address linter comment
ndyakov 3b65139
Merge branch 'master' into ndyakov/state-machine-conn
ndyakov 95af71c
Merge branch 'master' into ndyakov/state-machine-conn
ndyakov 18b46a1
fix flaky test
ndyakov 4673c62
use read instad of control
ndyakov 2b8023c
use pool size for semsize
ndyakov 41024f7
CAS instead of reading the state
ndyakov 3cb5ab3
Merge branch 'master' into ndyakov/state-machine-conn
ndyakov 9466c1c
preallocate errors and states
ndyakov cca0382
preallocate state slices
ndyakov c412436
Merge branch 'master' into ndyakov/state-machine-conn
ndyakov edf6bd7
fix flaky test
ndyakov 1dfa805
fix fast semaphore that could have been starved
ndyakov e418737
try to fix the semaphore
ndyakov dc319c0
should properly notify the waiters
ndyakov 3fd7e08
waiter may double-release (if closed/times out)
ndyakov 5b43c3c
priority of operations
ndyakov dd630c7
use simple approach of fifo waiters
ndyakov ac84940
use simple channel based semaphores
ndyakov 08d9744
address linter and tests
ndyakov 26ee59c
remove unused benchs
ndyakov d6159b9
change log message
ndyakov 82dcbab
Merge branch 'master' into ndyakov/state-machine-conn
ndyakov 56e3ae6
Merge branch 'master' into ndyakov/state-machine-conn
ndyakov 9e24679
address pr comments
ndyakov d84b34a
address pr comments
ndyakov 2d5a75b
fix data race
ndyakov 154a6ee
Merge branch 'master' into ndyakov/state-machine-conn
ndyakov File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,241 @@ | ||
| package streaming | ||
|
|
||
| import ( | ||
| "sync" | ||
| "sync/atomic" | ||
| "testing" | ||
| "time" | ||
|
|
||
| "github.com/redis/go-redis/v9/internal/pool" | ||
| ) | ||
|
|
||
| // TestReAuthOnlyWhenIdle verifies that re-authentication only happens when | ||
| // a connection is in IDLE state, not when it's IN_USE. | ||
| func TestReAuthOnlyWhenIdle(t *testing.T) { | ||
| // Create a connection | ||
| cn := pool.NewConn(nil) | ||
|
|
||
| // Initialize to IDLE state | ||
| cn.GetStateMachine().Transition(pool.StateInitializing) | ||
| cn.GetStateMachine().Transition(pool.StateIdle) | ||
|
|
||
| // Simulate connection being acquired (IDLE → IN_USE) | ||
| if !cn.CompareAndSwapUsed(false, true) { | ||
| t.Fatal("Failed to acquire connection") | ||
| } | ||
|
|
||
| // Verify state is IN_USE | ||
| if state := cn.GetStateMachine().GetState(); state != pool.StateInUse { | ||
| t.Errorf("Expected state IN_USE, got %s", state) | ||
| } | ||
|
|
||
| // Try to transition to UNUSABLE (for reauth) - should fail | ||
| _, err := cn.GetStateMachine().TryTransition([]pool.ConnState{pool.StateIdle}, pool.StateUnusable) | ||
| if err == nil { | ||
| t.Error("Expected error when trying to transition IN_USE → UNUSABLE, but got none") | ||
| } | ||
|
|
||
| // Verify state is still IN_USE | ||
| if state := cn.GetStateMachine().GetState(); state != pool.StateInUse { | ||
| t.Errorf("Expected state to remain IN_USE, got %s", state) | ||
| } | ||
|
|
||
| // Release connection (IN_USE → IDLE) | ||
| if !cn.CompareAndSwapUsed(true, false) { | ||
| t.Fatal("Failed to release connection") | ||
| } | ||
|
|
||
| // Verify state is IDLE | ||
| if state := cn.GetStateMachine().GetState(); state != pool.StateIdle { | ||
| t.Errorf("Expected state IDLE, got %s", state) | ||
| } | ||
|
|
||
| // Now try to transition to UNUSABLE - should succeed | ||
| _, err = cn.GetStateMachine().TryTransition([]pool.ConnState{pool.StateIdle}, pool.StateUnusable) | ||
| if err != nil { | ||
| t.Errorf("Failed to transition IDLE → UNUSABLE: %v", err) | ||
| } | ||
|
|
||
| // Verify state is UNUSABLE | ||
| if state := cn.GetStateMachine().GetState(); state != pool.StateUnusable { | ||
| t.Errorf("Expected state UNUSABLE, got %s", state) | ||
| } | ||
| } | ||
|
|
||
| // TestReAuthWaitsForConnectionToBeIdle verifies that the re-auth worker | ||
| // waits for a connection to become IDLE before performing re-authentication. | ||
| func TestReAuthWaitsForConnectionToBeIdle(t *testing.T) { | ||
| // Create a connection | ||
| cn := pool.NewConn(nil) | ||
|
|
||
| // Initialize to IDLE state | ||
| cn.GetStateMachine().Transition(pool.StateInitializing) | ||
| cn.GetStateMachine().Transition(pool.StateIdle) | ||
|
|
||
| // Simulate connection being acquired (IDLE → IN_USE) | ||
| if !cn.CompareAndSwapUsed(false, true) { | ||
| t.Fatal("Failed to acquire connection") | ||
| } | ||
|
|
||
| // Track re-auth attempts | ||
| var reAuthAttempts atomic.Int32 | ||
| var reAuthSucceeded atomic.Bool | ||
|
|
||
| // Start a goroutine that tries to acquire the connection for re-auth | ||
| var wg sync.WaitGroup | ||
| wg.Add(1) | ||
| go func() { | ||
| defer wg.Done() | ||
|
|
||
| // Try to acquire for re-auth with timeout | ||
| timeout := time.After(2 * time.Second) | ||
| acquired := false | ||
|
|
||
| for !acquired { | ||
| select { | ||
| case <-timeout: | ||
| t.Error("Timeout waiting to acquire connection for re-auth") | ||
| return | ||
| default: | ||
| reAuthAttempts.Add(1) | ||
| // Try to atomically transition from IDLE to UNUSABLE | ||
| _, err := cn.GetStateMachine().TryTransition([]pool.ConnState{pool.StateIdle}, pool.StateUnusable) | ||
| if err == nil { | ||
| // Successfully acquired | ||
| acquired = true | ||
| reAuthSucceeded.Store(true) | ||
| } else { | ||
| // Connection is still IN_USE, wait a bit | ||
| time.Sleep(10 * time.Millisecond) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Release the connection | ||
| cn.GetStateMachine().Transition(pool.StateIdle) | ||
| }() | ||
|
|
||
| // Keep connection IN_USE for 500ms | ||
| time.Sleep(500 * time.Millisecond) | ||
|
|
||
| // Verify re-auth hasn't succeeded yet (connection is still IN_USE) | ||
| if reAuthSucceeded.Load() { | ||
| t.Error("Re-auth succeeded while connection was IN_USE") | ||
| } | ||
|
|
||
| // Verify there were multiple attempts | ||
| attempts := reAuthAttempts.Load() | ||
| if attempts < 2 { | ||
| t.Errorf("Expected multiple re-auth attempts, got %d", attempts) | ||
| } | ||
|
|
||
| // Release connection (IN_USE → IDLE) | ||
| if !cn.CompareAndSwapUsed(true, false) { | ||
| t.Fatal("Failed to release connection") | ||
| } | ||
|
|
||
| // Wait for re-auth to complete | ||
| wg.Wait() | ||
|
|
||
| // Verify re-auth succeeded after connection became IDLE | ||
| if !reAuthSucceeded.Load() { | ||
| t.Error("Re-auth did not succeed after connection became IDLE") | ||
| } | ||
|
|
||
| // Verify final state is IDLE | ||
| if state := cn.GetStateMachine().GetState(); state != pool.StateIdle { | ||
| t.Errorf("Expected final state IDLE, got %s", state) | ||
| } | ||
| } | ||
|
|
||
| // TestConcurrentReAuthAndUsage verifies that re-auth and normal usage | ||
| // don't interfere with each other. | ||
| func TestConcurrentReAuthAndUsage(t *testing.T) { | ||
| // Create a connection | ||
| cn := pool.NewConn(nil) | ||
|
|
||
| // Initialize to IDLE state | ||
| cn.GetStateMachine().Transition(pool.StateInitializing) | ||
| cn.GetStateMachine().Transition(pool.StateIdle) | ||
|
|
||
| var wg sync.WaitGroup | ||
| var usageCount atomic.Int32 | ||
| var reAuthCount atomic.Int32 | ||
|
|
||
| // Goroutine 1: Simulate normal usage (acquire/release) | ||
| wg.Add(1) | ||
| go func() { | ||
| defer wg.Done() | ||
| for i := 0; i < 100; i++ { | ||
| // Try to acquire | ||
| if cn.CompareAndSwapUsed(false, true) { | ||
| usageCount.Add(1) | ||
| // Simulate work | ||
| time.Sleep(1 * time.Millisecond) | ||
| // Release | ||
| cn.CompareAndSwapUsed(true, false) | ||
| } | ||
| time.Sleep(1 * time.Millisecond) | ||
| } | ||
| }() | ||
|
|
||
| // Goroutine 2: Simulate re-auth attempts | ||
| wg.Add(1) | ||
| go func() { | ||
| defer wg.Done() | ||
| for i := 0; i < 50; i++ { | ||
| // Try to acquire for re-auth | ||
| _, err := cn.GetStateMachine().TryTransition([]pool.ConnState{pool.StateIdle}, pool.StateUnusable) | ||
| if err == nil { | ||
| reAuthCount.Add(1) | ||
| // Simulate re-auth work | ||
| time.Sleep(2 * time.Millisecond) | ||
| // Release | ||
| cn.GetStateMachine().Transition(pool.StateIdle) | ||
| } | ||
| time.Sleep(2 * time.Millisecond) | ||
| } | ||
| }() | ||
|
|
||
| wg.Wait() | ||
|
|
||
| // Verify both operations happened | ||
| if usageCount.Load() == 0 { | ||
| t.Error("No successful usage operations") | ||
| } | ||
| if reAuthCount.Load() == 0 { | ||
| t.Error("No successful re-auth operations") | ||
| } | ||
|
|
||
| t.Logf("Usage operations: %d, Re-auth operations: %d", usageCount.Load(), reAuthCount.Load()) | ||
|
|
||
| // Verify final state is IDLE | ||
| if state := cn.GetStateMachine().GetState(); state != pool.StateIdle { | ||
| t.Errorf("Expected final state IDLE, got %s", state) | ||
| } | ||
| } | ||
|
|
||
| // TestReAuthRespectsClosed verifies that re-auth doesn't happen on closed connections. | ||
| func TestReAuthRespectsClosed(t *testing.T) { | ||
| // Create a connection | ||
| cn := pool.NewConn(nil) | ||
|
|
||
| // Initialize to IDLE state | ||
| cn.GetStateMachine().Transition(pool.StateInitializing) | ||
| cn.GetStateMachine().Transition(pool.StateIdle) | ||
|
|
||
| // Close the connection | ||
| cn.GetStateMachine().Transition(pool.StateClosed) | ||
|
|
||
| // Try to transition to UNUSABLE - should fail | ||
| _, err := cn.GetStateMachine().TryTransition([]pool.ConnState{pool.StateIdle}, pool.StateUnusable) | ||
| if err == nil { | ||
| t.Error("Expected error when trying to transition CLOSED → UNUSABLE, but got none") | ||
| } | ||
|
|
||
| // Verify state is still CLOSED | ||
| if state := cn.GetStateMachine().GetState(); state != pool.StateClosed { | ||
| t.Errorf("Expected state to remain CLOSED, got %s", state) | ||
| } | ||
| } | ||
|
|
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.