From ea467783e965db157479e2389955bf52955cfcc7 Mon Sep 17 00:00:00 2001
From: eugene <elzeigel@gmail.com>
Date: Wed, 19 Aug 2020 11:09:48 -0400
Subject: [PATCH] htlcswitch+peer: remove fwd pkgs once before tick, bump timer
 to 1hr

This commit changes the logic when garbage collecting forwarding
packages such that they are removed once when the function is called,
and then again upon subsequent ticks. This allows us to bump the
peer timer to 1 hour to limit the number of db transactions happening
in lnd. The forwarding packages need to be removed initially as
otherwise a flappy node will never have them garbage collected.
---
 htlcswitch/link.go | 50 ++++++++++++++++++++++++++++++----------------
 peer/brontide.go   |  2 +-
 2 files changed, 34 insertions(+), 18 deletions(-)

diff --git a/htlcswitch/link.go b/htlcswitch/link.go
index d9ee8db3..974e3cc1 100644
--- a/htlcswitch/link.go
+++ b/htlcswitch/link.go
@@ -843,35 +843,51 @@ func (l *channelLink) fwdPkgGarbager() {
 	l.cfg.FwdPkgGCTicker.Resume()
 	defer l.cfg.FwdPkgGCTicker.Stop()
 
+	if err := l.loadAndRemove(); err != nil {
+		l.log.Warnf("unable to run initial fwd pkgs gc: %v", err)
+	}
+
 	for {
 		select {
 		case <-l.cfg.FwdPkgGCTicker.Ticks():
-			fwdPkgs, err := l.channel.LoadFwdPkgs()
-			if err != nil {
-				l.log.Warnf("unable to load fwdpkgs for gc: %v",
+			if err := l.loadAndRemove(); err != nil {
+				l.log.Warnf("unable to remove fwd pkgs: %v",
 					err)
 				continue
 			}
-
-			// TODO(conner): batch removal of forward packages.
-			for _, fwdPkg := range fwdPkgs {
-				if fwdPkg.State != channeldb.FwdStateCompleted {
-					continue
-				}
-
-				err = l.channel.RemoveFwdPkgs(fwdPkg.Height)
-				if err != nil {
-					l.log.Warnf("unable to remove fwd pkg "+
-						"for height=%d: %v",
-						fwdPkg.Height, err)
-				}
-			}
 		case <-l.quit:
 			return
 		}
 	}
 }
 
+// loadAndRemove loads all the channels forwarding packages and determines if
+// they can be removed. It is called once before the FwdPkgGCTicker ticks so that
+// a longer tick interval can be used.
+func (l *channelLink) loadAndRemove() error {
+	fwdPkgs, err := l.channel.LoadFwdPkgs()
+	if err != nil {
+		return err
+	}
+
+	var removeHeights []uint64
+	for _, fwdPkg := range fwdPkgs {
+		if fwdPkg.State != channeldb.FwdStateCompleted {
+			continue
+		}
+
+		removeHeights = append(removeHeights, fwdPkg.Height)
+	}
+
+	// If removeHeights is empty, return early so we don't use a db
+	// transaction.
+	if len(removeHeights) == 0 {
+		return nil
+	}
+
+	return l.channel.RemoveFwdPkgs(removeHeights...)
+}
+
 // htlcManager is the primary goroutine which drives a channel's commitment
 // update state-machine in response to messages received via several channels.
 // This goroutine reads messages from the upstream (remote) peer, and also from
diff --git a/peer/brontide.go b/peer/brontide.go
index e1ecaf88..cc637207 100644
--- a/peer/brontide.go
+++ b/peer/brontide.go
@@ -588,7 +588,7 @@ func (p *Brontide) addLink(chanPoint *wire.OutPoint,
 		OnChannelFailure:        onChannelFailure,
 		SyncStates:              syncStates,
 		BatchTicker:             ticker.New(50 * time.Millisecond),
-		FwdPkgGCTicker:          ticker.New(time.Minute),
+		FwdPkgGCTicker:          ticker.New(time.Hour),
 		PendingCommitTicker:     ticker.New(time.Minute),
 		BatchSize:               10,
 		UnsafeReplay:            p.cfg.UnsafeReplay,