Skip to content

Conversation

@alco
Copy link
Member

@alco alco commented Oct 15, 2025

Disable ApplicationTelemetry.process_memory() periodic measurement by default.

This is a costly probe that we've seen take 1.3 seconds to complete. At 5-second periodicity, it runs 26% of the time. There is an additional cost of ensuring the return value of Process.list() is a consistent snapshot of the system processes, we do not really need it.

Top processes by reductions or memory can be observed under human supervision by running etop (), as in

:etop.start(lines: 40, sort: :memory, interval: 2)

This PR also introduces the possibility of passing custom periodic measurements to be run at the same cadence as the builtin ones. This is done to make it easier to add probes in production without havgint to update the Electric dependency version every time.

… default

This is a costly probe that we've seen take 1.3 seconds to complete. At
5-second periodicity, it runs 26% of the time. There is an additional
cost of ensuring the return value of Process.list() is a consistent
snapshot of the system processes, we do not really need it.
@alco alco changed the title Alco/periodic measurements chore: Make periodic measurements customizable and drop the process memory probe Oct 15, 2025
@alco alco changed the title chore: Make periodic measurements customizable and drop the process memory probe chore: Make periodic measurements customizable and drop the top process memory probe Oct 15, 2025
@codecov
Copy link

codecov bot commented Oct 15, 2025

Codecov Report

❌ Patch coverage is 0% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 71.05%. Comparing base (bf44bbc) to head (9720b6a).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
...ce/lib/electric/telemetry/application_telemetry.ex 0.00% 7 Missing ⚠️
...-service/lib/electric/telemetry/stack_telemetry.ex 0.00% 5 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3291      +/-   ##
==========================================
- Coverage   71.12%   71.05%   -0.08%     
==========================================
  Files         176      177       +1     
  Lines        9366     9449      +83     
  Branches      318      336      +18     
==========================================
+ Hits         6662     6714      +52     
- Misses       2702     2733      +31     
  Partials        2        2              
Flag Coverage Δ
elixir 68.27% <0.00%> (-0.22%) ⬇️
elixir-client 73.94% <ø> (ø)
packages/experimental 88.95% <ø> (ø)
packages/react-hooks 86.48% <ø> (?)
packages/typescript-client 94.30% <ø> (-0.11%) ⬇️
packages/y-electric 55.12% <ø> (ø)
postgres-140000 67.63% <0.00%> (?)
postgres-150000 67.60% <0.00%> (-0.18%) ⬇️
postgres-170000 67.66% <0.00%> (-0.20%) ⬇️
postgres-180000 67.60% <0.00%> (-0.16%) ⬇️
sync-service 67.69% <0.00%> (-0.24%) ⬇️
typescript 87.39% <ø> (-0.13%) ⬇️
unit-tests 71.05% <0.00%> (-0.08%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@alco alco force-pushed the alco/periodic-measurements branch from c74aff7 to 8ec32d8 Compare October 15, 2025 23:54
@msfstef
Copy link
Contributor

msfstef commented Oct 16, 2025

Would we be able to still maintain a per-process probe without manual intervention? It is useful to have some stats on this over time and over various releases - if it's costly perhaps it could/should run in a much sparser frequency? Or if there's a way to not pay the cost for the consistent snapshot perhaps there's a cheaper alternative?

@alco alco changed the title chore: Make periodic measurements customizable and drop the top process memory probe chore: Make periodic measurements customizable Oct 20, 2025
@alco
Copy link
Member Author

alco commented Oct 20, 2025

@msfstef sorry for making it unnecessarily confusing. I have reverted the change that disabled memory collection as it didn't have a deciding impact on performance. We can make it even less noticeable by using process iteration in the same way as :recon.proc_count does.

I've opened an issue for that.

Copy link
Contributor

@msfstef msfstef left a comment

Choose a reason for hiding this comment

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

@alco thanks for clarifying as I couldn't match the PR desc to the code haha - this looks good!

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