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.

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

cdc's old value cache sometimes exceeds the quota #14815

Closed
YuJuncen opened this issue May 24, 2023 · 1 comment · Fixed by #14847
Closed

cdc's old value cache sometimes exceeds the quota #14815

YuJuncen opened this issue May 24, 2023 · 1 comment · Fixed by #14847
Labels
affects-6.1 This bug affects the 6.1.x(LTS) versions. affects-6.5 This bug affects the 6.5.x(LTS) versions. affects-7.1 This bug affects the 7.1.x(LTS) versions. severity/moderate type/bug The issue is confirmed as a bug.

Comments

@YuJuncen
Copy link
Contributor

Bug Report

What version of TiKV are you using?

v6.5.1

What operating system and CPU are you using?

Not sure, but probably irrelative.

Steps to reproduce

(Not checked, but this did be observed in a production cluster.)

  1. Create a table with small raws(perhaps with a sole column of primary key), let it be t1.
  2. Create a table with larger raws(perhaps with a huge blob or json column), let it be t2.
  3. Open a changefeed which observes both t1 and t2.
  4. Update as many different rows as possible in t1.
  5. Update as many different rows as possible in t2, until the memory usage of old value doesn't increase any more.

What did you expect?

After step 5, the memory usage of old value cache should be kept under the memory quota.

What did happened?

f0dfe499-6db5-4c5d-b612-989d3dcbb821

The old-value cache usage exceeds the quota.

@YuJuncen YuJuncen added the type/bug The issue is confirmed as a bug. label May 26, 2023
@YuJuncen
Copy link
Contributor Author

I have reproduced this by a unit test case:

#[test]
    fn test_old_value_capacity_not_exceed_quota() {
        let mut cache = OldValueCache::new(ReadableSize(1000));
        fn short_val() -> OldValue {
            OldValue::Value { value: b"s".to_vec() }
        }
        fn long_val() -> OldValue {
            OldValue::Value { value: vec![b'l'; 1024] }
        }
        fn enc(i :i32) -> Key {
        Key::from_encoded(i32::to_ne_bytes(i).to_vec())
        }

        for i in 0..100 {
            cache.insert(enc(i), (short_val(), None));
        }
        for i in 100..200 {
            // access the previous key for making it not be evicted
            cache.cache.get(&enc(i-1));
            cache.insert(enc(i), (long_val(), None));
        }
        assert!(cache.cache.size() <= 1000, "but it is {}", cache.cache.size());
    }

It panicked at:

thread 'old_value::tests::test_old_value_capacity_not_exceed_quota' panicked at 'but it is 28647'

Which means it exceeded about 28x of the quota.

@YuJuncen YuJuncen added severity/moderate may-affects-6.1 may-affects-6.5 may-affects-7.1 affects-6.1 This bug affects the 6.1.x(LTS) versions. affects-6.6 affects-7.1 This bug affects the 7.1.x(LTS) versions. affects-6.5 This bug affects the 6.5.x(LTS) versions. and removed may-affects-6.1 may-affects-6.5 may-affects-7.1 affects-6.6 labels May 30, 2023
ti-chi-bot bot pushed a commit that referenced this issue May 30, 2023
close #14815

LRU cache now will try to make its size strictly less than the capacity.

Signed-off-by: hillium <[email protected]>

Co-authored-by: qupeng <[email protected]>
ti-chi-bot bot added a commit that referenced this issue Jun 30, 2023
close #14815, ref #14847

LRU cache now will try to make its size strictly less than the capacity.

Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com>
ti-chi-bot bot pushed a commit that referenced this issue Jul 12, 2023
close #14815

LRU cache now will try to make its size strictly less than the capacity.

Signed-off-by: hillium <[email protected]>

Co-authored-by: hillium <[email protected]>
Co-authored-by: 山岚 <[email protected]>
YuJuncen added a commit to YuJuncen/tikv that referenced this issue Jul 21, 2023
close tikv#14815

LRU cache now will try to make its size strictly less than the capacity.

Signed-off-by: hillium <[email protected]>

Co-authored-by: qupeng <[email protected]>
ti-chi-bot bot pushed a commit that referenced this issue Jul 21, 2023
close #14815

LRU cache now will try to make its size strictly less than the capacity.

Signed-off-by: hillium <[email protected]>

Co-authored-by: qupeng <[email protected]>
ti-chi-bot bot added a commit that referenced this issue Jul 25, 2023
close #14815

LRU cache now will try to make its size strictly less than the capacity.

Signed-off-by: hillium <[email protected]>

Co-authored-by: hillium <[email protected]>
Co-authored-by: tonyxuqqi <[email protected]>
Co-authored-by: ti-chi-bot[bot] <108142056+ti-chi-bot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects-6.1 This bug affects the 6.1.x(LTS) versions. affects-6.5 This bug affects the 6.5.x(LTS) versions. affects-7.1 This bug affects the 7.1.x(LTS) versions. severity/moderate type/bug The issue is confirmed as a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant