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

Use extent hooks for per query memory limit #1340

Merged
merged 61 commits into from
Oct 25, 2023

Conversation

antoniofilipovic
Copy link
Contributor

@antoniofilipovic antoniofilipovic commented Oct 9, 2023

Each thread is mapped to one arena in jemalloc. Since we are working with a multi-thread environment where multiple threads can be mapped to the same arena, the thread ID is used as an identifier which is mapped to the transaction id. Each transaction can therefore have multiple threads that do allocation. Each thread is mapped internally to transaction id and has its own tracker. When a transaction with that id is done, the tracker is destroyed. If we have multiple queries inside an explicit transaction, each will have the same id, but they can't overwrite each other since queries are sequentially executed.

[master < Task] PR

  • Check, and update documentation if necessary
  • Provide the full content or a guide for the final git message

To keep docs changelog up to date, one more thing to do:

  • Write a release note here, including added/changed clauses
  • Tag someone from docs team in the comments

What needs to be done

  • Add test for limit per query only one - CREATE, SET, DELETE, procedure with one thread
  • Decide what happens with per-arena tracking if a user forgets to remove the thread from tracking
  • Move the new API for query tracking to a new namespace and isolate it inside the class

This commit introduces memory tracker with jemalloc extent hooks which fully works in case when jemalloc config is following:
MALLOC_CONF="retain:false,percpu_arena:percpu,oversize_threshold:1000000000000,muzzy_decay_ms:0,dirty_decay_ms:0" \
./configure \
    --disable-cxx \
    $COMMON_CONFIGURE_FLAGS \
    --with-malloc-conf="retain:false,percpu_arena:percpu,oversize_threshold:1000000000000,muzzy_decay_ms:0,dirty_decay_ms:0"

This config will for jemalloc not to use lazy purge or MADV_FREE(muzzy_decay_ms=0 and dirty_decay_ms=0), it will force jemalloc not to use
custom arena for huge allocations (oversize_threshold) and it will force jemalloc not extend virtual memory indefinitely (retain=false)
and therefore call alloc hook when allocation actually takes place.

Only problem is if we do huge allocations which are not mapped on alloc directly, in that case jemalloc uses cache and we can overcounter
allocation size.
@antoniofilipovic
Copy link
Contributor Author

@Ignition @Josipmrden made one final fix on PR. It turns out there was a deadlocking issue. When one thread tried to register and map the thread id to the transaction id, an allocation request was made which ended up in getting tracker by thread id, which would result in deadlock since the lock was still locked, but ended up in a different function. To fix issue, I used utils::SkipList which is concurrent so no need for synchronization any more and everything now works.

@antoniofilipovic
Copy link
Contributor Author

PR fixes this issue: #977

Base automatically changed from improve-memory-tracker to master October 23, 2023 10:48
@antoniofilipovic antoniofilipovic added the Docs needed Docs needed label Oct 23, 2023
@antoniofilipovic
Copy link
Contributor Author

antoniofilipovic commented Oct 24, 2023

One more thing left to solve here:

  • detach tracker from the transaction after all pulling is done

@antoniofilipovic antoniofilipovic merged commit a84f570 into master Oct 25, 2023
1 of 6 checks passed
@antoniofilipovic antoniofilipovic deleted the improve-memory-query-limit branch October 25, 2023 14:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants