-
Notifications
You must be signed in to change notification settings - Fork 2.2k
graph: fix zombie chan pruning #10340
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,138 @@ | ||
| //go:build test_db_postgres || test_db_sqlite | ||
|
|
||
| package graphdb | ||
|
|
||
| import ( | ||
| "testing" | ||
| "time" | ||
|
|
||
| "github.com/lightningnetwork/lnd/fn/v2" | ||
| "github.com/stretchr/testify/require" | ||
| ) | ||
|
|
||
| // TestChanUpdatesInHorizonWithNoPolicies tests that we're able to properly | ||
| // retrieve channels with no policies within the time range. | ||
| func TestChanUpdatesInHorizonWithNoPolicies(t *testing.T) { | ||
| t.Parallel() | ||
| ctx := t.Context() | ||
|
|
||
| graph := MakeTestGraph(t) | ||
|
|
||
| // We'll start by creating two nodes which will seed our test graph. | ||
| node1 := createTestVertex(t) | ||
| require.NoError(t, graph.AddNode(ctx, node1)) | ||
|
|
||
| node2 := createTestVertex(t) | ||
| require.NoError(t, graph.AddNode(ctx, node2)) | ||
|
|
||
| // Note: startTime and endTime only works if the channels have | ||
| // policies. If not, the channels are included irrespective of the | ||
| // time range. | ||
| startTime := time.Unix(1234, 0) | ||
| endTime := startTime | ||
| edges := make([]ChannelEdge, 0, 10) | ||
|
|
||
| // We'll now create 10 channels between the two nodes, with no policies. | ||
| const numChans = 10 | ||
| for i := range numChans { | ||
| channel, chanID := createEdge( | ||
| uint32(i*10), 0, 0, 0, node1, node2, | ||
| ) | ||
| require.NoError(t, graph.AddChannelEdge(ctx, &channel)) | ||
|
|
||
| // The first 5 channels will have no policies. | ||
| if i < numChans/2 { | ||
| edges = append(edges, ChannelEdge{ | ||
| Info: &channel, | ||
| }) | ||
|
|
||
| continue | ||
| } | ||
|
|
||
| edge1UpdateTime := endTime | ||
| edge2UpdateTime := edge1UpdateTime.Add(time.Second) | ||
| endTime = endTime.Add(time.Second * 10) | ||
|
|
||
| edge1 := newEdgePolicy( | ||
| chanID.ToUint64(), edge1UpdateTime.Unix(), | ||
| ) | ||
| edge1.ChannelFlags = 0 | ||
| edge1.ToNode = node2.PubKeyBytes | ||
| edge1.SigBytes = testSig.Serialize() | ||
| require.NoError(t, graph.UpdateEdgePolicy(ctx, edge1)) | ||
|
|
||
| edge2 := newEdgePolicy( | ||
| chanID.ToUint64(), edge2UpdateTime.Unix(), | ||
| ) | ||
| edge2.ChannelFlags = 1 | ||
| edge2.ToNode = node1.PubKeyBytes | ||
| edge2.SigBytes = testSig.Serialize() | ||
| require.NoError(t, graph.UpdateEdgePolicy(ctx, edge2)) | ||
|
|
||
| edges = append(edges, ChannelEdge{ | ||
| Info: &channel, | ||
| Policy1: edge1, | ||
| Policy2: edge2, | ||
| }) | ||
| } | ||
|
|
||
| // With our channels loaded, we'll now start our series of queries. | ||
| queryCases := []struct { | ||
| start time.Time | ||
| end time.Time | ||
| resp []ChannelEdge | ||
| }{ | ||
| // If we query for a time range that's strictly below our set | ||
| // of updates, then we'll get only the 5 channels with no | ||
| // policies. | ||
| { | ||
| start: time.Unix(100, 0), | ||
| end: time.Unix(200, 0), | ||
| resp: edges[:5], | ||
| }, | ||
|
|
||
| // If we query for a time range that's well beyond our set of | ||
| // updates, we should get only the 5 channels with no | ||
| // policies. | ||
| { | ||
| start: time.Unix(99999, 0), | ||
| end: time.Unix(999999, 0), | ||
| resp: edges[:5], | ||
| }, | ||
|
|
||
| // If we query for the start time, and 10 seconds directly | ||
| // after it, we should only get the 5 channels with no | ||
| // policies and one channel with a policy. | ||
| { | ||
| start: time.Unix(1234, 0), | ||
| end: startTime.Add(time.Second * 10), | ||
| resp: edges[:6], | ||
| }, | ||
|
|
||
| // If we use the start and end time as is, we should get the | ||
| // entire range. | ||
| { | ||
| start: startTime, | ||
| end: endTime, | ||
|
|
||
| resp: edges[:10], | ||
| }, | ||
| } | ||
|
|
||
| for _, queryCase := range queryCases { | ||
| respIter := graph.ChanUpdatesInHorizon( | ||
| queryCase.start, queryCase.end, | ||
| ) | ||
|
|
||
| resp, err := fn.CollectErr(respIter) | ||
| require.NoError(t, err) | ||
| require.Equal(t, len(resp), len(queryCase.resp)) | ||
|
|
||
| for i := range len(resp) { | ||
| chanExp := queryCase.resp[i] | ||
| chanRet := resp[i] | ||
|
|
||
| require.Equal(t, chanExp.Info, chanRet.Info) | ||
| } | ||
| } | ||
| } |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -471,6 +471,10 @@ WHERE c.version = @version | |
| (cp1.last_update >= @start_time AND cp1.last_update < @end_time) | ||
| OR | ||
| (cp2.last_update >= @start_time AND cp2.last_update < @end_time) | ||
| -- TODO(abdulkbk): see the potential of adding a created_at to channel | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if we should limit this query since for example couple of nodes will have like >2000 of these cases, should be fine I think just thinking out loud.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it makes sense, especially when this change is run for the first time and the node has many stale channels with no policy. It should be fine for the next ticks (1-hour pruning intervals) tho. |
||
| -- table to extend this query to include channels created in a time range. | ||
| OR | ||
| (cp1.last_update IS NULL AND cp2.last_update IS NULL) | ||
| ) | ||
| -- Pagination using compound cursor (max_update_time, id). | ||
| -- We use COALESCE with -1 as sentinel since timestamps are always positive. | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.