wallet: don't delete old htlcs when we forget a channel, do it on startup.

For old channels, this can take a while, and it stops everything.  But
we are only doing this to save space; it's not a *functional* necessity.

A quick and dirty test with 50,000 htlcs shows the htlc deletion took
450msec.  I tried adding an index, and changing it to set hstate to
HTLC_STATE_INVALID instead of deleting entries, but it still took about 350ms.

Reported-by: @michael1011
Signed-off-by: Rusty Russell <[email protected]>
Changelog-Changed: lightningd: we defer deletion of old htlcs on channel close, to avoid pausing for a long time.
Fixes: #7962

Author: rustyrussell

lightningd/lightningd.c

@@ -1371,6 +1371,9 @@ int main(int argc, char *argv[])
 	trace_span_end(ld->topology);
 
 	db_begin_transaction(ld->wallet->db);
+	trace_span_start("delete_old_htlcs", ld->wallet);
+	wallet_delete_old_htlcs(ld->wallet);
+	trace_span_end(ld->wallet);
 
 	/*~ Pull peers, channels and HTLCs from db. Needs to happen after the
 	 *  topology is initialized since some decisions rely on being able to

wallet/wallet.c

@@ -2957,6 +2957,21 @@ void wallet_channel_insert(struct wallet *w, struct channel *chan)
 	wallet_channel_save(w, chan);
 }
 
+void wallet_delete_old_htlcs(struct wallet *w)
+{
+	struct db_stmt *stmt;
+
+	/* Delete htlcs for closed channels */
+	stmt = db_prepare_v2(w->db, SQL("DELETE FROM channel_htlcs"
+					" WHERE id IN ("
+					"  SELECT ch.id"
+					"  FROM channel_htlcs AS ch"
+					"  JOIN channels AS c ON c.id = ch.channel_id"
+					"  WHERE c.state = ?);"));
+	db_bind_int(stmt, channel_state_in_db(CLOSED));
+	db_exec_prepared_v2(take(stmt));
+}
+
 void wallet_channel_close(struct wallet *w,
 			  const struct channel *chan)
 {
@@ -2968,15 +2983,9 @@ void wallet_channel_close(struct wallet *w,
 	struct db_stmt *stmt;
 	u64 new_move_id;
 
-	/* Delete entries from `channel_htlcs` */
-	stmt = db_prepare_v2(w->db, SQL("DELETE FROM channel_htlcs "
-					"WHERE channel_id=?"));
-	db_bind_u64(stmt, chan->dbid);
-	db_exec_prepared_v2(stmt);
-	/* FIXME: We don't actually tell them what was deleted! */
-	if (db_count_changes(stmt) != 0)
-		htlcs_index_deleted(w->ld, chan, db_count_changes(stmt));
-	tal_free(stmt);
+	/* The channel_htlcs table is quite large, and deleting it can take a
+	 * while.  So we do that on next restart by calling
+	 * wallet_delete_old_htlcs. */
 
 	/* Delete entries from `htlc_sigs` */
 	stmt = db_prepare_v2(w->db, SQL("DELETE FROM htlc_sigs "

wallet/wallet.h

@@ -725,6 +725,14 @@ void wallet_state_change_add(struct wallet *w,
  */
 void wallet_delete_peer_if_unused(struct wallet *w, u64 peer_dbid);
 
+/**
+ * wallet_delete_old_htlcs -- delete htlcs associated with CLOSED channels.
+ *
+ * We do this at startup, instead of when we finally CLOSED a channel, to
+ * avoid a significant pause.
+ */
+void wallet_delete_old_htlcs(struct wallet *w);
+
 /**
  * wallet_init_channels -- Loads active channels into peers
  *    and inits the dbid counter for next channel.