Merge pull request #10356 from Abdulkbk/ispublicnode-perf

graph: fix inefficient query for IsPublicNode

docs/release-notes/release-notes-0.20.1.md

@@ -98,6 +98,10 @@
   safe single-writer behavior until the wallet subsystem is fully 
   concurrent-safe.
 
+* [Modified the query for `IsPublicV1Node`](https://github.com/lightningnetwork/lnd/pull/10356)
+  to use `UNION ALL` instead of `OR` conditions in the `WHERE` clause, improving
+  performance when checking for public nodes especially in large graphs when using `SQL` backends.
+
 ## Deprecations
 
 # Technical and Architectural Updates
@@ -113,5 +117,6 @@
 
 # Contributors (Alphabetical Order)
 
+* Abdulkbk
 * bitromortac
 * Ziggie

graph/db/graph_test.go

@@ -1640,6 +1640,8 @@ func TestGraphCacheTraversal(t *testing.T) {
 	require.Equal(t, numChannels*2*(numNodes-1), numNodeChans)
 }
 
+// fillTestGraph fills the graph with a given number of nodes and create a given
+// number of channels between each node.
 func fillTestGraph(t testing.TB, graph *ChannelGraph, numNodes,
 	numChannels int) (map[uint64]struct{}, []*models.Node) {
 
@@ -4042,6 +4044,28 @@ func TestNodeIsPublic(t *testing.T) {
 	)
 }
 
+// BenchmarkIsPublicNode measures the performance of IsPublicNode when checking
+// a large number of nodes.
+func BenchmarkIsPublicNode(b *testing.B) {
+	graph := MakeTestGraph(b)
+
+	// Create a graph with a reasonable number of nodes and channels.
+	numNodes := 100
+	numChans := 4
+	_, nodes := fillTestGraph(b, graph, numNodes, numChans)
+
+	// Use deterministic random number generator for reproducible results.
+	rng := prand.New(prand.NewSource(42))
+
+	for b.Loop() {
+		// Query random nodes to avoid query caching and better
+		// represent real-world query patterns.
+		nodePub := nodes[rng.Intn(len(nodes))].PubKeyBytes
+		_, err := graph.IsPublicNode(nodePub)
+		require.NoError(b, err)
+	}
+}
+
 // TestDisabledChannelIDs ensures that the disabled channels within the
 // disabledEdgePolicyBucket are managed properly and the list returned from
 // DisabledChannelIDs is correct.

sqldb/sqlc/graph.sql.go

@@ -2653,7 +2653,7 @@ const isPublicV1Node = `-- name: IsPublicV1Node :one
 SELECT EXISTS (
     SELECT 1
     FROM graph_channels c
-    JOIN graph_nodes n ON n.id = c.node_id_1 OR n.id = c.node_id_2
+    JOIN graph_nodes n ON n.id = c.node_id_1
     -- NOTE: we hard-code the version here since the clauses
     -- here that determine if a node is public is specific
     -- to the V1 gossip protocol. In V1, a node is public
@@ -2665,6 +2665,13 @@ SELECT EXISTS (
     WHERE c.version = 1
       AND c.bitcoin_1_signature IS NOT NULL
       AND n.pub_key = $1
+    UNION ALL
+    SELECT 1
+    FROM graph_channels c
+    JOIN graph_nodes n ON n.id = c.node_id_2
+    WHERE c.version = 1
+      AND c.bitcoin_1_signature IS NOT NULL
+      AND n.pub_key = $1
 )
 `
 

sqldb/sqlc/queries/graph.sql

@@ -77,7 +77,7 @@ LIMIT $3;
 SELECT EXISTS (
     SELECT 1
     FROM graph_channels c
-    JOIN graph_nodes n ON n.id = c.node_id_1 OR n.id = c.node_id_2
+    JOIN graph_nodes n ON n.id = c.node_id_1
     -- NOTE: we hard-code the version here since the clauses
     -- here that determine if a node is public is specific
     -- to the V1 gossip protocol. In V1, a node is public
@@ -89,6 +89,13 @@ SELECT EXISTS (
     WHERE c.version = 1
       AND c.bitcoin_1_signature IS NOT NULL
       AND n.pub_key = $1
+    UNION ALL
+    SELECT 1
+    FROM graph_channels c
+    JOIN graph_nodes n ON n.id = c.node_id_2
+    WHERE c.version = 1
+      AND c.bitcoin_1_signature IS NOT NULL
+      AND n.pub_key = $1
 );
 
 -- name: DeleteUnconnectedNodes :many