commit 1a41dcd2ff3401ae7c6bf321a58b54f748c04b17
parent 472f8f47423a2c014d76975ee3c17c88aa6615d9
Author: Antoine A <>
Date: Thu, 2 Jan 2025 15:00:21 +0100
bank: optimize db locking and clean SQL
Diffstat:
5 files changed, 31 insertions(+), 117 deletions(-)
diff --git a/bank/src/main/kotlin/tech/libeufin/bank/db/TransactionDAO.kt b/bank/src/main/kotlin/tech/libeufin/bank/db/TransactionDAO.kt
@@ -1,6 +1,6 @@
/*
* This file is part of LibEuFin.
- * Copyright (C) 2023-2024 Taler Systems S.A.
+ * Copyright (C) 2023-2025 Taler Systems S.A.
* LibEuFin is free software; you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as
@@ -115,7 +115,7 @@ class TransactionDAO(private val db: Database) {
} else if (exchangeCreditor) {
val bounceCause = runCatching { parseIncomingTxMetadata(subject) }.fold(
onSuccess = { metadata ->
- val registered = conn.withStatement("CALL register_incoming(?, ?::taler_incoming_type, ?, ?)") {
+ val registered = conn.withStatement("CALL register_incoming(?, ?::taler_incoming_type, ?, ?, ?)") {
setLong(1, creditRowId)
setString(2, metadata.type.name)
when (metadata.type) {
@@ -129,6 +129,7 @@ class TransactionDAO(private val db: Database) {
}
TalerIncomingType.wad -> throw UnsupportedOperationException()
}
+ setLong(5, creditAccountId)
executeProcedureViolation()
}
if (!registered) {
diff --git a/bank/src/test/kotlin/DatabaseTest.kt b/bank/src/test/kotlin/DatabaseTest.kt
@@ -1,6 +1,6 @@
/*
* This file is part of LibEuFin.
- * Copyright (C) 2023-2024 Taler Systems S.A.
+ * Copyright (C) 2023-2025 Taler Systems S.A.
* LibEuFin is free software; you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as
@@ -49,93 +49,6 @@ class DatabaseTest {
}
@Test
- fun serialisation() = bankSetup { db ->
- client.cachedToken("customer")
- client.cachedToken("merchant")
- client.cachedToken("admin")
-
- assertBalance("customer", "+KUDOS:0")
- assertBalance("merchant", "+KUDOS:0")
-
- // Generate concurrent write transactions and check that they are all successful
- coroutineScope {
- repeat(10) {
- launch {
- tx("customer", "KUDOS:0.$it", "merchant", "concurrent $it")
- }
- }
- }
- assertBalance("customer", "-KUDOS:4.5")
- assertBalance("merchant", "+KUDOS:4.5")
-
- // Generate concurrent write and read transactions and check that they only write transaction sometimes fails
- var failures = 0
- coroutineScope {
- repeat(100) {
- // Write transaction from merchant to customer
- launch {
- while (true) {
- val result = client.postA("/accounts/customer/transactions") {
- json {
- "payto_uri" to "$merchantPayto?message=${"concurrent customer 0$it".encodeURLParameter()}&amount=KUDOS:0.0$it"
- }
- }
- if (result.status == HttpStatusCode.InternalServerError) {
- val body = result.json<TalerError>()
- assertEquals(TalerErrorCode.BANK_SOFT_EXCEPTION.code, body.code)
- failures += 1
- delay(10)
- continue // retry
- } else {
- result.assertOk()
- break
- }
- }
- }
- // Write transactions from customer to merchant
- launch {
- while (true) {
- val result = client.postA("/accounts/merchant/transactions") {
- json {
- "payto_uri" to "$customerPayto?message=${"concurrent merchant 0$it".encodeURLParameter()}&amount=KUDOS:0.0$it"
- }
- }
- if (result.status == HttpStatusCode.InternalServerError) {
- val body = result.json<TalerError>()
- assertEquals(TalerErrorCode.BANK_SOFT_EXCEPTION.code, body.code)
- failures += 1
- delay(10)
- continue // retry
- } else {
- result.assertOk()
- break
- }
- }
- }
- // Simple read transaction :SELECT
- launch {
- client.getA("/accounts/merchant/transactions").assertOk()
- }
- // Complex read transaction: stored procedure
- launch {
- client.getAdmin("/monitor").assertOk()
- }
- // GC logic
- launch {
- try {
- db.gc.collect(Instant.now(), Duration.ofMinutes(1), Duration.ofMinutes(1), Duration.ofMinutes(1))
- } catch (e: Exception) {
- // Check only serialization exception
- }
- }
- }
- }
- assert(failures > 0) { "Write operation must sometimes fail under contention" }
- assertBalance("customer", "-KUDOS:4.5")
- assertBalance("merchant", "+KUDOS:4.5")
- }
-
- @Test
fun tanChallenge() = bankSetup { db -> db.conn { conn ->
val createStmt = conn.prepareStatement("SELECT tan_challenge_create('','account_reconfig'::op_enum,?,?,?,?,'customer',NULL,NULL)")
val markSentStmt = conn.prepareStatement("SELECT tan_challenge_mark_sent(?,?,?)")
diff --git a/common/src/main/kotlin/Constants.kt b/common/src/main/kotlin/Constants.kt
@@ -1,6 +1,6 @@
/*
* This file is part of LibEuFin.
- * Copyright (C) 2024 Taler Systems S.A.
+ * Copyright (C) 2024-2025 Taler Systems S.A.
* LibEuFin is free software; you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as
@@ -20,7 +20,7 @@ package tech.libeufin.common
// DB
const val MIN_VERSION: Int = 14
-const val SERIALIZATION_RETRY: Int = 10
+const val SERIALIZATION_RETRY: Int = 30
// Security
const val MAX_BODY_LENGTH: Int = 4 * 1024 // 4kB
diff --git a/common/src/main/kotlin/db/transaction.kt b/common/src/main/kotlin/db/transaction.kt
@@ -1,6 +1,6 @@
/*
* This file is part of LibEuFin.
- * Copyright (C) 2024 Taler Systems S.A.
+ * Copyright (C) 2025 Taler Systems S.A.
*
* LibEuFin is free software; you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as
@@ -31,7 +31,7 @@ import java.sql.SQLException
internal val logger: Logger = LoggerFactory.getLogger("libeufin-db")
val SERIALIZATION_ERROR = setOf(
- "40001", //serialization_failure
+ "40001", // serialization_failure
"40P01", // deadlock_detected
"55P03", // lock_not_available
)
diff --git a/database-versioning/libeufin-bank-procedures.sql b/database-versioning/libeufin-bank-procedures.sql
@@ -288,12 +288,12 @@ IF has_fee THEN
admin_payto, admin_name
FROM bank_accounts
JOIN customers ON customer_id=owning_customer_id
- WHERE username = 'admin'
- FOR UPDATE;
+ WHERE username = 'admin';
IF NOT FOUND THEN
RAISE EXCEPTION 'No admin';
END IF;
END IF;
+
-- Retrieve debtor info
SELECT
has_debt,
@@ -307,8 +307,7 @@ SELECT
debtor_payto, debtor_name
FROM bank_accounts
JOIN customers ON customer_id=owning_customer_id
- WHERE bank_account_id=in_debtor_account_id
- FOR UPDATE;
+ WHERE bank_account_id=in_debtor_account_id;
IF NOT FOUND THEN
RAISE EXCEPTION 'Unknown debtor %', in_debtor_account_id;
END IF;
@@ -323,8 +322,7 @@ SELECT
creditor_payto, creditor_name
FROM bank_accounts
JOIN customers ON customer_id=owning_customer_id
- WHERE bank_account_id=in_creditor_account_id
- FOR UPDATE NOWAIT;
+ WHERE bank_account_id=in_creditor_account_id;
IF NOT FOUND THEN
RAISE EXCEPTION 'Unknown creditor %', in_creditor_account_id;
END IF;
@@ -440,6 +438,12 @@ IF has_fee THEN
END IF;
END IF;
+-- Lock account in order to prevent deadlocks
+PERFORM FROM bank_accounts
+ WHERE bank_account_id IN (in_debtor_account_id, in_creditor_account_id, admin_account_id)
+ ORDER BY bank_account_id
+ FOR UPDATE;
+
-- now actually create the bank transaction.
-- debtor side:
INSERT INTO bank_account_transactions (
@@ -571,8 +575,7 @@ SELECT
,out_balance_not_zero
FROM customers
JOIN bank_accounts ON owning_customer_id = customer_id
- WHERE username = in_username AND deleted_at IS NULL
- FOR UPDATE;
+ WHERE username = in_username AND deleted_at IS NULL;
IF NOT FOUND OR out_balance_not_zero OR out_tan_required THEN
out_not_found=NOT FOUND;
RETURN;
@@ -587,12 +590,12 @@ CREATE PROCEDURE register_incoming(
IN in_tx_row_id INT8,
IN in_type taler_incoming_type,
IN in_reserve_pub BYTEA,
- IN in_account_pub BYTEA
+ IN in_account_pub BYTEA,
+ IN in_account_id INT8
)
LANGUAGE plpgsql AS $$
DECLARE
local_amount taler_amount;
-local_bank_account_id INT8;
BEGIN
-- Register incoming transaction
INSERT
@@ -607,16 +610,15 @@ INSERT
in_tx_row_id,
in_type
);
--- Get bank transaction info
-SELECT (amount).val, (amount).frac, bank_account_id
-INTO local_amount.val, local_amount.frac, local_bank_account_id
-FROM bank_account_transactions WHERE bank_transaction_id=in_tx_row_id;
-- Update stats
IF in_type = 'reserve' THEN
+ SELECT (amount).val, (amount).frac
+ INTO local_amount.val, local_amount.frac
+ FROM bank_account_transactions WHERE bank_transaction_id=in_tx_row_id;
CALL stats_register_payment('taler_in', NULL, local_amount, null);
END IF;
-- Notify new incoming transaction
-PERFORM pg_notify('bank_incoming_tx', local_bank_account_id || ' ' || in_tx_row_id);
+PERFORM pg_notify('bank_incoming_tx', in_account_id || ' ' || in_tx_row_id);
END $$;
COMMENT ON PROCEDURE register_incoming
IS 'Register a bank transaction as a taler incoming transaction and announce it';
@@ -844,8 +846,8 @@ IF out_debitor_balance_insufficient THEN
END IF;
-- Register incoming transaction
CASE in_type
- WHEN 'reserve' THEN CALL register_incoming(out_tx_row_id, 'reserve', in_key, NULL);
- WHEN 'kyc' THEN CALL register_incoming(out_tx_row_id, 'kyc', NULL, in_key);
+ WHEN 'reserve' THEN CALL register_incoming(out_tx_row_id, 'reserve', in_key, NULL, exchange_bank_account_id);
+ WHEN 'kyc' THEN CALL register_incoming(out_tx_row_id, 'kyc', NULL, in_key, exchange_bank_account_id);
ELSE RAISE EXCEPTION 'Unsupported incoming type %', in_type;
END CASE;
END $$;
@@ -1045,8 +1047,7 @@ SELECT
wallet_bank_account
INTO not_selected, out_status, out_already_selected, out_amount_differs, account_id
FROM taler_withdrawal_operations
- WHERE withdrawal_uuid=in_withdrawal_uuid
- FOR UPDATE;
+ WHERE withdrawal_uuid=in_withdrawal_uuid;
IF NOT FOUND OR out_already_selected OR out_amount_differs THEN
out_no_op=NOT FOUND;
RETURN;
@@ -1181,8 +1182,7 @@ SELECT
FROM taler_withdrawal_operations
JOIN bank_accounts ON wallet_bank_account=bank_account_id
JOIN customers ON owning_customer_id=customer_id
- WHERE withdrawal_uuid=in_withdrawal_uuid AND username=in_username AND deleted_at IS NULL
- FOR UPDATE;
+ WHERE withdrawal_uuid=in_withdrawal_uuid AND username=in_username AND deleted_at IS NULL;
out_no_op=NOT FOUND;
IF out_no_op OR already_confirmed OR out_aborted OR out_not_selected OR out_missing_amount OR out_amount_differs THEN
RETURN;
@@ -1226,7 +1226,7 @@ UPDATE taler_withdrawal_operations
WHERE withdrawal_uuid=in_withdrawal_uuid;
-- Register incoming transaction
-CALL register_incoming(tx_row_id, 'reserve'::taler_incoming_type, reserve_pub_local, NULL);
+CALL register_incoming(tx_row_id, 'reserve'::taler_incoming_type, reserve_pub_local, NULL, exchange_bank_account_id);
-- Notify status change
PERFORM pg_notify('bank_withdrawal_status', in_withdrawal_uuid::text || ' confirmed');
@@ -1304,7 +1304,7 @@ IF out_balance_insufficient THEN
END IF;
-- Register incoming transaction
-CALL register_incoming(tx_row_id, 'reserve'::taler_incoming_type, in_reserve_pub, NULL);
+CALL register_incoming(tx_row_id, 'reserve'::taler_incoming_type, in_reserve_pub, NULL, exchange_account_id);
-- update stats
CALL stats_register_payment('cashin', NULL, converted_amount, in_amount);
