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

fix(ebpf): ignore kthreads, do full pids cleanup #2778

Merged
merged 4 commits into from
Nov 30, 2023
Merged

Conversation

korniltsev
Copy link
Collaborator

@korniltsev korniltsev commented Nov 29, 2023

This PR addresses two issues

  1. address ebpf: pids map mem leak #2769

I've noticed I was incorrectly tracking kthreads, which I dont care about.

Ignore kthreads.

Also added a "stale check" to cleanup pids which are not existing, but we did not get a dead notification for them(I am not aware of any other reason, so in theory this should not be needed)

  1. I've noticed sometime ebpf program requests information for the same PID multiple times
ts=2023-11-30T06:36:59.455026088Z level=debug msg="pid info request" component=pyroscope.ebpf.instance pid=1879972 target="{__container_id__=\"3a1ace6dfee59c11d984f688d8ef9a5c3a9dbf4c0be45ffdef15009f6631fb64\", __name__=\"process_cpu\", app=\"grafana\", conprof=\"true\", container=\"grafana\", insights=\"true\", name=\"grafana\", namespace=\"hosted-grafana\", node=\"gke-prod-us-central-0-hg-n2s8-9-b0d8ad12-gb65\", org=\"marathon\", plan=\"advanced\", pod=\"marathon-grafana-55cd8dfc94-8blkc\", pod_template_hash=\"55cd8dfc94\", service_name=\"ebpf/hosted-grafana/grafana\", slug=\"marathon\", stackId=\"492115\"}"
ts=2023-11-30T06:36:59.455026088Z level=debug msg="pid info request" component=pyroscope.ebpf.instance pid=1879972 target="{__container_id__=\"3a1ace6dfee59c11d984f688d8ef9a5c3a9dbf4c0be45ffdef15009f6631fb64\", __name__=\"process_cpu\", app=\"grafana\", conprof=\"true\", container=\"grafana\", insights=\"true\", name=\"grafana\", namespace=\"hosted-grafana\", node=\"gke-prod-us-central-0-hg-n2s8-9-b0d8ad12-gb65\", org=\"marathon\", plan=\"advanced\", pod=\"marathon-grafana-55cd8dfc94-8blkc\", pod_template_hash=\"55cd8dfc94\", service_name=\"ebpf/hosted-grafana/grafana\", slug=\"marathon\", stackId=\"492115\"}"

This can probably happen when multiple threads are profiled.
So i updated ebpf program to only request pid info once.

@korniltsev korniltsev enabled auto-merge (squash) November 30, 2023 10:12
}

// iterate over all pids and check if they are alive
// it is only needed in case disassociate_ctty hook somehow mises a process death
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// it is only needed in case disassociate_ctty hook somehow mises a process death
// it is only needed in case disassociate_ctty hook somehow misses a process death

Copy link
Contributor

@cyriltovena cyriltovena left a comment

Choose a reason for hiding this comment

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

LGTM

@korniltsev korniltsev merged commit 8dc366a into main Nov 30, 2023
19 checks passed
@korniltsev korniltsev deleted the fix-pids-leak branch November 30, 2023 15:36
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.

None yet

2 participants