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

chore: Add --allocator_tracker for default tracking #3901

Merged
merged 3 commits into from
Oct 13, 2024

Conversation

chakaz
Copy link
Collaborator

@chakaz chakaz commented Oct 9, 2024

Before, in order to use allocation tracker, one had to issue a MEMORY TRACK command. This flag is identical to that, but allows starting Dragonfly with certain ranges without issuing a command.

While here, fix a bug. Apparently, absl::InlinedVector<> has a bug in the implementation of max_size() and so in practice we did not limit the number of trackers. I switched to use capacity() instead, which I tested and it works well.

Notes:

  1. Currently the flag always add 100% "sampling", we can extend that in the future if need be
  2. I added the flag in dfly_main.cc with custom initialization, because it's low level, and I couldn't get it reasonably working with changes only to allocation_tracker.cc

Before, in order to use allocation tracker, one had to issue a `MEMORY
TRACK` command. This flag is identical to that, but allows starting
Dragonfly with certain ranges without issuing a command.

While here, fix a bug. Apparently, `absl::InlinedVector<>` has a bug in
the implementation of `max_size()` and so in practice we did not limit
the number of trackers. I switched to use `capacity()` instead, which I
tested and it works well.

Notes:
1. Currently the flag always add 100% "sampling", we can extend that in
   the future if need be
2. I added the flag in `dfly_main.cc` with custom initialization,
   because it's low level, and I couldn't get it reasonably working with
   changes only to `allocation_tracker.cc`
@chakaz chakaz requested a review from adiholden October 9, 2024 12:25
@@ -547,6 +549,40 @@ bool UpdateResourceLimitsIfInsideContainer(io::MemInfoData* mdata, size_t* max_t

#endif

void SetupAllocationTracker(ProactorPool* pool) {
string flag = absl::GetFlag(FLAGS_allocation_tracker);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DFLY_ENABLE_MEMORY_TRACKING must be defined for adding allocation tracking

@@ -73,6 +73,8 @@ ABSL_FLAG(string, unixsocketperm, "", "Set permissions for unixsocket, in octal
ABSL_FLAG(bool, force_epoll, false,
"If true - uses linux epoll engine underneath. "
"Can fit for kernels older than 5.10.");
ABSL_FLAG(string, allocation_tracker, "",
"Allocation ranges to log. Format is min:max,min:max,....");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe
Print stack trace of allocation within ranges

@chakaz chakaz requested a review from adiholden October 13, 2024 07:11
@chakaz chakaz merged commit cf2e94f into main Oct 13, 2024
12 checks passed
@chakaz chakaz deleted the chakaz/allocator-tracker-flag branch October 13, 2024 09:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants