hyperledger / aries-askar

Secure storage designed for Hyperledger Aries agents.
Apache License 2.0
58 stars 42 forks source link

`for_update` does not seem to lock until the transaction is completed #241

Closed berendsliedrecht closed 2 months ago

berendsliedrecht commented 2 months ago

We have a setup where postgres is a separate docker instance and we have N agents using this database. Because of this, when updating a row, we must lock the row until it is explicitly released. Looking at the code, this is this main purpose of the for_update property. Browsing through the code, it seems like the query is extended with FOR NO KEY UPDATE to make sure the row stays locked on a DB level until the transaction is committed. However, when testing this it seems like this is not the case. I hope I just made a mistake in my test code..

# Cargo.toml
[package]
name = "askar-test-update-lock"
version = "0.1.0"
edition = "2021"

[dependencies]
aries-askar "0.3.1"
tokio = { version = "1.37.0", features = ["full"] }
// src/main.rs
use aries_askar::storage::generate_raw_store_key;
use aries_askar::{storage::postgres::TestDB, PassKey, Store};

#[tokio::main]
async fn main() {
    // Generate the key
    let key = generate_raw_store_key(Some(&[0u8; 32])).unwrap();

    // Provision a store
    let store = Store::provision(
        "postgres://postgres:mysecretpassword@localhost:5432/test-db",
        aries_askar::StoreKeyMethod::RawKey,
        key,
        None,
        true,
    )
    .await
    .unwrap();

    // Start a transaction
    let mut sesh = store.transaction(None).await.unwrap();

    // Insert a row
    sesh.insert("category", "name", b"value", None, None)
        .await
        .unwrap();

    // Commit the insert
    sesh.commit().await.unwrap();

    // Start a new transaction
    let mut sesh = store.transaction(None).await.unwrap();

    // fetch the row once
    let entry = sesh.fetch("category", "name", true).await.unwrap().unwrap();
    println!(
        "value: {:?}",
        String::from_utf8_lossy(&entry.value.to_vec())
    );

    // fetch the row again
    // should fail right as we did not commit and `for_update` is true
    let entry_two = sesh.fetch("category", "name", true).await.unwrap().unwrap();

    println!(
        "value: {:?}",
        String::from_utf8_lossy(&entry_two.value.to_vec())
    );

    sesh.commit().await.unwrap();
}
# launch postgres command
docker run --rm -p 5432:5432 --name aries-test-postgres -e POSTGRES_PASSWORD=mysecretpassword -d postgres
swcurran commented 2 months ago

@andrewwhitehead — can you please take a look at this? Looks like an troublesome issue, if real.

andrewwhitehead commented 2 months ago

Hi @berendsliedrecht, just tested this out and I don't think there's an issue. In this test case the row is being fetched twice for update, but within the same transaction, so there is no conflict. If you open another transaction and try to fetch the same row for update, then it will block until the first transaction is completed (or time out).

berendsliedrecht commented 2 months ago

Ah ofcourse, that makes complete sense. Glad my test code was incorrect. I just checked it with two different transactions and it does lock correctly, thanks!