Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

TxnLockNotFound #315

Open
gfreezy opened this issue Aug 17, 2021 · 10 comments · May be fixed by #316
Open

TxnLockNotFound #315

gfreezy opened this issue Aug 17, 2021 · 10 comments · May be fixed by #316
Labels
bug Something isn't working good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines.

Comments

@gfreezy
Copy link
Contributor

gfreezy commented Aug 17, 2021 

#[tokio::main]
async fn main() -> Result<()> {
    let txn_client = TransactionClient::new(vec!["127.0.0.1:2379"]).await?;
    let mut txn = txn_client.begin_optimistic().await.unwrap();
    let key = "aa".to_owned();
    txn.insert(key.clone(), "value".to_owned()).await.unwrap();
    txn.delete(key.clone()).await.unwrap();
    txn.commit().await.unwrap();
    Ok(())
}

The above code raise the following error:

thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: StringError("KeyError { locked: None, retryable: \"Error(Txn(Error(Mvcc(Error(TxnLockNotFound { start_ts: TimeStamp(427084398190919681), commit_ts: TimeStamp(427084398190919682), key: [97, 97] })))))\", abort: \"\", conflict: None, already_exist: None, deadlock: None, commit_ts_expired: None, txn_not_found: None, commit_ts_too_large: None }")', src/main.rs:347:24
stack backtrace:
   0: rust_begin_unwind
             at /rustc/ae90dcf0207c57c3034f00b07048d63f8b2363c8/library/std/src/panicking.rs:517:5
   1: core::panicking::panic_fmt
             at /rustc/ae90dcf0207c57c3034f00b07048d63f8b2363c8/library/core/src/panicking.rs:93:14
   2: core::result::unwrap_failed
             at /rustc/ae90dcf0207c57c3034f00b07048d63f8b2363c8/library/core/src/result.rs:1617:5
   3: core::result::Result<T,E>::unwrap
             at /rustc/ae90dcf0207c57c3034f00b07048d63f8b2363c8/library/core/src/result.rs:1299:23
   4: scratch::main::{{closure}}
             at ./src/main.rs:347:5
   5: <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll
             at /rustc/ae90dcf0207c57c3034f00b07048d63f8b2363c8/library/core/src/future/mod.rs:80:19
   6: tokio::park::thread::CachedParkThread::block_on::{{closure}}
             at /Users/feichao/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.10.0/src/park/thread.rs:263:54
   7: tokio::coop::with_budget::{{closure}}
             at /Users/feichao/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.10.0/src/coop.rs:106:9
   8: std::thread::local::LocalKey<T>::try_with
             at /rustc/ae90dcf0207c57c3034f00b07048d63f8b2363c8/library/std/src/thread/local.rs:399:16
   9: std::thread::local::LocalKey<T>::with
             at /rustc/ae90dcf0207c57c3034f00b07048d63f8b2363c8/library/std/src/thread/local.rs:375:9
  10: tokio::coop::with_budget
             at /Users/feichao/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.10.0/src/coop.rs:99:5
  11: tokio::coop::budget
             at /Users/feichao/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.10.0/src/coop.rs:76:5
  12: tokio::park::thread::CachedParkThread::block_on
             at /Users/feichao/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.10.0/src/park/thread.rs:263:31
  13: tokio::runtime::enter::Enter::block_on
             at /Users/feichao/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.10.0/src/runtime/enter.rs:151:13
  14: tokio::runtime::thread_pool::ThreadPool::block_on
             at /Users/feichao/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.10.0/src/runtime/thread_pool/mod.rs:71:9
  15: tokio::runtime::Runtime::block_on
             at /Users/feichao/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.10.0/src/runtime/mod.rs:452:43
  16: scratch::main
             at ./src/main.rs:348:5
  17: core::ops::function::FnOnce::call_once
             at /rustc/ae90dcf0207c57c3034f00b07048d63f8b2363c8/library/core/src/ops/function.rs:227:5

Tikv setup:

image
@ekexium
Copy link
Collaborator

ekexium commented Aug 18, 2021 •
edited
Loading

Which version are you using?

Thanks for your feedback! Confirmed it's a bug. TiKV doesn't actually write mutations whose op are CheckNotExists. client-rust mistakenly treats it the same way as other operations.

@ekexium ekexium added bug Something isn't working good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. labels Aug 18, 2021
@gfreezy
Copy link
Contributor Author

gfreezy commented Aug 18, 2021

Is it a TiKV bug or client-rust's ?

@ekexium
Copy link
Collaborator

ekexium commented Aug 18, 2021

It's cilent-rust's

@gfreezy
Copy link
Contributor Author

gfreezy commented Aug 18, 2021

I'd like to try fixing the bug. Any Docs or guide how to do it?

@ekexium
Copy link
Collaborator

ekexium commented Aug 18, 2021

CheckNotExists is checked in the prewrite phase and should not appear in the commit phase. So I think there are basically two things to do:

  1. When we set the primary key of a transaction, the corresponding operation must not be CheckNotExists.
  2. We should filter out mutations with CheckNotExists in the commit phase.

@gfreezy
Copy link
Contributor Author

gfreezy commented Aug 24, 2021

What does primary key means in a tikv transaction?

@gfreezy gfreezy linked a pull request Aug 24, 2021 that will close this issue
Open
@ekexium
Copy link
Collaborator

ekexium commented Aug 24, 2021

The primary key can be viewed as a representative of the transaction. Each transaction that needs to write some key-values must have a primary key.

In client-rust, the primary key of a transaction is set at

client-rust/src/transaction/buffer.rs

Line 12 in eb1d2da

primary_key: Option<Key>,

There is a difference in how primary keys are set between TiDB(client-go) and client-rust. cilent-go doesn't set the primary key until committing. client-rust, in the current implementation, can set the primary key when locking keys. I think we may need to consider whether there is a better way to do this.

@gfreezy
Copy link
Contributor Author

gfreezy commented Aug 25, 2021

Should we change the way how primary key is set to be the same as go?

@ekexium
Copy link
Collaborator

ekexium commented Aug 25, 2021

Should we change the way how primary key is set to be the same as go?

That's one way to fix the issue. But it may introduce other subtleties. For example, pessimistic lock requests require a primary key, even when the transaction isn't ready to commit. Maybe we can temporarily set the PK as the key that needs to be locked? We must carefully reason its correctness.

I don't have a simple solution for now. @andylokandy do you have any good ideas?

@andylokandy
Copy link
Collaborator

andylokandy commented Jul 10, 2023 •
edited
Loading

@ekexium I have a question about the case this PR is going to resolve: what should the PK be if a transaction's mutations contain only one CheckNotExist? e.g.: begin, insert('k1'), delete('k1'), commit

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines.
Projects
None yet
Milestone
No milestone
3 participants
@gfreezy @andylokandy @ekexium