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.

Sign up for GitHub

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

Cleanup expired queries #175

Merged
merged 3 commits into from
Feb 7, 2024
Merged

Cleanup expired queries #175

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
225c67e
Cleanup expired queries
SimonVerkada Feb 7, 2024
5662268
Cleanup queries by reference instead of name
SimonVerkada Feb 7, 2024
dfc49cb
Create ticker outside critical section
SimonVerkada Feb 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions conn.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,15 @@ func (c *Conn) Query(ctx context.Context, name string) (dnsmessage.ResourceHeade
c.mu.Unlock()

defer ticker.Stop()
defer func() {
c.mu.Lock()
defer c.mu.Unlock()
for i := len(c.queries) - 1; i >= 0; i-- {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

super-nit

Suggested change
for i := len(c.queries) - 1; i >= 0; i-- {
for i := 0; i < len(c.queries); i++ {

if c.queries[i].nameWithSuffix == nameWithSuffix {
c.queries = append(c.queries[:i], c.queries[i+1:]...)
}
}
SimonVerkada marked this conversation as resolved.
Show resolved Hide resolved
}()

c.sendQuestion(nameWithSuffix)
for {
Expand Down
22 changes: 22 additions & 0 deletions conn_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,13 @@ func TestValidCommunication(t *testing.T) {

check(aServer.Close(), t)
check(bServer.Close(), t)

if len(aServer.queries) > 0 {
t.Fatalf("Queries not cleaned up after aServer close")
}
if len(bServer.queries) > 0 {
t.Fatalf("Queries not cleaned up after bServer close")
}
}

func TestValidCommunicationWithAddressConfig(t *testing.T) {
Expand All @@ -93,6 +100,9 @@ func TestValidCommunicationWithAddressConfig(t *testing.T) {
}

check(aServer.Close(), t)
if len(aServer.queries) > 0 {
t.Fatalf("Queries not cleaned up after aServer close")
}
}

func TestMultipleClose(t *testing.T) {
Expand All @@ -109,6 +119,10 @@ func TestMultipleClose(t *testing.T) {

check(server.Close(), t)
check(server.Close(), t)

if len(server.queries) > 0 {
t.Fatalf("Queries not cleaned up after server close")
}
}

func TestQueryRespectTimeout(t *testing.T) {
Expand All @@ -133,6 +147,10 @@ func TestQueryRespectTimeout(t *testing.T) {
if closeErr := server.Close(); closeErr != nil {
t.Fatal(closeErr)
}

if len(server.queries) > 0 {
t.Fatalf("Queries not cleaned up after context expiration")
}
}

func TestQueryRespectClose(t *testing.T) {
Expand All @@ -159,6 +177,10 @@ func TestQueryRespectClose(t *testing.T) {
if _, _, err = server.Query(context.TODO(), "invalid-host"); !errors.Is(err, errConnectionClosed) {
t.Fatalf("Query on closed server but returned unexpected error %v", err)
}

if len(server.queries) > 0 {
t.Fatalf("Queries not cleaned up after query")
}
}

func TestResourceParsing(t *testing.T) {
Expand Down
Loading