gossipd: don't keep node_announcement messages in memory.

MCP results from 5 runs, min-max(mean +/- stddev):
store_load_msec:34779-38628(36903.4+/-1.4e+03)
vsz_kb:1792996
store_rewrite_sec:14.440000-15.040000(14.672+/-0.24)
listnodes_sec:1.030000-1.120000(1.068+/-0.032)
listchannels_sec:27.860000-32.850000(30.05+/-1.7)
routing_sec:30.020000-31.700000(31.044+/-0.56)
peer_write_all_sec:65.100000-70.600000(68.422+/-2)

-vsz_kb:1780516
+vsz_kb:1792996
-listnodes_sec:1.280000-1.530000(1.382+/-0.096)
+listnodes_sec:1.030000-1.120000(1.068+/-0.032)

MCP notable changes from previous patch (>1 stddev):
	-store_load_msec:30640-33236(32202+/-8.7e+02)
	+store_load_msec:34779-38628(36903.4+/-1.4e+03)
	-vsz_kb:1812956
	+vsz_kb:1792996
	-listnodes_sec:0.590000-0.660000(0.62+/-0.033)
	+listnodes_sec:1.030000-1.120000(1.068+/-0.032)
	-peer_write_all_sec:60.380000-61.320000(60.836+/-0.37)
	+peer_write_all_sec:65.100000-70.600000(68.422+/-2)

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

gossipd/gossipd.c

@@ -235,6 +235,17 @@ static void queue_peer_msg(struct peer *peer, const u8 *msg TAKES)
 	daemon_conn_send(peer->dc, take(send));
 }
 
+/* Load a message from the gossip_store, and queue to send. */
+static void queue_peer_from_store(struct peer *peer,
+				  const struct broadcastable *bcast)
+{
+	const u8 *msg;
+
+	msg = gossip_store_get(NULL, peer->daemon->rstate->broadcasts->gs,
+			       bcast->index);
+	queue_peer_msg(peer, take(msg));
+}
+
 /* This pokes daemon_conn, which calls dump_gossip: the NULL gossip_timer
  * tells it that the gossip timer has expired and it should send any queued
  * gossip messages. */
@@ -435,20 +446,25 @@ static bool node_announcement_redundant(struct daemon *daemon)
 	u8 *features, *addresses;
 	struct wireaddr *wireaddrs;
 	struct node *n = get_node(daemon->rstate, &daemon->id);
+	const u8 *msg;
 
 	if (!n)
 		return false;
 
-	if (!n->node_announcement)
+	if (!n->bcast.index)
 		return false;
 
+	msg = gossip_store_get(tmpctx, daemon->rstate->broadcasts->gs,
+			       n->bcast.index);
+
 	/* Note: validity of node_id is already checked. */
-	if (!fromwire_node_announcement(tmpctx, n->node_announcement,
-					&signature, &features, &timestamp,
+	if (!fromwire_node_announcement(tmpctx, msg,
+					&signature, &features,
+					&timestamp,
 					&node_id, rgb_color, alias,
 					&addresses)) {
-		status_broken("Bad local node_announcement: %s",
-			      tal_hex(tmpctx, n->node_announcement));
+		status_broken("Bad local node_announcement @%u: %s",
+			      n->bcast.index, tal_hex(tmpctx, msg));
 		return false;
 	}
 
@@ -1096,7 +1112,7 @@ static void maybe_create_next_scid_reply(struct peer *peer)
 		if (!n || !n->bcast.index)
 			continue;
 
-		queue_peer_msg(peer, n->node_announcement);
+		queue_peer_from_store(peer, &n->bcast);
 		sent = true;
 	}
 	peer->scid_query_nodes_idx = i;
@@ -2077,7 +2093,7 @@ static void append_node(const struct gossip_getnodes_entry ***entries,
 	e->nodeid = n->id;
 	/* Timestamp on wire is an unsigned 32 bit: we use a 64-bit signed, so
 	 * -1 means "we never received a channel_update". */
-	if (!n->node_announcement)
+	if (!n->bcast.index)
 		e->last_timestamp = -1;
 	else {
 		e->last_timestamp = n->bcast.timestamp;

gossipd/routing.c

@@ -264,7 +264,6 @@ static struct node *new_node(struct routing_state *rstate,
 	n->id = *id;
 	memset(n->chans.arr, 0, sizeof(n->chans.arr));
 	n->globalfeatures = NULL;
-	n->node_announcement = NULL;
 	broadcastable_init(&n->bcast);
 	n->addresses = tal_arr(n, struct wireaddr, 0);
 	node_map_add(rstate->nodes, n);
@@ -352,14 +351,17 @@ static void remove_chan_from_node(struct routing_state *rstate,
 	if (!node_has_broadcastable_channels(node)) {
 		broadcast_del(rstate->broadcasts, &node->bcast);
 	} else if (node_announce_predates_channels(node)) {
+		const u8 *announce;
+
+		announce = gossip_store_get(tmpctx, rstate->broadcasts->gs,
+					    node->bcast.index);
+
 		/* node announcement predates all channel announcements?
 		 * Move to end (we could, in theory, move to just past next
 		 * channel_announce, but we don't care that much about spurious
 		 * retransmissions in this corner case */
 		broadcast_del(rstate->broadcasts, &node->bcast);
-		insert_broadcast(&rstate->broadcasts,
-				 node->node_announcement,
-				 &node->bcast);
+		insert_broadcast(&rstate->broadcasts, announce, &node->bcast);
 	}
 }
 
@@ -1636,20 +1638,21 @@ bool routing_add_node_announcement(struct routing_state *rstate,
 	if (!fromwire_node_announcement(tmpctx, msg,
 					&signature, &features, &timestamp,
 					&node_id, rgb_color, alias,
-					&addresses))
+					&addresses)) {
 		return false;
+	}
 
 	node = get_node(rstate, &node_id);
 
 	/* May happen if we accepted the node_announcement due to a local
 	 * channel, for which we didn't have the announcement yet. */
-	if (node == NULL) {
-		if (taken(msg))
-			tal_free(msg);
+	if (node == NULL)
+		return false;
+
+	/* Shouldn't get here, but gossip_store bugs are possible. */
+	if (!node_has_broadcastable_channels(node))
 		return false;
-	}
 
-	tal_free(node->node_announcement);
 	/* Harmless if it was never added */
 	broadcast_del(rstate->broadcasts, &node->bcast);
 
@@ -1664,14 +1667,7 @@ bool routing_add_node_announcement(struct routing_state *rstate,
 	tal_free(node->globalfeatures);
 	node->globalfeatures = tal_steal(node, features);
 
-	node->node_announcement = tal_dup_arr(node, u8, msg, tal_count(msg), 0);
-
-	/* We might be waiting for channel_announce to be released. */
-	if (node_has_broadcastable_channels(node)) {
-		insert_broadcast(&rstate->broadcasts,
-				 node->node_announcement,
-				 &node->bcast);
-	}
+	insert_broadcast(&rstate->broadcasts, msg, &node->bcast);
 	return true;
 }
 

gossipd/routing.h

@@ -145,9 +145,6 @@ struct node {
 
 	/* (Global) features */
 	u8 *globalfeatures;
-
-	/* Cached `node_announcement` we might forward to new peers (or NULL). */
-	const u8 *node_announcement;
 };
 
 const struct node_id *node_map_keyof_node(const struct node *n);