Use file metadata to determine whether profiler config should be reloaded. #464
Conversation
Codecov Report
@@ Coverage Diff @@
## master #464 +/- ##
==========================================
- Coverage 65.62% 56.60% -9.03%
==========================================
Files 172 113 -59
Lines 13260 10277 -2983
==========================================
- Hits 8702 5817 -2885
+ Misses 4558 4460 -98
Continue to review full report at Codecov.
|
| def get_last_modified_time(filepath): | ||
| """ | ||
| Get the last time that the file at the given filepath was modified, in the form of a datetime object. | ||
| """ | ||
| last_modified_time = os.path.getmtime(filepath) | ||
| return datetime.fromtimestamp(last_modified_time) # get the last time the config was modified |
There was a problem hiding this comment.
I want to make sure this does not return false positives.
What if an agent owned by the platform team touches the file which causes changes in the last_modified_time value?
Checking for a change in file_size itself might provide a stronger signal.
What do you think?
There was a problem hiding this comment.
Interesting. I see you've used getatime below and getmtime here.
What is the difference between the two?
There was a problem hiding this comment.
Are there any long term risks? Do we have to worry about the OS the container is running on?
Description of changes:
(Unable to reopen #463 so I'm creating a new PR).
For each step, we need to determine if the profiler config JSON has changed, and if so, we should reload the profiler config. Currently, we reload the JSON into memory and physically check whether the file contents have changed in order to determine if the profiler config should be reloaded. However, this may pose problems for performance at scale because we would be loading a JSON object into memory at each step.
This change replaces the above check by inspecting the file metadata for the last modified time. If the last modified time has changed, that means the file has changed and we should reload the profiler config. This is done without loading the JSON into memory (see tests, which verify that the config file is not accessed (read into memory) if the file has not been modified).
Style and formatting:
I have run
pre-commit installto ensure that auto-formatting happens with every commit.Issue number, if available
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.