Replace cp with cpfs/cpreq for atomic copies to COM directories #4130

Copilot · 2025-10-06T17:54:38Z

Description

This PR addresses issue #3051 by replacing standard cp commands with atomic copy utilities (cpfs and cpreq from prod_util) for all copies to COM directories. This is required per NCO Implementation Standards v11 to prevent downstream programs from accessing incomplete files.

Problem

Using standard cp for large files can result in downstream programs accessing incomplete copies while the copy operation is still in progress. This is particularly problematic in operational weather forecasting where timing and data integrity are critical.

Solution

Replaced all instances of cp to COM directories with:

cpfs: For output files being copied TO COM directories (11 instances)
cpreq: For required input files being copied FROM COM directories (1 instance)

The cpfs utility provides atomic copies by:

Copying to a temporary file
Renaming the temp file only when the copy is complete

This ensures downstream processes never see partially written files.

Changes

Modified 3 files with 12 total changes:

ush/tropcy_relocate.sh (5 changes)

Pathname files copied within COMOUT_OBS
Required tcvitals input from COMOUT_OBS
Empty file creation for error handling
Relocation output files (inform, tcvitals)

ush/syndat_qctropcy.sh (6 changes)

Empty file creation for null tcvitals scenarios
Processed tcvitals output to COMOUT_OBS
JTWC/FNOC tcvitals output to COMOUT_OBS

ush/wave_prnc_ice.sh (1 change)

Ice data output to COMOUT_WAVE_PREP

Testing

✅ All modified scripts pass bash syntax validation
✅ No new shellcheck warnings introduced
✅ Verified no remaining plain cp to COM directories
✅ Verified no manual atomic copy patterns (copy to temp + mv)
✅ Changes maintain functional equivalence (identical output)

Impact

Functional: No change to output files or behavior
Operational: Improved reliability by preventing access to incomplete files during copy operations
Standards: Now complies with NCO Implementation Standards v11

Fixes #3051

Co-authored-by: @DavidHuber-NOAA

Original prompt

This section details on the original issue you should resolve

<issue_title>Use cpfs for copies that should be atomic</issue_title>
<issue_description>### What new functionality do you need?

Using the standard cp can result in downstream programs using incomplete copies, particularly for large files. prod_util provides a utility, cpfs, that does an atomic copy by copying to a temp file and then renaming the temp file when the copy is complete. This is an NCO requirement (see Implementation Standards v11).

There are also places where we are doing this type of copy manually. These should be replaced with using cpfs to simplify scripts.

What are the requirements for the new functionality?

All copies that should be atomic (e.g. most or all copies to COM) should use cpfs or cpreq instead of cp. Should use cpreq when files are required to be able to run, and cpfs when output is being copied to COMOUT.

cpfs does not allow globbing, so some modification may be needed.

Acceptance Criteria

No usage of cp when the copy could be used before completion (replaced with cpfs)

Replace instances where we are manually doing an atmoic copy with cpfs

Output identical to previous

Suggest a solution (optional)

Should we leverage NCP? There are still a lot of places we are directly using cp instead of NCP.</issue_description>

Comments on the Issue (you are @copilot in this section)
@WalterKolczynski-NOAA Related: #712 @DavidHuber-NOAA A thorough search of the code shows only a handful of calls to `cp` rather than `cpfs` or `cpreq`. Of these, only the following lines copy into COM (where `cpfs` should be used):

global-workflow/ush/tropcy_relocate.sh

Lines 351 to 352 in f294f5e

cp "${COMOUT_OBS}/${RUN}.${cycle}.sgesprep_pre-relocate_pathname.${tmmark}" \

"${COMOUT_OBS}/${RUN}.${cycle}.sgesprep_pathname.${tmmark}"

global-workflow/ush/tropcy_relocate.sh

Line 394 in f294f5e

cp "${COMOUT_OBS}/${RUN}.${cycle}.syndata.tcvitals.${tmmark}" "tcvitals.now"

global-workflow/ush/tropcy_relocate.sh

Lines 573 to 574 in f294f5e

cp "rel_inform1" "${COMOUT_OBS}/${RUN}.${cycle}.inform.relocate.${tmmark}"

cp "tcvitals" "${COMOUT_OBS}/${RUN}.${cycle}.tcvitals.relocate.${tmmark}"

global-workflow/ush/syndat_qctropcy.sh

Lines 357 to 365 in f294f5e

cp current "${COMOUT_OBS}/${RUN}.${cycle}.syndata.tcvitals.${tmmark}"

# Create the DBNet alert

if [[ "${SENDDBN}" == "YES" ]]; then

"${DBNROOT}/bin/dbn_alert" "MODEL" "GDAS_TCVITALS" "${job}" "${COMOUT_OBS}/${RUN}.${cycle}.syndata.tcvitals.${tmmark}"

fi

# Write JTWC/FNOC Tcvitals to /com path since not saved anywhere else

cp fnoc "${COMOUT_OBS}/${RUN}.${cycle}.jtwc-fnoc.tcvitals.${tmmark}"

global-workflow/ush/wave_prnc_ice.sh

Line 142 in f294f5e

cp ice.ww3 "${COMOUT_WAVE_PREP}/${icefile}"

</comment_new>

Fixes #2349

💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.

Co-authored-by: DavidHuber-NOAA <[email protected]>

JessicaMeixner-NOAA

The change in ush/wave_prnc_ice.sh is fine.

aerorahul

looks ok.
@copilot Are these the only place where cp is being used now?

…dc16050

aerorahul

copying dev/null is the same as touching the file

ush/syndat_qctropcy.sh

ush/tropcy_relocate.sh

Co-authored-by: Rahul Mahajan <[email protected]>

…ed-45fd-4baa-9f7e-6bcebdc16050

EdwardSafford-NOAA

The changes to the 3 minmon scripts look good to me.

DavidHuber-NOAA · 2025-10-23T12:13:20Z

Launching CI on WCOSS2.

DavidHuber-NOAA · 2025-10-23T18:57:52Z

All tests passed on WCOSS2. Awaiting approval from @aerorahul before merging.

aerorahul

A few suggestions, some questions.

aerorahul · 2025-10-23T19:09:01Z

ush/minmon_xtrct_costs.pl

      if( -e $filename2 ) {
         my $newfile2 = "${tankdir}/${filename2}";
-         system("cp -f $filename2 $newfile2"); 
+         system("cpreq -f $filename2 $newfile2");


why cpreq and not cpfs?

also, does cpreq take -f?

Probably don't need it regardless.

ush/getges.sh

aerorahul · 2025-10-23T19:11:35Z

ush/minmon_xtrct_reduct.pl

   if( -e $outfile ) {
      my $newfile = "${tankdir}/${outfile}";
-      system("cp -f $outfile $newfile");
+      system("cpfs -f $outfile $newfile");


Suggested change

system("cpfs -f $outfile $newfile");

system("cpfs $outfile $newfile");

Does cpfs take -f?

No it does not.

aerorahul · 2025-10-23T19:11:51Z

ush/minmon_xtrct_gnorms.pl

+
      if( -e $filename2 ) {
-         system("cp -f $filename2 ${tankdir}/.");
+         system("cpfs -f $filename2 ${tankdir}/.");


same here and a few other places

ush/wave_prnc_ice.sh

DavidHuber-NOAA · 2025-10-23T20:19:31Z

I note in the logs the following error messages from the vminmon jobs (before making the recent changes):

+ exglobal_atmos_vminmon.sh[54]/lfs/h2/emc/global/noscrub/david.huber/GW/gw_cpfs/ush/minmon_xtrct_reduct.pl gfs 20211221 00 /lfs/h2/emc/ptmp/david.huber/rt_4130/COMROOT/C96C48mx500_S2SW_cyc_gfs_4130/gdas.20211221/00//analysis/atmos/gdas.t00z.gsistat
minmon_xtrct_reduct.pl has started
+ cpfs[3]'[' 3 -ne 2 ']' 
+ cpfs[4]echo 'This script requires two arguments: a source file and a destination file path.'
This script requires two arguments: a source file and a destination file path.
+ cpfs[5]exit 16
minmon_xtrct_reduct.pl has ended, return code = 0 
+ exglobal_atmos_vminmon.sh[55]rc_reduct=0
+ exglobal_atmos_vminmon.sh[56]echo 'rc_reduct = 0'
rc_reduct = 0 
+ exglobal_atmos_vminmon.sh[61]err=0

So any errors from the minimization package are not being handled correctly. I'll open up a follow-up issue for this.

DavidHuber-NOAA · 2025-10-23T20:24:50Z

Rerunning all 4 (gfs|gdas)_vminmon jobs for the C96C48mx500_S2SW_cyc_gfs test, the logs now show that cpfs handled the copying correctly and the Perl scripts ran as expected.

DavidHuber-NOAA · 2025-10-23T20:31:22Z

Opened #4171 as a follow-up issue to harden the minimization scripts.

aerorahul

lgtm.

Initial plan

e3a2e62

Copilot AI assigned Copilot and DavidHuber-NOAA Oct 6, 2025

Copilot started work on behalf of DavidHuber-NOAA October 6, 2025 17:55 View session

Replace cp with cpfs/cpreq for atomic copies to COM directories

9141f12

Co-authored-by: DavidHuber-NOAA <[email protected]>

Copilot AI changed the title ~~[WIP] Use cpfs for copies that should be atomic~~ Replace cp with cpfs/cpreq for atomic copies to COM directories Oct 6, 2025

Copilot AI requested a review from DavidHuber-NOAA October 6, 2025 18:08

Copilot finished work on behalf of DavidHuber-NOAA October 6, 2025 18:08

DavidHuber-NOAA unassigned Copilot Oct 6, 2025

DavidHuber-NOAA marked this pull request as ready for review October 6, 2025 18:26

DavidHuber-NOAA requested review from JessicaMeixner-NOAA, JiayiPeng-NOAA and sbanihash as code owners October 6, 2025 18:26

DavidHuber-NOAA requested a review from aerorahul October 6, 2025 18:26

JessicaMeixner-NOAA previously approved these changes Oct 6, 2025

View reviewed changes

aerorahul approved these changes Oct 6, 2025

View reviewed changes

DavidHuber-NOAA added 2 commits October 8, 2025 13:21

Merge branch 'develop' into copilot/fix-e87036ed-45fd-4baa-9f7e-6bceb…

2137a91

…dc16050

Merge branch 'develop' into copilot/fix-e87036ed-45fd-4baa-9f7e-6bceb…

cac08db

…dc16050

aerorahul requested changes Oct 20, 2025

View reviewed changes

ush/syndat_qctropcy.sh Outdated Show resolved Hide resolved

ush/syndat_qctropcy.sh Outdated Show resolved Hide resolved

ush/syndat_qctropcy.sh Outdated Show resolved Hide resolved

ush/syndat_qctropcy.sh Outdated Show resolved Hide resolved

DavidHuber-NOAA reviewed Oct 20, 2025

View reviewed changes

ush/tropcy_relocate.sh Outdated Show resolved Hide resolved

Apply suggestions from code review

96ec98a

Co-authored-by: Rahul Mahajan <[email protected]>

DavidHuber-NOAA dismissed JessicaMeixner-NOAA’s stale review via 96ec98a October 20, 2025 19:33

DavidHuber-NOAA added 2 commits October 22, 2025 15:32

Replace all other instances of cp with cpreq/fs

69ebd41

Merge remote-tracking branch 'origin/develop' into copilot/fix-e87036…

7917ae5

…ed-45fd-4baa-9f7e-6bcebdc16050

DavidHuber-NOAA requested a review from EdwardSafford-NOAA as a code owner October 22, 2025 19:32

EdwardSafford-NOAA previously approved these changes Oct 22, 2025

View reviewed changes

DavidHuber-NOAA added CI-Wcoss2-Running CI testing on WCOSS for this PR is in-progress CI-Wcoss2-Passed CI testing on WCOSS for this PR has completed successfully and removed CI-Wcoss2-Running CI testing on WCOSS for this PR is in-progress labels Oct 23, 2025

aerorahul requested changes Oct 23, 2025

View reviewed changes

Correct cpfs/cpreq usage

e95f278

DavidHuber-NOAA dismissed EdwardSafford-NOAA’s stale review via e95f278 October 23, 2025 20:14

DavidHuber-NOAA requested a review from aerorahul October 23, 2025 20:25

DavidHuber-NOAA mentioned this pull request Oct 23, 2025

vminmon jobs can fail silently #4171

Open

aerorahul approved these changes Oct 27, 2025

View reviewed changes

DavidHuber-NOAA merged commit f970bcd into develop Oct 27, 2025
8 checks passed

	cp "${COMOUT_OBS}/${RUN}.${cycle}.sgesprep_pre-relocate_pathname.${tmmark}" \
	"${COMOUT_OBS}/${RUN}.${cycle}.sgesprep_pathname.${tmmark}"

	cp "rel_inform1" "${COMOUT_OBS}/${RUN}.${cycle}.inform.relocate.${tmmark}"
	cp "tcvitals" "${COMOUT_OBS}/${RUN}.${cycle}.tcvitals.relocate.${tmmark}"

	cp current "${COMOUT_OBS}/${RUN}.${cycle}.syndata.tcvitals.${tmmark}"

	# Create the DBNet alert
	if [[ "${SENDDBN}" == "YES" ]]; then
	"${DBNROOT}/bin/dbn_alert" "MODEL" "GDAS_TCVITALS" "${job}" "${COMOUT_OBS}/${RUN}.${cycle}.syndata.tcvitals.${tmmark}"
	fi

	# Write JTWC/FNOC Tcvitals to /com path since not saved anywhere else
	cp fnoc "${COMOUT_OBS}/${RUN}.${cycle}.jtwc-fnoc.tcvitals.${tmmark}"

	system("cpfs -f $outfile $newfile");
	system("cpfs $outfile $newfile");

Uh oh!

Replace cp with cpfs/cpreq for atomic copies to COM directories #4130

Replace cp with cpfs/cpreq for atomic copies to COM directories #4130

Uh oh!

Conversation

Copilot AI commented Oct 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Problem

Solution

Changes

Testing

Impact

What are the requirements for the new functionality?

Acceptance Criteria

Suggest a solution (optional)

Comments on the Issue (you are @copilot in this section)

Uh oh!

JessicaMeixner-NOAA left a comment

Choose a reason for hiding this comment

Uh oh!

aerorahul left a comment

Choose a reason for hiding this comment

Uh oh!

aerorahul left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

EdwardSafford-NOAA left a comment

Choose a reason for hiding this comment

Uh oh!

DavidHuber-NOAA commented Oct 23, 2025

Uh oh!

DavidHuber-NOAA commented Oct 23, 2025

Uh oh!

aerorahul left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

DavidHuber-NOAA commented Oct 23, 2025

Uh oh!

DavidHuber-NOAA commented Oct 23, 2025

Uh oh!

DavidHuber-NOAA commented Oct 23, 2025

Uh oh!

aerorahul left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Copilot AI commented Oct 6, 2025 •

edited

Loading