[Bug]: zstash update --non-blocking not behaving as expected

[golaz]

What happened?

I'm testing out E3SM Unified 1.11.0rc6 on Chrysalis with Globus trasfer to NERSC HPSS

zstash create ... and zstash create --non-blocking ... are now behaving as expected. Thank you for correcting this previous problem.

However, zstash update --non-blocking ... does not behave as expected. Instead, it operates as in blocking mode. I wonder if command line arguments are not being captured or passed properly.

What machine were you running on?

Chrysalis

Environment

source /lcrc/soft/climate/e3sm-unified/test_e3sm_unified_1.11.0rc6_chrysalis.sh

Minimal Complete Verifiable Example (MCVE)

Relevant log output

Anything else we need to know?

No response

[forsyth2]

I wonder if command line arguments are not being captured or passed properly.

@TonyB9000 this is almost certainly the case. I don't know how we missed it, but update.py isn't even modified in https://github.com/E3SM-Project/zstash/pull/355/files. I guess we were too focused on zstash create?

[TonyB9000]

@forsyth2 @golaz That is very possible. I only considered the "zstash create" pathway. In order to push the "--non-blocking" flag down to where it was needed, I added "non_blocking=args.non_blocking" to all call-chain arguments. I did not consider that "update" was an independent workflow. I can look at "update", the changes should be (essentially) identical,

[TonyB9000]

@forsyth2 I am curious why we have "non_blocking" implies "keep = True" (in "create"). I could see it the other way around (blocking implies "keep = False"), as the stated reason for blocking is to save space. The implication is missing in "update".

I think I see a problem with non_blocking and "keep = False". While in non_blocking mode, we continue to create new tar files (4.tar, 5.tar, 6.tar,...) and with each new tar file, we "attempt" to transfer. But if the previous transfer is still active, we simply add the new tarfile path to the list maintained by the transfer structure and then return with status ACTIVE. However, if the submission of (say) "6.tar" found that globus had completed the previous transfer, the "stored list of tar files" (say, 4.tar, 5.tar, 6.tar) are then transferred. When we return from that transfer (status = SUCCEEDED), and "keep = False", we delete only the last tarfile we had passed in (6.tar). The files 4.tar and 5.tar are not deleted, as they were passed in earlier.

We need the transfer to return to us the entire LIST of files that were transferred, so that they can all be deleted when keep = False.

(This is not a concern with blocking = True, since only ONE file gets transferred, so it must be the one to delete.)

[TonyB9000]

@forsyth2 @golaz Is there an urgency to this fix? I suspect it will take a bit of work, as the flow is not quite 1-for-1 between "create" and "update". I will need to distill the flow to understand the difference and make updates. Perhaps by Friday I can be testing the changes. I will conduct testing as I had with "create", using the same endpoint as source and destination for speed and convenience.

[forsyth2]

Perhaps by Friday I can be testing the changes.

I think that should be ok. We're aiming for releasing E3SM Unified 1.11.0 by next Friday (2/21), so ideally we have this fix merged by early next week. If that's feasible.

[TonyB9000]

@forsyth2 OK, three issues:

(1), I have made the only change I believe is needed. This was to add "non_blocking=args.non_blocking" to the add_files() arguments in "update.py", as it had been in "create.py". Nothing below that point (hpss_put, hpss_transfer, globus_transfer) seems to care if this was an "create" or "update", so subsequent behavior will be identical.

(2) I am unsure how to test this change, I assume I can first use "create" to make a small remote archive, and then issue an "update" with lots of tar-files to create/update, and see if they "race-ahead" of the transfers (so subsequent transfers involve multiple tar-files).

(3) [side-issue], You have not responded to my comment regarding "non_blocking" + "keep = False", which I believe will fail to delete all but the last local file of a transfer, rather than all of them. Should I test for this as well, and address the issue?

[forsyth2]

@TonyB9000

(1) Ok, I was thinking at a certain point, they would have identical behavior, so that seems logical to me.

(2) So, the problem is zstash update is now always blocking, so we want to make sure zstash update --non-blocking will create tar files while waiting for transfer. So yeah what you suggest is probably the best way. (These sorts of race-condition/parallelization tasks are very hard to test!)

(3) Your logic in #361 (comment) makes sense to me. I can't recall why it was implemented that way; I would need to spend more time looking at the code. I'll see if I can fit that in. My first thought is maybe we wanted to keep tars as a sort of safety check -- e.g., make sure everything actually gets transferred?

[TonyB9000]

Right. I can understand why keep=True should be automatic when hpss=NONE, since the cache is the only thing you want to create (or keep for future extraction). Otherwise it would be natural to have keep=False.

But in any case, if the user wants keep=False, the system should ensure that ALL tar-files that are transferred (and ONLY those) are deleted locally. I will attempt to test that behavior. It is tricky to make happen. I need to set "maxsize" to about 100K to do rapid testing.

[forsyth2]

Well, using the Blame functionality on https://github.com/E3SM-Project/zstash/blob/main/zstash/create.py, we see that that code block was added in #154.

From #154 (comment):

If --keep is not specified, by default each tar file is deleted immediately, after the file is put/transferred to the storage. Tar files are created, put/transferred, and deleted sequentially.

[TonyB9000]

@forsyth2 I checked on those links, but I cannot determine which "code block" you refer to. With non-blocking, globus can transfer multiple tar-files in one pass. Upon completion, I see no provision for their removal (keep or not keep). Can you point me in the direction of the code that implements the "deleted immediately". I can only find the spot where I (recently) forced os.remove in hpss.py following the "successful" globus_transfer return under "blocking" (aside from "extract.py", which is another matter.)

BTW: The code on line 154 was just me expanding on the help-text describing the non-blocking argument.

[forsyth2]

I checked on those links, but I cannot determine which "code block" you refer to.

The code block setting keep to True:

    if args.non_blocking:
        args.keep = True

[TonyB9000]

@forsyth2 I see. I meant, where historically (in create or update) were ANY tarfiles "not kept" when "--keep" was not specified.

[forsyth2]

Upon completion, I see no provision for their removal (keep or not keep).

Hmm, that's going to require a deeper dive into the code; I don't think I can get to that today.

where historically (in create or update) were ANY tarfiles "not kept" when "--keep" was not specified.

Hmm, --keep goes back quite far -- to before I joined the project even. If --keep was NOT specified, tarfiles got deleted (i.e., not "kept") after transferring to HPSS, but if --keep was specified, then the tar files wouldn't be deleted.

[TonyB9000]

@forsyth2 I recall complaining - perhaps 3-4 years ago (when we used zstash cache files AS the local long-term archives (on /p/user_pub) that when I extracted some files to our warehouse, the corresponding tar files disappeared. Around that time, I think you enabled "hpss=none" to set "keep=True", But all of that is on the extraction-side. I see no place where keep=True/False affected the disposition of tarfiles on "create" (or on "update"), except where I added it a few weeks ago for (keep==False AND non-blocking=False) in hpss.py lines 115-118.

Thing is, I am trying to make sure that any "remove" logic I add will not interfere with any existing "remove" logic that depends upon "not keep", but I cannot find any. I must assume there is none, and even without "--keep" the tar files on "update" are not deleted.