Skip to content

Conversation

marksvc
Copy link
Collaborator

@marksvc marksvc commented Aug 19, 2025

This change is Reviewable

Copy link

codecov bot commented Aug 19, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.22%. Comparing base (f4c61a0) to head (88b8335).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #3383   +/-   ##
=======================================
  Coverage   82.21%   82.22%           
=======================================
  Files         608      608           
  Lines       36199    36199           
  Branches     5935     5930    -5     
=======================================
+ Hits        29761    29764    +3     
+ Misses       5563     5562    -1     
+ Partials      875      873    -2     

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

Copy link
Collaborator Author

@marksvc marksvc left a comment

Choose a reason for hiding this comment

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

I have been using this script on our servers and it seems to be working well. I wanted to add it to our code repo to keep track of it.

Reviewable status: 0 of 1 files reviewed, all discussions resolved


scripts/watch-for-rts-spike.mts line 45 at r1 (raw file):

      }

      const aboveThreshold: boolean = memoryUsageMB >= this.options.thresholdMib;

Hmm, I would be inclined to change this from >= to >, except that this is what is running on the server right now :)

@marksvc
Copy link
Collaborator Author

marksvc commented Aug 27, 2025

This does not require testing because it is a developer tool.

Although it can be used / tested by running ./watch-for-rts-spike.mts. --help shows options. Deno needs to be installed.

@marksvc marksvc marked this pull request as ready for review August 27, 2025 21:00
@RaymondLuong3 RaymondLuong3 self-assigned this Aug 29, 2025
Copy link
Collaborator

@RaymondLuong3 RaymondLuong3 left a comment

Choose a reason for hiding this comment

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

Mark and I looked at this together. It seems we are not able to get it working on my windows machine. I looked through the code and it seems to be doing the right things.

@RaymondLuong3 reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @marksvc)


scripts/watch-for-rts-spike.mts line 6 at r1 (raw file):

// sends SIGUSR2 signals at a configured interval so the target process can perform diagnostic actions.
// Designed to help capture data around sudden RealtimeServer memory spikes.

Could you add a usage line with the necessary environment variables here.

@RaymondLuong3 RaymondLuong3 removed their assignment Aug 29, 2025
@marksvc
Copy link
Collaborator Author

marksvc commented Aug 29, 2025

Raymond and I had a look at how we might run this on Windows. pgrep was not available. ps -W was able to show Windows PIDs, but it did not appear to see the RealtimeServer process. A Windows specific task list program might be successful. /bin/kill -f -W was able to send signals to RealtimeServer, though when specifying -SIGUSR2, it killed RealtimeServer and did not generate .csv files. More work could be done to make the script multi-platform; though right now it just needs to run on the server.

@marksvc
Copy link
Collaborator Author

marksvc commented Aug 29, 2025

When testing, make sure to run dotnet with environment variable setting SF_SIGUSR2_ACTION=resourceUsage.

Copy link
Collaborator Author

@marksvc marksvc left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @RaymondLuong3)


scripts/watch-for-rts-spike.mts line 6 at r1 (raw file):

Previously, RaymondLuong3 (Raymond Luong) wrote…

Could you add a usage line with the necessary environment variables here.

I added a note about SF_SIGUSR2_ACTION, as well as a pointer to diagnostics.ts, which can be followed to learn about what happens when a report is requested.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants