Skip to content

Conversation

mackenzie-grimes-noaa
Copy link
Contributor

@mackenzie-grimes-noaa mackenzie-grimes-noaa commented Apr 26, 2025

Linear Issue

IDSSE-1164

Changes

  • Add utils.FileBasedLock

Example:

with FileBasedLock('my_file.grib2', max_age=30):
    # now we're sure our thread alone is reading/writing my_file.grib2
    open('my_file.grib2', 'r') # ...
print('File released to other threads!')

Explanation

These changes will make a blank foo.<ext>.lock file on the filesystem before reading from/writing to a foo.<ext> file.

  • If we find a .lock file already there, we loop waiting for it to disappear.
  • If we've waited max_age seconds, we assume the thread that locked it is dead somehow, and we take over the file lock anyway.

Doesn't every OS have locks built into their file I/O?

Yes, but IDSSe can't always rely on that. Our services can be running on different Kubernetes nodes, or different containers in that node, or different Python/Gunicorn processes, and the filesystem they're reading/writing files from could be local FS, or a Network File Share (NFS). This makes it hard to know for sure that no one else is tinkering with your file when your thread goes to do file I/O.

@mackenzie-grimes-noaa mackenzie-grimes-noaa marked this pull request as ready for review April 28, 2025 23:06
…t thread safe, so these warnings can't be fixed
@mackenzie-grimes-noaa mackenzie-grimes-noaa changed the title bug: protect NetCDF files from multi-thread collisions bug: add FileBasedLock (NFS proof) Apr 28, 2025
@mackenzie-grimes-noaa
Copy link
Contributor Author

@rabellino-noaa not sure what you want to do with Python code reviews going forward.

I don't think it's worth your time to rubber-stamp each one, since the microservice app code is pretty much only my problem now. But we technically set up "at least 1 review" to protect this repo, so this time I requested a rubber stamp.

@mackenzie-grimes-noaa
Copy link
Contributor Author

@rabellino-noaa not sure what you want to do with Python code reviews going forward.

I don't think it's worth your time to rubber-stamp each one, since the microservice app code is pretty much only my problem now. But we technically set up "at least 1 review" to protect this repo, so this time I requested a rubber stamp.

Oops, Mike it out of the office this week. I'll rubber stamp it myself:
✅ approved

@mackenzie-grimes-noaa mackenzie-grimes-noaa merged commit 8daca57 into main Apr 29, 2025
2 checks passed
@mackenzie-grimes-noaa mackenzie-grimes-noaa deleted the bug/file-lock branch April 29, 2025 16:28
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.

1 participant