Skip to content

Commit

Permalink
unskip TestPooledQuery_deadlock_regression test
Browse files Browse the repository at this point in the history
  • Loading branch information
dennis-tra committed Oct 13, 2023
1 parent c0f91ca commit 270f20c
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 12 deletions.
5 changes: 3 additions & 2 deletions internal/coord/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -467,9 +467,10 @@ func (w *queryNotifier[E]) NotifyFinished(ctx context.Context, ev E) {
w.DrainPending()
close(w.monitor.NotifyProgressed())

finishedChan := w.monitor.NotifyFinished()
select {
case w.monitor.NotifyFinished() <- CtxEvent[E]{Ctx: ctx, Event: ev}:
case finishedChan <- CtxEvent[E]{Ctx: ctx, Event: ev}:
default:
}
close(w.monitor.NotifyFinished())
close(finishedChan)
}
19 changes: 9 additions & 10 deletions internal/coord/query_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package coord

import (
"context"
"sync"
"testing"

"github.com/benbjohnson/clock"
Expand Down Expand Up @@ -275,7 +274,6 @@ func (ts *QueryBehaviourBaseTestSuite) TestNotifiesQueryFinished() {
}

func TestPooledQuery_deadlock_regression(t *testing.T) {
t.Skip()
ctx := kadtest.CtxShort(t)
msg := &pb.Message{}
queryID := coordt.QueryID("test")
Expand Down Expand Up @@ -342,16 +340,13 @@ func TestPooledQuery_deadlock_regression(t *testing.T) {
// Advance the query pool state machine. Because we returned a new node
// above, the query pool state machine wants to send another outbound query
ev, _ = c.queryBehaviour.Perform(ctx)
require.IsType(t, &EventAddNode{}, ev) // event to notify the routing table
ev, _ = c.queryBehaviour.Perform(ctx)
require.IsType(t, &EventOutboundSendMessage{}, ev)
ev, _ = c.queryBehaviour.Perform(ctx)
require.IsType(t, &EventAddNode{}, ev) // event to notify the routing table

hasLock := make(chan struct{})
var once sync.Once
wrappedWaiter.BeforeProgressed = func() {
once.Do(func() {
close(hasLock)
})
wrappedWaiter.BeforeFinished = func() {
close(hasLock)
}

// Simulate a successful response from the new node. This node didn't return
Expand All @@ -361,7 +356,11 @@ func TestPooledQuery_deadlock_regression(t *testing.T) {
// of 1, the channel cannot hold both events. At the same time, the waiter
// doesn't consume the messages because it's busy processing the previous
// query event (because we haven't released the blocking waiterMsg call above).
go c.queryBehaviour.Notify(ctx, successMsg(nodes[2].NodeID))
c.queryBehaviour.Notify(ctx, successMsg(nodes[2].NodeID))

ev, ok := c.queryBehaviour.Perform(ctx)
require.Nil(t, ev)
require.False(t, ok)

// wait until the above Notify call was handled by waiting until the hasLock
// channel was closed in the above BeforeNotify hook. If that hook is called
Expand Down

0 comments on commit 270f20c

Please sign in to comment.