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

Jump to bottom

Fix query cache goleaks #675

Merged
merged 8 commits into from
Feb 1, 2021
Merged

Fix query cache goleaks #675

merged 8 commits into from
Feb 1, 2021

Conversation

Bolodya1997
Copy link

@Bolodya1997 Bolodya1997 commented Jan 27, 2021 •
edited
Loading

Issues

  1. Query cache starts watching Find on chain context, it lasts until the application closes.
  2. There is a possible leak when registry deletes NSE in process of cache Find.

Solutions

  1. Start watching Find on special context and close it after NSE remove.
  2. Double check that NSE exists in registry before starting to wait events for it.

denis-tingaikin
denis-tingaikin requested changes Jan 27, 2021
View reviewed changes
pkg/registry/common/querycache/nse_client.go Outdated Show resolved Hide resolved
pkg/registry/common/querycache/nse_client.go Show resolved Hide resolved
pkg/tools/sandbox/utils.go Outdated Show resolved Hide resolved
pkg/networkservice/chains/nsmgr/server_test.go Outdated Show resolved Hide resolved
@Bolodya1997 Bolodya1997 marked this pull request as ready for review January 27, 2021 09:50
@Bolodya1997 Bolodya1997 marked this pull request as draft January 27, 2021 10:08
@Bolodya1997 Bolodya1997 marked this pull request as ready for review January 28, 2021 04:42
@Bolodya1997 Bolodya1997 marked this pull request as draft January 28, 2021 04:44
@Bolodya1997 Bolodya1997 marked this pull request as ready for review January 28, 2021 11:55
Vladimir Popov added 6 commits January 29, 2021 13:35
Fix query cache
4787178 
Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>
Add sandbox test for Request/Close goleaks
e7dd431 
Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>
Remove queryCache.checkQuery()
198e891 
Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>
Fix tests
447859c 
Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>
Rework TestNSMGR_ShouldCleanAllClientAndEndpointGoroutines to allow i…
53ed936 
…nit() goroutines

Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>
Refactor tests
2e7b290 
Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>
@Bolodya1997 Bolodya1997 mentioned this pull request Jan 29, 2021
Closed
@Bolodya1997 Bolodya1997 marked this pull request as draft January 29, 2021 11:10
@Bolodya1997 Bolodya1997 marked this pull request as ready for review January 29, 2021 11:39
Add sandbox.Unregister()
fc55c72 
Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>
denis-tingaikin
denis-tingaikin reviewed Jan 29, 2021
View reviewed changes
pkg/tools/sandbox/utils.go Outdated
return &EndpointEntry{Endpoint: ep, URL: u}, nil
}

// UnregisterEndpoint unregisters endpoint from NSMgr
func UnregisterEndpoint(ctx context.Context, nse *registry.NetworkServiceEndpoint, mgr nsmgr.Nsmgr) (err error) {
Copy link
Member

@denis-tingaikin denis-tingaikin Jan 29, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

As I can see this func is using by one test. Can we make this inline?

Copy link
Author

Choose a reason for hiding this comment

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

Yep, inlined.

Inline sandbox.UnregisterEndpoint()
8e23ffb 
Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>
@denis-tingaikin denis-tingaikin merged commit 20d5104 into networkservicemesh:master Feb 1, 2021
nsmbot pushed a commit to networkservicemesh/cmd-registry-memory that referenced this pull request Feb 1, 2021
Update go.mod and go.sum to latest version from networkservicemesh/sd…
6d05359 
…k@master networkservicemesh/sdk#675

networkservicemesh/sdk PR link: networkservicemesh/sdk#675

networkservicemesh/sdk commit message:
commit 20d510498cab80f1edfc44462da56403cd700f6b
Author: Vladimir Popov <vladimir.popov@xored.com>
Date:   Mon Feb 1 12:30:32 2021 +0700

    Fix query cache goleaks (#675)

    * Fix query cache

    Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>

    * Add sandbox test for Request/Close goleaks

    Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>

    * Remove queryCache.checkQuery()

    Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>

    * Fix tests

    Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>

    * Rework TestNSMGR_ShouldCleanAllClientAndEndpointGoroutines to allow init() goroutines

    Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>

    * Refactor tests

    Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>

    * Add sandbox.Unregister()

    Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>

    * Inline sandbox.UnregisterEndpoint()

    Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>

Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
nsmbot pushed a commit to networkservicemesh/cmd-nse-vfio that referenced this pull request Feb 1, 2021
Update go.mod and go.sum to latest version from networkservicemesh/sd…
95bc58e 
…k@master networkservicemesh/sdk#675

networkservicemesh/sdk PR link: networkservicemesh/sdk#675

networkservicemesh/sdk commit message:
commit 20d510498cab80f1edfc44462da56403cd700f6b
Author: Vladimir Popov <vladimir.popov@xored.com>
Date:   Mon Feb 1 12:30:32 2021 +0700

    Fix query cache goleaks (#675)

    * Fix query cache

    Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>

    * Add sandbox test for Request/Close goleaks

    Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>

    * Remove queryCache.checkQuery()

    Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>

    * Fix tests

    Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>

    * Rework TestNSMGR_ShouldCleanAllClientAndEndpointGoroutines to allow init() goroutines

    Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>

    * Refactor tests

    Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>

    * Add sandbox.Unregister()

    Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>

    * Inline sandbox.UnregisterEndpoint()

    Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>

Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
nsmbot pushed a commit to networkservicemesh/cmd-registry-proxy-dns that referenced this pull request Feb 1, 2021
Update go.mod and go.sum to latest version from networkservicemesh/sd…
50626b2 
…k@master networkservicemesh/sdk#675

networkservicemesh/sdk PR link: networkservicemesh/sdk#675

networkservicemesh/sdk commit message:
commit 20d510498cab80f1edfc44462da56403cd700f6b
Author: Vladimir Popov <vladimir.popov@xored.com>
Date:   Mon Feb 1 12:30:32 2021 +0700

    Fix query cache goleaks (#675)

    * Fix query cache

    Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>

    * Add sandbox test for Request/Close goleaks

    Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>

    * Remove queryCache.checkQuery()

    Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>

    * Fix tests

    Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>

    * Rework TestNSMGR_ShouldCleanAllClientAndEndpointGoroutines to allow init() goroutines

    Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>

    * Refactor tests

    Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>

    * Add sandbox.Unregister()

    Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>

    * Inline sandbox.UnregisterEndpoint()

    Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>

Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
nsmbot pushed a commit to networkservicemesh/sdk-kernel that referenced this pull request Feb 1, 2021
Update go.mod and go.sum to latest version from networkservicemesh/sd…
11e104e 
…k@master networkservicemesh/sdk#675

networkservicemesh/sdk PR link: networkservicemesh/sdk#675

networkservicemesh/sdk commit message:
commit 20d510498cab80f1edfc44462da56403cd700f6b
Author: Vladimir Popov <vladimir.popov@xored.com>
Date:   Mon Feb 1 12:30:32 2021 +0700

    Fix query cache goleaks (#675)

    * Fix query cache

    Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>

    * Add sandbox test for Request/Close goleaks

    Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>

    * Remove queryCache.checkQuery()

    Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>

    * Fix tests

    Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>

    * Rework TestNSMGR_ShouldCleanAllClientAndEndpointGoroutines to allow init() goroutines

    Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>

    * Refactor tests

    Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>

    * Add sandbox.Unregister()

    Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>

    * Inline sandbox.UnregisterEndpoint()

    Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>

Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
nsmbot pushed a commit to networkservicemesh/sdk-vpp that referenced this pull request Feb 1, 2021
Update go.mod and go.sum to latest version from networkservicemesh/sd…
0a8f9dc 
…k@master networkservicemesh/sdk#675

networkservicemesh/sdk PR link: networkservicemesh/sdk#675

networkservicemesh/sdk commit message:
commit 20d510498cab80f1edfc44462da56403cd700f6b
Author: Vladimir Popov <vladimir.popov@xored.com>
Date:   Mon Feb 1 12:30:32 2021 +0700

    Fix query cache goleaks (#675)

    * Fix query cache

    Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>

    * Add sandbox test for Request/Close goleaks

    Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>

    * Remove queryCache.checkQuery()

    Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>

    * Fix tests

    Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>

    * Rework TestNSMGR_ShouldCleanAllClientAndEndpointGoroutines to allow init() goroutines

    Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>

    * Refactor tests

    Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>

    * Add sandbox.Unregister()

    Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>

    * Inline sandbox.UnregisterEndpoint()

    Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>

Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
nsmbot pushed a commit to networkservicemesh/cmd-nsmgr that referenced this pull request Feb 1, 2021
Update go.mod and go.sum to latest version from networkservicemesh/sd…
e71453b 
…k@master networkservicemesh/sdk#675

networkservicemesh/sdk PR link: networkservicemesh/sdk#675

networkservicemesh/sdk commit message:
commit 20d510498cab80f1edfc44462da56403cd700f6b
Author: Vladimir Popov <vladimir.popov@xored.com>
Date:   Mon Feb 1 12:30:32 2021 +0700

    Fix query cache goleaks (#675)

    * Fix query cache

    Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>

    * Add sandbox test for Request/Close goleaks

    Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>

    * Remove queryCache.checkQuery()

    Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>

    * Fix tests

    Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>

    * Rework TestNSMGR_ShouldCleanAllClientAndEndpointGoroutines to allow init() goroutines

    Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>

    * Refactor tests

    Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>

    * Add sandbox.Unregister()

    Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>

    * Inline sandbox.UnregisterEndpoint()

    Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>

Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
nsmbot pushed a commit to networkservicemesh/cmd-nse-icmp-responder that referenced this pull request Feb 1, 2021
Update go.mod and go.sum to latest version from networkservicemesh/sd…
caf7a03 
…k@master networkservicemesh/sdk#675

networkservicemesh/sdk PR link: networkservicemesh/sdk#675

networkservicemesh/sdk commit message:
commit 20d510498cab80f1edfc44462da56403cd700f6b
Author: Vladimir Popov <vladimir.popov@xored.com>
Date:   Mon Feb 1 12:30:32 2021 +0700

    Fix query cache goleaks (#675)

    * Fix query cache

    Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>

    * Add sandbox test for Request/Close goleaks

    Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>

    * Remove queryCache.checkQuery()

    Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>

    * Fix tests

    Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>

    * Rework TestNSMGR_ShouldCleanAllClientAndEndpointGoroutines to allow init() goroutines

    Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>

    * Refactor tests

    Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>

    * Add sandbox.Unregister()

    Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>

    * Inline sandbox.UnregisterEndpoint()

    Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>

Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
nsmbot pushed a commit to networkservicemesh/cmd-nsmgr-proxy that referenced this pull request Feb 1, 2021
Update go.mod and go.sum to latest version from networkservicemesh/sd…
05b2028 
…k@master networkservicemesh/sdk#675

networkservicemesh/sdk PR link: networkservicemesh/sdk#675

networkservicemesh/sdk commit message:
commit 20d510498cab80f1edfc44462da56403cd700f6b
Author: Vladimir Popov <vladimir.popov@xored.com>
Date:   Mon Feb 1 12:30:32 2021 +0700

    Fix query cache goleaks (#675)

    * Fix query cache

    Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>

    * Add sandbox test for Request/Close goleaks

    Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>

    * Remove queryCache.checkQuery()

    Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>

    * Fix tests

    Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>

    * Rework TestNSMGR_ShouldCleanAllClientAndEndpointGoroutines to allow init() goroutines

    Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>

    * Refactor tests

    Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>

    * Add sandbox.Unregister()

    Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>

    * Inline sandbox.UnregisterEndpoint()

    Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>

Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
nsmbot pushed a commit to networkservicemesh/sdk-k8s that referenced this pull request Feb 1, 2021
Update go.mod and go.sum to latest version from networkservicemesh/sd…
50acfb9 
…k@master networkservicemesh/sdk#675

networkservicemesh/sdk PR link: networkservicemesh/sdk#675

networkservicemesh/sdk commit message:
commit 20d510498cab80f1edfc44462da56403cd700f6b
Author: Vladimir Popov <vladimir.popov@xored.com>
Date:   Mon Feb 1 12:30:32 2021 +0700

    Fix query cache goleaks (#675)

    * Fix query cache

    Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>

    * Add sandbox test for Request/Close goleaks

    Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>

    * Remove queryCache.checkQuery()

    Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>

    * Fix tests

    Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>

    * Rework TestNSMGR_ShouldCleanAllClientAndEndpointGoroutines to allow init() goroutines

    Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>

    * Refactor tests

    Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>

    * Add sandbox.Unregister()

    Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>

    * Inline sandbox.UnregisterEndpoint()

    Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>

Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
This was referenced Feb 1, 2021
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
@Bolodya1997 Bolodya1997 deleted the fix-find-watch branch February 12, 2021 05:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@denis-tingaikin denis-tingaikin denis-tingaikin approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Bolodya1997 @denis-tingaikin