Reapply 8644 on 9260

.github/workflows/main.yml

@@ -23,7 +23,14 @@ defaults:
 env:
   BITCOIN_VERSION: "28"
 
-  TRANCHES: 8
+  # TRANCHES defines the number of tranches used in the itests.
+  TRANCHES: 16
+
+  # SMALL_TRANCHES defines the number of tranches used in the less stable itest
+  # builds
+  #
+  # TODO(yy): remove this value and use TRANCHES.
+  SMALL_TRANCHES: 8
 
   # If you change this please also update GO_VERSION in Makefile (then run
   # `make lint` to see where else it needs to be updated as well).
@@ -229,10 +236,10 @@ jobs:
 
 
   ########################
-  # run ubuntu integration tests
+  # run integration tests with TRANCHES
   ########################
-  ubuntu-integration-test:
-    name: run ubuntu itests
+  basic-integration-test:
+    name: basic itests
     runs-on: ubuntu-latest
     if: '!contains(github.event.pull_request.labels.*.name, ''no-itest'')'
     strategy:
@@ -246,20 +253,75 @@ jobs:
             args: backend=bitcoind cover=1
           - name: bitcoind-notxindex
             args: backend="bitcoind notxindex"
+          - name: neutrino
+            args: backend=neutrino cover=1
+    steps:
+      - name: git checkout
+        uses: actions/checkout@v3
+        with:
+          fetch-depth: 0
+
+      - name: fetch and rebase on ${{ github.base_ref }}
+        if: github.event_name == 'pull_request'
+        uses: ./.github/actions/rebase
+
+      - name: setup go ${{ env.GO_VERSION }}
+        uses: ./.github/actions/setup-go
+        with:
+          go-version: '${{ env.GO_VERSION }}'
+          key-prefix: integration-test
+
+      - name: install bitcoind
+        run: ./scripts/install_bitcoind.sh $BITCOIN_VERSION
+
+      - name: run ${{ matrix.name }}
+        run: make itest-parallel tranches=${{ env.TRANCHES }} ${{ matrix.args }} shuffleseed=${{ github.run_id }}${{ strategy.job-index }}
+
+      - name: Send coverage
+        if: ${{ contains(matrix.args, 'cover=1') }}
+        uses: shogo82148/actions-goveralls@v1
+        with:
+          path-to-profile: coverage.txt
+          flag-name: 'itest-${{ matrix.name }}'
+          parallel: true
+
+      - name: Zip log files on failure
+        if: ${{ failure() }}
+        timeout-minutes: 5 # timeout after 5 minute
+        run: 7z a logs-itest-${{ matrix.name }}.zip itest/**/*.log
+
+      - name: Upload log files on failure
+        uses: actions/upload-artifact@v3
+        if: ${{ failure() }}
+        with:
+          name: logs-itest-${{ matrix.name }}
+          path: logs-itest-${{ matrix.name }}.zip
+          retention-days: 5
+
+  ########################
+  # run integration tests with SMALL_TRANCHES
+  ########################
+  integration-test:
+    name: itests
+    runs-on: ubuntu-latest
+    if: '!contains(github.event.pull_request.labels.*.name, ''no-itest'')'
+    strategy:
+      # Allow other tests in the matrix to continue if one fails.
+      fail-fast: false
+      matrix:
+        include:
           - name: bitcoind-rpcpolling
-            args: backend="bitcoind rpcpolling" cover=1
+            args: backend="bitcoind rpcpolling"
           - name: bitcoind-etcd
             args: backend=bitcoind dbbackend=etcd
-          - name: bitcoind-postgres
-            args: backend=bitcoind dbbackend=postgres
           - name: bitcoind-sqlite
             args: backend=bitcoind dbbackend=sqlite
-          - name: bitcoind-postgres-nativesql
-            args: backend=bitcoind dbbackend=postgres nativesql=true
           - name: bitcoind-sqlite-nativesql
             args: backend=bitcoind dbbackend=sqlite nativesql=true
-          - name: neutrino
-            args: backend=neutrino cover=1
+          - name: bitcoind-postgres
+            args: backend=bitcoind dbbackend=postgres
+          - name: bitcoind-postgres-nativesql
+            args: backend=bitcoind dbbackend=postgres nativesql=true
     steps:
       - name: git checkout
         uses: actions/checkout@v3
@@ -280,7 +342,7 @@ jobs:
         run: ./scripts/install_bitcoind.sh $BITCOIN_VERSION
 
       - name: run ${{ matrix.name }}
-        run: make itest-parallel tranches=${{ env.TRANCHES }} ${{ matrix.args }}
+        run: make itest-parallel tranches=${{ env.SMALL_TRANCHES }} ${{ matrix.args }} shuffleseed=${{ github.run_id }}${{ strategy.job-index }}
 
       - name: Send coverage
         if: ${{ contains(matrix.args, 'cover=1') }}
@@ -293,7 +355,7 @@ jobs:
       - name: Zip log files on failure
         if: ${{ failure() }}
         timeout-minutes: 5 # timeout after 5 minute
-        run: 7z a logs-itest-${{ matrix.name }}.zip itest/**/*.log
+        run: 7z a logs-itest-${{ matrix.name }}.zip itest/**/*.log itest/postgres.log
 
       - name: Upload log files on failure
         uses: actions/upload-artifact@v3
@@ -308,7 +370,7 @@ jobs:
   # run windows integration test
   ########################
   windows-integration-test:
-    name: run windows itest
+    name: windows itest
     runs-on: windows-latest
     if: '!contains(github.event.pull_request.labels.*.name, ''no-itest'')'
     steps:
@@ -328,7 +390,7 @@ jobs:
           key-prefix: integration-test
 
       - name: run itest
-        run: make itest-parallel tranches=${{ env.TRANCHES }} windows=1
+        run: make itest-parallel tranches=${{ env.SMALL_TRANCHES }} windows=1 shuffleseed=${{ github.run_id }}
 
       - name: kill any remaining lnd processes
         if: ${{ failure() }}
@@ -352,7 +414,7 @@ jobs:
   # run macOS integration test
   ########################
   macos-integration-test:
-    name: run macOS itest
+    name: macOS itest
     runs-on: macos-14
     if: '!contains(github.event.pull_request.labels.*.name, ''no-itest'')'
     steps:
@@ -371,14 +433,8 @@ jobs:
           go-version: '${{ env.GO_VERSION }}'
           key-prefix: integration-test
 
-      - name: install bitcoind
-        run: |
-          wget https://bitcoincore.org/bin/bitcoin-core-${BITCOIN_VERSION}.0/bitcoin-${BITCOIN_VERSION}.0-arm64-apple-darwin.tar.gz
-          tar zxvf bitcoin-${BITCOIN_VERSION}.0-arm64-apple-darwin.tar.gz
-          mv bitcoin-${BITCOIN_VERSION}.0 /tmp/bitcoin
-
       - name: run itest
-        run: PATH=$PATH:/tmp/bitcoin/bin make itest-parallel tranches=${{ env.TRANCHES }} backend=bitcoind
+        run: make itest-parallel tranches=${{ env.SMALL_TRANCHES }} shuffleseed=${{ github.run_id }}
 
       - name: Zip log files on failure
         if: ${{ failure() }}
@@ -431,7 +487,7 @@ jobs:
   # Notify about the completion of all coverage collecting jobs.
   finish:
     if: ${{ always() }}
-    needs: [unit-test, ubuntu-integration-test]
+    needs: [unit-test, basic-integration-test]
     runs-on: ubuntu-latest
     steps:
       - uses: shogo82148/actions-goveralls@v1

Makefile

@@ -195,20 +195,32 @@ ifeq ($(dbbackend),postgres)
 	docker rm lnd-postgres --force || echo "Starting new postgres container"
 
 	# Start a fresh postgres instance. Allow a maximum of 500 connections so
-	# that multiple lnd instances with a maximum number of connections of 50
-	# each can run concurrently.
-	docker run --name lnd-postgres -e POSTGRES_PASSWORD=postgres -p 6432:5432 -d postgres:13-alpine -N 500
-	docker logs -f lnd-postgres &
+	# that multiple lnd instances with a maximum number of connections of 20
+	# each can run concurrently. Note that many of the settings here are
+	# specifically for integration testing and are not fit for running
+	# production nodes. The increase in max connections ensures that there
+	# are enough entries allocated for the RWConflictPool to allow multiple
+	# conflicting transactions to track serialization conflicts. The
+	# increase in predicate locks and locks per transaction is to allow the
+	# queries to lock individual rows instead of entire tables, helping
+	# reduce serialization conflicts. Disabling sequential scan for small
+	# tables also helps prevent serialization conflicts by ensuring lookups
+	# lock only relevant rows in the index rather than the entire table.
+	docker run --name lnd-postgres -e POSTGRES_PASSWORD=postgres -p 6432:5432 -d postgres:13-alpine -N 1500 -c max_pred_locks_per_transaction=1024 -c max_locks_per_transaction=128 -c enable_seqscan=off
+	docker logs -f lnd-postgres >itest/postgres.log 2>&1 &
 
 	# Wait for the instance to be started.
 	sleep $(POSTGRES_START_DELAY)
 endif
 
+clean-itest-logs:
+	rm -rf itest/*.log itest/.logs-*
+
 #? itest-only: Only run integration tests without re-building binaries
-itest-only: db-instance
+itest-only: clean-itest-logs db-instance
 	@$(call print, "Running integration tests with ${backend} backend.")
-	rm -rf itest/*.log itest/.logs-*; date
-	EXEC_SUFFIX=$(EXEC_SUFFIX) scripts/itest_part.sh 0 1 $(TEST_FLAGS) $(ITEST_FLAGS) -test.v
+	date
+	EXEC_SUFFIX=$(EXEC_SUFFIX) scripts/itest_part.sh 0 1 $(SHUFFLE_SEED) $(TEST_FLAGS) $(ITEST_FLAGS) -test.v
 	$(COLLECT_ITEST_COVERAGE)
 
 #? itest: Build and run integration tests
@@ -218,10 +230,10 @@ itest: build-itest itest-only
 itest-race: build-itest-race itest-only
 
 #? itest-parallel: Build and run integration tests in parallel mode, running up to ITEST_PARALLELISM test tranches in parallel (default 4)
-itest-parallel: build-itest db-instance
+itest-parallel: clean-itest-logs build-itest db-instance
 	@$(call print, "Running tests")
-	rm -rf itest/*.log itest/.logs-*; date
-	EXEC_SUFFIX=$(EXEC_SUFFIX) scripts/itest_parallel.sh $(ITEST_PARALLELISM) $(NUM_ITEST_TRANCHES) $(TEST_FLAGS) $(ITEST_FLAGS)
+	date
+	EXEC_SUFFIX=$(EXEC_SUFFIX) scripts/itest_parallel.sh $(ITEST_PARALLELISM) $(NUM_ITEST_TRANCHES) $(SHUFFLE_SEED) $(TEST_FLAGS) $(ITEST_FLAGS)
 	$(COLLECT_ITEST_COVERAGE)
 
 #? itest-clean: Kill all running itest processes

batch/batch.go

@@ -5,6 +5,7 @@ import (
 	"sync"
 
 	"github.com/lightningnetwork/lnd/kvdb"
+	"github.com/lightningnetwork/lnd/sqldb"
 )
 
 // errSolo is a sentinel error indicating that the requester should re-run the
@@ -55,8 +56,20 @@ func (b *batch) run() {
 			for i, req := range b.reqs {
 				err := req.Update(tx)
 				if err != nil {
-					failIdx = i
-					return err
+					// If we get a serialization error, we
+					// want the underlying SQL retry
+					// mechanism to retry the entire batch.
+					// Otherwise, we can succeed in an
+					// sqldb retry and still re-execute the
+					// failing request individually.
+					dbErr := sqldb.MapSQLError(err)
+					if !sqldb.IsSerializationError(dbErr) {
+						failIdx = i
+
+						return err
+					}
+
+					return dbErr
 				}
 			}
 			return nil

batch/batch_test.go

@@ -0,0 +1,74 @@
+package batch
+
+import (
+	"errors"
+	"path/filepath"
+	"sync"
+	"testing"
+	"time"
+
+	"github.com/btcsuite/btcwallet/walletdb"
+	"github.com/lightningnetwork/lnd/kvdb"
+	"github.com/stretchr/testify/require"
+)
+
+func TestRetry(t *testing.T) {
+	dbDir := t.TempDir()
+
+	dbName := filepath.Join(dbDir, "weks.db")
+	db, err := walletdb.Create(
+		"bdb", dbName, true, kvdb.DefaultDBTimeout,
+	)
+	if err != nil {
+		t.Fatalf("unable to create walletdb: %v", err)
+	}
+	t.Cleanup(func() {
+		db.Close()
+	})
+
+	var (
+		mu     sync.Mutex
+		called int
+	)
+	sched := NewTimeScheduler(db, &mu, time.Second)
+
+	// First, we construct a request that should retry individually and
+	// execute it non-lazily. It should still return the error the second
+	// time.
+	req := &Request{
+		Update: func(tx kvdb.RwTx) error {
+			called++
+
+			return errors.New("test")
+		},
+	}
+	err = sched.Execute(req)
+
+	// Check and reset the called counter.
+	mu.Lock()
+	require.Equal(t, 2, called)
+	called = 0
+	mu.Unlock()
+
+	require.ErrorContains(t, err, "test")
+
+	// Now, we construct a request that should NOT retry because it returns
+	// a serialization error, which should cause the underlying postgres
+	// transaction to retry. Since we aren't using postgres, this will
+	// cause the transaction to not be retried at all.
+	req = &Request{
+		Update: func(tx kvdb.RwTx) error {
+			called++
+
+			return errors.New("could not serialize access")
+		},
+	}
+	err = sched.Execute(req)
+
+	// Check the called counter.
+	mu.Lock()
+	require.Equal(t, 1, called)
+	mu.Unlock()
+
+	require.ErrorContains(t, err, "could not serialize access")
+}