CF icon indicating copy to clipboard operation
CF copied to clipboard

Follow temporary file + rename pattern for all files

Open jphickey opened this issue 3 years ago • 3 comments

Checklist (Please check before submitting)

  • [x] I reviewed the Contributing Guide.
  • [x] I reviewed the CF README.md file to see if the feature is in the major future work.
  • [x] I performed a cursory search to see if the feature request is relevant, not redundant, nor in conflict with other tickets.

Is your feature request related to a problem? Please describe. Currently, there is a special case to handle situations where the MD PDU arrives out of order. A temporary file is opened, and then renamed once the MD packet does arrive.

However, because this is only done for off-nominal cases, it is likely not as well tested as other paths (see bug #131).

Describe the solution you'd like This pattern of using a temp file should be standard operating procedure, not just something for special cases. Reasons/advantages for always doing it this way described in my comment here: https://github.com/nasa/CF/issues/131#issuecomment-1183239857

This improves atomicity of file updates, prevents clobbering files and avoids cases where other apps might see partial files or other bad content.

Describe alternatives you've considered N/A

Additional context As the fix for #131 strictly only fixed the issue described, this is a more general enhancement that would improve CF.

Requester Info Joseph Hickey, Vantage Systems, Inc.

jphickey avatar Jul 13 '22 14:07 jphickey

consider doing the move in a child task if it's across file systems... could take a long time and shouldn't block PDU processing

skliper avatar Jul 13 '22 15:07 skliper

Adding a child task just for that could be complicated because AFAIK there isn't such a post-processing thread right now... that would be a bigger impact design-wise to add such a thing.

I was thinking one could ensure that they are on the same filesystem if the temp file is only a name variation, such as adding .tmp (e.g. /path/to/file.xyz.tmp for /path/to/file.xyz) ... then you know OS_rename should work, but unfortunately that is only possible if you know what the final filename is (so not applicable for the case where MD arrives late...)

I think its also valid (at least at first) to defer with documentation - i.e. reiterate the recommendation that the temp dir for CF should be on the same filesystem as the final destination - and wait until there is an actual use-case before adding complexity of another thread to address it. That is, if files are small OR the system is set up for fast OS_rename then there is no issue operationally.

The potential problem is really only an issue for a user that is transferring relatively large files, has separate filesystem for tmp, and also sends multiple files in parallel (such that the gap in PDU processing is noticeable in other transfers) ...

jphickey avatar Jul 13 '22 16:07 jphickey

Good points/plan... I guess the MD out of order is somewhat unique in that you don't know the destination filesystem until the MD arrives... which leads to needing a temporary directory for storage. Easy enough for data with an associated MD to create it as you mentioned (in the same filesystem).

skliper avatar Jul 13 '22 17:07 skliper