channeld: remove dead HTLCs from htable and free them (eventually)

The channel->htlcs map was exhibiting unbounded growth, as elements were
never removed from it. This was causing lightning_channeld processes to
consume ever-increasing amounts of memory, and iterating over the map
was causing ever-increasing CPU utilization. There were FIXME comments
suggesting that the intention was to remove HTLCs from the map upon
their deaths. This commit implements that intention.

Changelog-Fixed: channeld no longer retains dead HTLCs in memory.
This commit is contained in:
Matt Whitlock 2023-01-05 21:34:56 -05:00 committed by Alex Myers
parent 82c94330a5
commit acfb63e4bf
3 changed files with 21 additions and 12 deletions

View file

@ -72,7 +72,6 @@ static inline struct htlc *htlc_get(struct htlc_map *htlcs, u64 id, enum side ow
return NULL;
}
/* FIXME: Move these out of the hash! */
static inline bool htlc_is_dead(const struct htlc *htlc)
{
return htlc->state == RCVD_REMOVE_ACK_REVOCATION

View file

@ -1110,10 +1110,15 @@ static int change_htlcs(struct channel *channel,
for (i = 0; i < n_hstates; i++) {
if (h->state == htlc_states[i]) {
htlc_incstate(channel, h, sidechanged, owed);
if (htlc_is_dead(h)) {
htlc_map_delval(channel->htlcs, &it);
tal_steal(htlcs ? *htlcs : tmpctx, h);
}
dump_htlc(h, prefix);
htlc_arr_append(htlcs, h);
cflags |= (htlc_state_flags(htlc_states[i])
^ htlc_state_flags(h->state));
break;
}
}
}
@ -1391,20 +1396,24 @@ bool channel_sending_revoke_and_ack(struct channel *channel)
return (change & HTLC_REMOTE_F_PENDING);
}
size_t num_channel_htlcs(const struct channel *channel)
static inline bool any_htlc_is_dead(const struct channel *channel)
{
struct htlc_map_iter it;
const struct htlc *htlc;
size_t n = 0;
for (htlc = htlc_map_first(channel->htlcs, &it);
htlc;
htlc = htlc_map_next(channel->htlcs, &it)) {
/* FIXME: Clean these out! */
if (!htlc_is_dead(htlc))
n++;
if (htlc_is_dead(htlc))
return true;
}
return n;
return false;
}
size_t num_channel_htlcs(const struct channel *channel)
{
assert(!any_htlc_is_dead(channel));
return htlc_map_count(channel->htlcs);
}
static bool adjust_balance(struct balance view_owed[NUM_SIDES][NUM_SIDES],

View file

@ -254,7 +254,7 @@ static void send_and_fulfill_htlc(struct channel *channel,
struct sha256 rhash;
u8 *dummy_routing = tal_arr(channel, u8, TOTAL_PACKET_SIZE(ROUTING_INFO_SIZE));
bool ret;
const struct htlc **changed_htlcs;
const struct htlc *htlc, **changed_htlcs;
memset(&r, 0, sizeof(r));
sha256(&rhash, &r, sizeof(r));
@ -262,6 +262,8 @@ static void send_and_fulfill_htlc(struct channel *channel,
assert(channel_add_htlc(channel, sender, 1337, msatoshi, 900, &rhash,
dummy_routing, NULL, NULL, NULL, true)
== CHANNEL_ERR_ADD_OK);
htlc = channel_get_htlc(channel, sender, 1337);
assert(htlc);
changed_htlcs = tal_arr(channel, const struct htlc *, 0);
@ -285,8 +287,7 @@ static void send_and_fulfill_htlc(struct channel *channel,
assert(ret);
ret = channel_rcvd_revoke_and_ack(channel, &changed_htlcs);
assert(!ret);
assert(channel_get_htlc(channel, sender, 1337)->state
== RCVD_REMOVE_ACK_REVOCATION);
assert(htlc->state == RCVD_REMOVE_ACK_REVOCATION);
} else {
ret = channel_rcvd_commit(channel, &changed_htlcs);
assert(ret);
@ -306,9 +307,9 @@ static void send_and_fulfill_htlc(struct channel *channel,
assert(ret);
ret = channel_sending_revoke_and_ack(channel);
assert(!ret);
assert(channel_get_htlc(channel, sender, 1337)->state
== SENT_REMOVE_ACK_REVOCATION);
assert(htlc->state == SENT_REMOVE_ACK_REVOCATION);
}
assert(!channel_get_htlc(channel, sender, 1337));
}
static void update_feerate(struct channel *channel, u32 feerate)