From 08f4940e881186fdc93fca71d56a18caef73f1fb Mon Sep 17 00:00:00 2001 From: yyforyongyu Date: Mon, 4 Sep 2023 15:35:17 +0800 Subject: [PATCH 1/4] kvdb/sqlbase: fix params used in `randRetryDelay` --- kvdb/sqlbase/db.go | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/kvdb/sqlbase/db.go b/kvdb/sqlbase/db.go index ad0634ae2..7defed2d7 100644 --- a/kvdb/sqlbase/db.go +++ b/kvdb/sqlbase/db.go @@ -234,7 +234,9 @@ func (db *db) Update(f func(tx walletdb.ReadWriteTx) error, // randRetryDelay returns a random retry delay between -50% and +50% of the // configured delay that is doubled for each attempt and capped at a max value. -func randRetryDelay(initialRetryDelay, maxRetryDelay, attempt int) time.Duration { +func randRetryDelay(initialRetryDelay, maxRetryDelay time.Duration, + attempt int) time.Duration { + halfDelay := initialRetryDelay / 2 randDelay := prand.Int63n(int64(initialRetryDelay)) //nolint:gosec @@ -273,8 +275,8 @@ func (db *db) executeTransaction(f func(tx walletdb.ReadWriteTx) error, // should abort the retries. waitBeforeRetry := func(attemptNumber int) bool { retryDelay := randRetryDelay( - attemptNumber, DefaultInitialRetryDelay, - DefaultMaxRetryDelay, + DefaultInitialRetryDelay, DefaultMaxRetryDelay, + attemptNumber, ) log.Debugf("Retrying transaction due to tx serialization "+ From 5296509474080c7cbb0fe6ab2a0718bed7784188 Mon Sep 17 00:00:00 2001 From: yyforyongyu Date: Mon, 4 Sep 2023 19:17:53 +0800 Subject: [PATCH 2/4] Makefile+action: add `make unit-module` to test submodules This commit adds a new command `make unit-module` to run unit tests for submodules to avoid future build errors. --- .github/workflows/main.yml | 3 +++ Makefile | 4 ++++ scripts/unit_test_modules.sh | 27 +++++++++++++++++++++++++++ 3 files changed, 34 insertions(+) create mode 100755 scripts/unit_test_modules.sh diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index c2b5280d1..fbe499466 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -186,6 +186,8 @@ jobs: - unit tags="kvdb_postgres" - unit tags="kvdb_sqlite" - btcd unit-race + - unit-module + steps: - name: git checkout uses: actions/checkout@v3 @@ -217,6 +219,7 @@ jobs: path-to-profile: coverage.txt parallel: true + ######################## # run ubuntu integration tests ######################## diff --git a/Makefile b/Makefile index 3199ca29a..c099d8a8f 100644 --- a/Makefile +++ b/Makefile @@ -184,6 +184,10 @@ unit: $(BTCD_BIN) @$(call print, "Running unit tests.") $(UNIT) +unit-module: + @$(call print, "Running submodule unit tests.") + scripts/unit_test_modules.sh + unit-debug: $(BTCD_BIN) @$(call print, "Running debug unit tests.") $(UNIT_DEBUG) diff --git a/scripts/unit_test_modules.sh b/scripts/unit_test_modules.sh new file mode 100755 index 000000000..1e6f92823 --- /dev/null +++ b/scripts/unit_test_modules.sh @@ -0,0 +1,27 @@ +#!/bin/bash + +IGNORE="tools" +SUBMODULES=$(find . -mindepth 2 -name "go.mod" | cut -d'/' -f2 | grep -v "$IGNORE") + +for submodule in $SUBMODULES +do + pushd $submodule + + echo "Running submodule unit tests in $(pwd)" + echo "testing $submodule..." + go test -timeout=5m || exit 1 + + if [[ "$submodule" == "kvdb" ]] + then + echo "testing $submodule with sqlite..." + go test -tags="kvdb_sqlite" -timeout=5m || exit 1 + + echo "testing $submodule with postgres..." + go test -tags="kvdb_postgres" -timeout=5m || exit 1 + + echo "testing $submodule with etcd..." + go test -tags="kvdb_etcd" -timeout=5m || exit 1 + fi + + popd +done From 8813bc7ba83da418d8c502dd29d0543b511e17f2 Mon Sep 17 00:00:00 2001 From: yyforyongyu Date: Mon, 4 Sep 2023 17:31:51 +0800 Subject: [PATCH 3/4] kvdb/sqlbase: fix build errors We copy the `sqldb/sqlerrors.go` into `kvdb/sqlbase` to avoid import cycles. --- kvdb/go.mod | 3 +- kvdb/go.sum | 2 + kvdb/sqlbase/db.go | 13 ++--- kvdb/sqlbase/sqlerrors.go | 115 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 123 insertions(+), 10 deletions(-) create mode 100644 kvdb/sqlbase/sqlerrors.go diff --git a/kvdb/go.mod b/kvdb/go.mod index a07c76e05..daca93119 100644 --- a/kvdb/go.mod +++ b/kvdb/go.mod @@ -6,6 +6,8 @@ require ( github.com/davecgh/go-spew v1.1.1 github.com/fergusstrange/embedded-postgres v1.10.0 github.com/google/btree v1.0.1 + github.com/jackc/pgconn v1.14.0 + github.com/jackc/pgerrcode v0.0.0-20220416144525-469b46aa5efa github.com/jackc/pgx/v4 v4.18.1 github.com/lightningnetwork/lnd/healthcheck v1.0.0 github.com/stretchr/testify v1.8.2 @@ -37,7 +39,6 @@ require ( github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0 // indirect github.com/grpc-ecosystem/grpc-gateway v1.16.0 // indirect github.com/jackc/chunkreader/v2 v2.0.1 // indirect - github.com/jackc/pgconn v1.14.0 // indirect github.com/jackc/pgio v1.0.0 // indirect github.com/jackc/pgpassfile v1.0.0 // indirect github.com/jackc/pgproto3/v2 v2.3.2 // indirect diff --git a/kvdb/go.sum b/kvdb/go.sum index 0bb9edee8..9986ec980 100644 --- a/kvdb/go.sum +++ b/kvdb/go.sum @@ -143,6 +143,8 @@ github.com/jackc/pgconn v1.9.0/go.mod h1:YctiPyvzfU11JFxoXokUOOKQXQmDMoJL9vJzHH8 github.com/jackc/pgconn v1.9.1-0.20210724152538-d89c8390a530/go.mod h1:4z2w8XhRbP1hYxkpTuBjTS3ne3J48K83+u0zoyvg2pI= github.com/jackc/pgconn v1.14.0 h1:vrbA9Ud87g6JdFWkHTJXppVce58qPIdP7N8y0Ml/A7Q= github.com/jackc/pgconn v1.14.0/go.mod h1:9mBNlny0UvkgJdCDvdVHYSjI+8tD2rnKK69Wz8ti++E= +github.com/jackc/pgerrcode v0.0.0-20220416144525-469b46aa5efa h1:s+4MhCQ6YrzisK6hFJUX53drDT4UsSW3DEhKn0ifuHw= +github.com/jackc/pgerrcode v0.0.0-20220416144525-469b46aa5efa/go.mod h1:a/s9Lp5W7n/DD0VrVoyJ00FbP2ytTPDVOivvn2bMlds= github.com/jackc/pgio v1.0.0 h1:g12B9UwVnzGhueNavwioyEEpAmqMe1E/BN9ES+8ovkE= github.com/jackc/pgio v1.0.0/go.mod h1:oP+2QK2wFfUWgr+gxjoBH9KGBb31Eio69xUb0w5bYf8= github.com/jackc/pgmock v0.0.0-20190831213851-13a1b77aafa2/go.mod h1:fGZlG77KXmcq05nJLRkk0+p82V8B8Dw8KN2/V9c/OAE= diff --git a/kvdb/sqlbase/db.go b/kvdb/sqlbase/db.go index 7defed2d7..f6f2c7590 100644 --- a/kvdb/sqlbase/db.go +++ b/kvdb/sqlbase/db.go @@ -9,6 +9,7 @@ import ( "fmt" "io" "math" + "math/rand" "sync" "time" @@ -37,12 +38,6 @@ const ( DefaultMaxRetryDelay = time.Second * 5 ) -var ( - // ErrRetriesExceeded is returned when a transaction is retried more - // than the max allowed valued without a success. - ErrRetriesExceeded = errors.New("db tx retries exceeded") -) - // Config holds a set of configuration options of a sql database connection. type Config struct { // DriverName is the string that defines the registered sql driver that @@ -238,7 +233,7 @@ func randRetryDelay(initialRetryDelay, maxRetryDelay time.Duration, attempt int) time.Duration { halfDelay := initialRetryDelay / 2 - randDelay := prand.Int63n(int64(initialRetryDelay)) //nolint:gosec + randDelay := rand.Int63n(int64(initialRetryDelay)) //nolint:gosec // 50% plus 0%-100% gives us the range of 50%-150%. initialDelay := halfDelay + time.Duration(randDelay) @@ -286,11 +281,11 @@ func (db *db) executeTransaction(f func(tx walletdb.ReadWriteTx) error, select { // Before we try again, we'll wait with a random backoff based // on the retry delay. - case time.After(retryDelay): + case <-time.After(retryDelay): return true // If the daemon is shutting down, then we'll exit early. - case <-db.Context.Done(): + case <-db.ctx.Done(): return false } } diff --git a/kvdb/sqlbase/sqlerrors.go b/kvdb/sqlbase/sqlerrors.go new file mode 100644 index 000000000..ccbb9c610 --- /dev/null +++ b/kvdb/sqlbase/sqlerrors.go @@ -0,0 +1,115 @@ +//go:build kvdb_postgres || (kvdb_sqlite && !(windows && (arm || 386)) && !(linux && (ppc64 || mips || mipsle || mips64))) + +package sqlbase + +import ( + "errors" + "fmt" + + "github.com/jackc/pgconn" + "github.com/jackc/pgerrcode" + "modernc.org/sqlite" + sqlite3 "modernc.org/sqlite/lib" +) + +var ( + // ErrRetriesExceeded is returned when a transaction is retried more + // than the max allowed valued without a success. + ErrRetriesExceeded = errors.New("db tx retries exceeded") +) + +// MapSQLError attempts to interpret a given error as a database agnostic SQL +// error. +func MapSQLError(err error) error { + // Attempt to interpret the error as a sqlite error. + var sqliteErr *sqlite.Error + if errors.As(err, &sqliteErr) { + return parseSqliteError(sqliteErr) + } + + // Attempt to interpret the error as a postgres error. + var pqErr *pgconn.PgError + if errors.As(err, &pqErr) { + return parsePostgresError(pqErr) + } + + // Return original error if it could not be classified as a database + // specific error. + return err +} + +// parsePostgresError attempts to parse a sqlite error as a database agnostic +// SQL error. +func parseSqliteError(sqliteErr *sqlite.Error) error { + switch sqliteErr.Code() { + // Handle unique constraint violation error. + case sqlite3.SQLITE_CONSTRAINT_UNIQUE: + return &ErrSQLUniqueConstraintViolation{ + DBError: sqliteErr, + } + + // Database is currently busy, so we'll need to try again. + case sqlite3.SQLITE_BUSY: + return &ErrSerializationError{ + DBError: sqliteErr, + } + + default: + return fmt.Errorf("unknown sqlite error: %w", sqliteErr) + } +} + +// parsePostgresError attempts to parse a postgres error as a database agnostic +// SQL error. +func parsePostgresError(pqErr *pgconn.PgError) error { + switch pqErr.Code { + // Handle unique constraint violation error. + case pgerrcode.UniqueViolation: + return &ErrSQLUniqueConstraintViolation{ + DBError: pqErr, + } + + // Unable to serialize the transaction, so we'll need to try again. + case pgerrcode.SerializationFailure: + return &ErrSerializationError{ + DBError: pqErr, + } + + default: + return fmt.Errorf("unknown postgres error: %w", pqErr) + } +} + +// ErrSQLUniqueConstraintViolation is an error type which represents a database +// agnostic SQL unique constraint violation. +type ErrSQLUniqueConstraintViolation struct { + DBError error +} + +func (e ErrSQLUniqueConstraintViolation) Error() string { + return fmt.Sprintf("sql unique constraint violation: %v", e.DBError) +} + +// ErrSerializationError is an error type which represents a database agnostic +// error that a transaction couldn't be serialized with other concurrent db +// transactions. +type ErrSerializationError struct { + DBError error +} + +// Unwrap returns the wrapped error. +func (e ErrSerializationError) Unwrap() error { + return e.DBError +} + +// Error returns the error message. +func (e ErrSerializationError) Error() string { + return e.DBError.Error() +} + +// IsSerializationError returns true if the given error is a serialization +// error. +func IsSerializationError(err error) bool { + var serializationError *ErrSerializationError + return errors.As(err, &serializationError) +} From c568ba9fa26f2adcbdb2456f7a0b9521d9b01308 Mon Sep 17 00:00:00 2001 From: yyforyongyu Date: Wed, 6 Sep 2023 02:17:00 +0800 Subject: [PATCH 4/4] github+Makefile: add new action `make tidy-module-check` This commit adds `tidy-module` and `tidy-module-check` to make sure the modules are always tidy. --- .github/workflows/main.yml | 3 +++ Makefile | 7 +++++++ scripts/tidy_modules.sh | 17 +++++++++++++++++ 3 files changed, 27 insertions(+) create mode 100755 scripts/tidy_modules.sh diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index fbe499466..66125fbb4 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -131,6 +131,9 @@ jobs: - name: check code format run: make fmt-check + - name: check go modules tidiness + run: make tidy-module-check + - name: lint run: GOGC=50 make lint diff --git a/Makefile b/Makefile index c099d8a8f..e31c0adde 100644 --- a/Makefile +++ b/Makefile @@ -246,6 +246,13 @@ lint: docker-tools @$(call print, "Linting source.") $(DOCKER_TOOLS) golangci-lint run -v $(LINT_WORKERS) +tidy-module: + echo "Running 'go mod tidy' for all modules" + scripts/tidy_modules.sh + +tidy-module-check: tidy-module + if test -n "$$(git status --porcelain)"; then echo "modules not updated, please run `make tidy-module` again!"; git status; exit 1; fi + list: @$(call print, "Listing commands.") @$(MAKE) -qp | \ diff --git a/scripts/tidy_modules.sh b/scripts/tidy_modules.sh new file mode 100755 index 000000000..3fa5bfb25 --- /dev/null +++ b/scripts/tidy_modules.sh @@ -0,0 +1,17 @@ +#!/bin/bash + +SUBMODULES=$(find . -mindepth 2 -name "go.mod" | cut -d'/' -f2) + + +# Run 'go mod tidy' for root. +go mod tidy + +# Run 'go mod tidy' for each module. +for submodule in $SUBMODULES +do + pushd $submodule + + go mod tidy + + popd +done