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

kv: introduce memory accounting / limits for lock table #49387

Open
knz opened this issue May 21, 2020 · 3 comments
Open

kv: introduce memory accounting / limits for lock table #49387

knz opened this issue May 21, 2020 · 3 comments
Labels
A-kv-transactions Relating to MVCC and the transactional model. C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-kv KV Team
Projects

Comments

@knz
Copy link
Contributor

knz commented May 21, 2020

There's a limit on the max number of locks per lock table; however:

  • There's one lock table per range, so with large number of ranges there's a large number of lock tables.
  • There's no limit on the number of waiters. Under contention the per-table size can grow arbitrarily large.

Context:

@knz: nathan sumeer where is the memory usage of the lock table accounted for? What's the metric? What's the memory monitor that it's accounted to?
@nvanbenschoten: Like all of KV, it’s not connected to a memory monitor. The accounting here is pretty naive. We limit the size of each lockTable to a maximum number of locks and don’t allow it to grow beyond that. In my testing, this prevented the memory footprint from growing above 3MB per Range. There’s plenty of room for improvement here.
@knz: there's a limit on the number of locks, but is there also a limit on the number of waiters per lock? Otherwise, with just 1 lock I can get an arbitrary large data structure
@sumeerbhola : one place where this naive accounting would break is when the number of locks is below the limit, but the length of each lock queue is big.
@nvanbenschoten : the waiters per lock is bounded by the number of requests in the system
@knz : 3MB per range x 100 hot ranges = 300MB. That's enough to make a node that's close to the brim in RAM usage fall over (edited)
@nvanbenschoten The first step here is to make this memory accounting per-store instead of per-range

Jira issue: CRDB-4232

@knz knz added C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) A-kv-transactions Relating to MVCC and the transactional model. labels May 21, 2020
@knz
Copy link
Contributor Author

knz commented May 21, 2020

NB: number one priority is to add a metric.

It had been identified in #44976 but not done yet (also there was no issue filed yet)

@knz
Copy link
Contributor Author

knz commented May 21, 2020

a thought: it's a bit odd that if I have 100000 ranges each containing 1 row, I can have 100000 lock entries, but if I have just 1 range containing 100000 rows, my lock table will just contain 1000 entries (because that's the per-range limit)

(I actually don't know what the specific limit is, but you get my drift- the mechanism shouldn't be different depending on how rows are spread across ranges)

it makes it a bit hard to explain to users IMHO

@tbg tbg added this to Incoming in KV Mar 23, 2021
@jlinder jlinder added the T-kv KV Team label Jun 16, 2021
@lunevalex lunevalex moved this from Incoming to On Hold in KV Sep 2, 2021
@github-actions
Copy link

We have marked this issue as stale because it has been inactive for
18 months. If this issue is still relevant, removing the stale label
or adding a comment will keep it active. Otherwise, we'll close it in
10 days to keep the issue queue tidy. Thank you for your contribution
to CockroachDB!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-kv-transactions Relating to MVCC and the transactional model. C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-kv KV Team
Projects
KV
On Hold
Development

No branches or pull requests

2 participants