diff --git a/graph/builder.go b/graph/builder.go index b446d1a7b..59e9b19d5 100644 --- a/graph/builder.go +++ b/graph/builder.go @@ -1313,7 +1313,7 @@ func (b *Builder) IsStaleNode(ctx context.Context, node route.Vertex, // then we know that this is actually a stale announcement. err := b.assertNodeAnnFreshness(ctx, node, timestamp) if err != nil { - log.Debugf("Checking stale node %x got %v", node, err) + log.Debugf("Checking stale node %s got %v", node, err) return true } diff --git a/graph/db/graph_test.go b/graph/db/graph_test.go index 482c1aa18..ee5cf8dbf 100644 --- a/graph/db/graph_test.go +++ b/graph/db/graph_test.go @@ -125,6 +125,14 @@ func TestNodeInsertionAndDeletion(t *testing.T) { require.NoError(t, graph.AddNode(ctx, node)) assertNodeInCache(t, graph, node, testFeatures) + // Our AddNode implementation uses the batcher meaning that it is + // possible that two updates for the same node announcement may be + // processed in the same batch. So to avoid the conflict error (since we + // require at the DB level that the new timestamp is strictly + // greater than the previous one), we need to gracefully handle the + // case where the exact same node announcement is added twice. + require.NoError(t, graph.AddNode(ctx, node)) + // Next, fetch the node from the database to ensure everything was // serialized properly. dbNode, err := graph.FetchNode(ctx, testPub) @@ -936,6 +944,13 @@ func TestEdgePolicyCRUD(t *testing.T) { require.NoError(t, graph.UpdateEdgePolicy(ctx, edge1)) require.NoError(t, graph.UpdateEdgePolicy(ctx, edge2)) + // Even though we assert at the DB level that any newer edge + // update has a newer timestamp, we need to still gracefully + // handle the case where the same exact policy is re-added since + // it could be possible that our batch executor has two of the + // same policy updates in the same batch. + require.NoError(t, graph.UpdateEdgePolicy(ctx, edge1)) + // Use the ForEachChannel method to fetch the policies and // assert that the deserialized policies match the original // ones. diff --git a/graph/db/sql_store.go b/graph/db/sql_store.go index f13408765..f67894e4c 100644 --- a/graph/db/sql_store.go +++ b/graph/db/sql_store.go @@ -255,6 +255,17 @@ func (s *SQLStore) AddNode(ctx context.Context, Opts: batch.NewSchedulerOptions(opts...), Do: func(queries SQLQueries) error { _, err := upsertNode(ctx, queries, node) + + // It is possible that two of the same node + // announcements are both being processed in the same + // batch. This may case the UpsertNode conflict to + // be hit since we require at the db layer that the + // new last_update is greater than the existing + // last_update. We need to gracefully handle this here. + if errors.Is(err, sql.ErrNoRows) { + return nil + } + return err }, } @@ -769,7 +780,15 @@ func (s *SQLStore) UpdateEdgePolicy(ctx context.Context, from, to, isUpdate1, err = updateChanEdgePolicy( ctx, tx, edge, ) - if err != nil { + // It is possible that two of the same policy + // announcements are both being processed in the same + // batch. This may case the UpsertEdgePolicy conflict to + // be hit since we require at the db layer that the + // new last_update is greater than the existing + // last_update. We need to gracefully handle this here. + if errors.Is(err, sql.ErrNoRows) { + return nil + } else if err != nil { log.Errorf("UpdateEdgePolicy faild: %v", err) } diff --git a/lncfg/db.go b/lncfg/db.go index 0f55c736e..9eb027887 100644 --- a/lncfg/db.go +++ b/lncfg/db.go @@ -86,7 +86,7 @@ type DB struct { Sqlite *sqldb.SqliteConfig `group:"sqlite" namespace:"sqlite" description:"Sqlite settings."` - UseNativeSQL bool `long:"use-native-sql" description:"If set to true, native SQL will be used instead of KV emulation for tables that support it. Subsystems which support native SQL tables: Invoices."` + UseNativeSQL bool `long:"use-native-sql" description:"If set to true, native SQL will be used instead of KV emulation for tables that support it. Subsystems which support native SQL tables: Invoices, Graph."` SkipNativeSQLMigration bool `long:"skip-native-sql-migration" description:"If set to true, the KV to native SQL migration will be skipped. Note that this option is intended for users who experience non-resolvable migration errors. Enabling after there is a non-resolvable migration error that resulted in an incomplete migration will cause that partial migration to be abandoned and ignored and an empty database will be used instead. Since invoices are currently the only native SQL database used, our channels will still work but the invoice history will be forgotten. This option has no effect if native SQL is not in use (db.use-native-sql=false)."` diff --git a/sample-lnd.conf b/sample-lnd.conf index 506b99914..c3b3a96b1 100644 --- a/sample-lnd.conf +++ b/sample-lnd.conf @@ -1506,6 +1506,7 @@ ; that support it. ; Subsystems which support native SQL tables: ; - Invoices +; - Graph ; db.use-native-sql=false ; If set to true, the KV to native SQL migration will be skipped. Note that