txn: only wake up waiters when locks are indeed released

[youjiali1995]

Signed-off-by: youjiali1995 zlwgx1023@gmail.com

What problem does this PR solve?

Problem Summary:
TiKV will wake up waiters as long as it receives requests that may release locks, e.g., pessimistic_rollback, rollback, commit. If a request doesn't release locks, typically the lock doesn't exist, it needn't wake up waiters.

In TiDB, if a pessimistic DML meets write conflict, it will use pessimistic_rollback to clean up all locks it needs to lock in this DML and then retry the DML. If a transaction is waked up and there are other transactions waiting for the lock, these transactions will be waked up by pessimistic_rollback one by one. It dramatically affects performance and results in useless retry.

What is changed and how it works?

What's Changed:
Only wake up waiters when locks are indeed released and small refactor.

Related changes

• Need to cherry-pick to the release branch

Check List

Tests

• Manual test (add detailed scripts or steps below)
• No code

I think existing tests are enough and I benched it using sysbench with the workload below:

con:query(CREATE TABLE wmtest(k INT PRIMARY KEY, v INT))
con:query(string.format("UPDATE wmtest SET v = v + 1 WHERE k IN (%d, 1)", sysbench.rand.uniform(2, sysbench.opt.table_size)))

master:

threads	tps	avg lat(ms)	.95 lat(ms)	.99 retry count
1	783	1.28	1.79	0
100	71	1393	2493	800
500	16	30111	72316	1457
1000	12	71181	100000	654

This PR:

threads	tps	avg lat(ms)	.95 lat(ms)	.99 retry count
100	772	129	155	1
500	724	688	787	1
1000	647	1534	1903	5

Release note

Fix the issue that needless wake-up results in useless retry and performance reduction in heavy contention workloads.

[youjiali1995]

/release

[sre-bot]

download tikv at http://fileserver.pingcap.net/download/builds/pingcap/tikv/pr/f0940bded80643cd53de49b5ff30fee7e6397e10/centos7/tikv-server.tar.gz

[sticnarf]

LGTM except a small question.

[youjiali1995]

If the DML only need to lock one key, it won't send pessimistic_rollback before retry. That's why I didn't find the bug earlier:
https://github.com/pingcap/tidb/blob/b54ac5b2ec3bd1f5de37eb813d17de43cc500bf3/store/tikv/txn.go#L397

[nrc]

Looks good, a few comments inline, none are major

[nrc]

You could implement a from_iter method to build the hashes Vec in one go, rather than using for loops and push

[youjiali1995]

Sorry, I don't get it. I still need to push hash to a vec to get the iter..

[nrc]

So for example, rather than use

for k in keys {
    released_locks.push(txn.rollback(k)?);
}

you can use

released_locks.hashes(keys.iter().map(|k| txn.rollback(k)))?;

where hashes is something like:

fn hashes<I, T, E>(&mut self, iter: I) -> Result<(), E>
    where
        I: Iterator<Item = Result<T, E>>
{
    self.hashes = iter.collect()?;
    Ok(())
}

[youjiali1995]

I see. Thanks!

[youjiali1995]

I tried to implement it but I found it's more complex than for loop:

impl ReleasedLocks {
    fn hashes<I, E>(&mut self, iter: I) -> std::result::Result<(), E>
    where
        I: Iterator<Item = std::result::Result<Option<ReleasedLock>, E>>,
    {
        self.hashes = iter
            .filter_map(|v| match v {
                Ok(Some(lock)) => {
                    if !self.pessimistic {
                        self.pessimistic = lock.pessimistic;
                    }
                    Some(Ok(lock.hash))
                }
                Ok(None) => None,
                Err(e) => Some(Err(e)),
            })
            .collect::<std::result::Result<Vec<u64>, E>>()?;
        Ok(())
    }
}

fn process_write_impl(...) {
...
            let mut released_locks = ReleasedLocks::new(lock_ts, commit_ts);
            // for k in keys {
            // released_locks.push(txn.commit(k, commit_ts)?);
            // }
            released_locks.hashes(keys.into_iter().map(|k| txn.commit(k, commit_ts)))?;
            released_locks.wake_up(lock_mgr.as_ref());
...
}

[youjiali1995]

And I still need to call push when processing ResolveLock..

[youjiali1995]

@nrc PTAL again. Thanks!

[youjiali1995]

/run-all-tests

[youjiali1995]

/bench +tpcc

[sre-bot]

Benchmark Report

Run TPC-C Performance Test on VMs

@@                               Benchmark Diff                               @@
================================================================================
tidb: c1a31708b0a3eb8e75adbc5cf75c86926fcf4d1b
--- tikv: 2b1b9b2cd537e47591795dc034b59a0585cfe7a8
+++ tikv: 1641dbb41273c50b7a0b630295d65fc9e722076e
pd: 8438f3fc004da1bff7442229e53fe4272f74ce2d
================================================================================
Measured tpmC (NewOrders): 8296.83 ± 1.98% (std=106.77), delta: -2.84% (p=0.036)

[youjiali1995]

/bench +tpcc

[sre-bot]

Benchmark Report

Run TPC-C Performance Test on VMs

@@                               Benchmark Diff                               @@
================================================================================
tidb: c1a31708b0a3eb8e75adbc5cf75c86926fcf4d1b
--- tikv: 7935019849d46b7d32b1a6b0d14e795cd7da1591
+++ tikv: 1641dbb41273c50b7a0b630295d65fc9e722076e
pd: 8438f3fc004da1bff7442229e53fe4272f74ce2d
================================================================================
Measured tpmC (NewOrders): 7007.72 ± 1.01% (std=66.19), delta: -1.77% (p=0.072)

[nrc]

LGTM, thanks for the changes!

[youjiali1995]

@MyonKeminta PTAL

[sre-bot]

/run-all-tests

[sre-bot]

cherry pick to release-3.0 in PR #7549

[sre-bot]

cherry pick to release-3.1 in PR #7550

[sre-bot]

cherry pick to release-4.0 in PR #7551