Heads up @snh. Since you last looked at this, I've switched out the use of parallel in favour of xargs -P64 for all of the finalize steps (with a few force pushes to hide my mess 😊).

My testing shows this works just as well, but I'd appreciate it if you could specifically double check me here - I'm not sure why you went for parallel over xargs when you first implemented this. 🙇

@lildude I'm concerned that 64 will be a little high in some circumstances, as there are a number of Cluster customers who restore to DR and test clusters that are quite light-on resources wise.

From my quick testing, each dgit-cluster-restore-finalize, without having been feed any routes yet, uses 76MB of PSS memory, which adds up to close to 5GB for 64 of these.

For this reason I'm quite a fan of the approach we were using with parallel to scale the number of processes based on the CPU count, which is often a good indicator of the resources a node has available to it.

Did you have a chance to try and stub parallel? As this runs on the appliance itself, which already had parallel, the dependency on parallel has never really been an issue until now.

I'm concerned that 64 will be a little high in some circumstances, as there are a number of Cluster customers who restore to DR and test clusters that are quite light-on resources wise.

This would only affect customers who actually have up to 64 CPUs on their node as -P is for maximum number of processes. From the man page (emphasis is mine): "Run up to max-procs processes at a time;".

I'd hope if they're going to assign that many CPUs, they'd allocate an equivalently large amount of memory too as they'd hit other problems long before they hit backup-utils related problems.

For this reason I'm quite a fan of the approach we were using with parallel to scale the number of processes based on the CPU count

I'd have liked to do the same with xargs by not specifying a number or using 0 to use all available procs, but xargs on macOS requires a value > 0 and we still test on macOS, hence I've gone for a rather large value to mimic the same behaviour.

Did you have a chance to try and stub parallel?

I did, but this turned out to be a lot more complex than anticipated and I soon found myself effectively writing a passthrough version of parallel that stripped the args so we could run the subsequent commands. This adds unnecessary complexity and potential tech debt, just for testing, when a simple and just as effective alternate solution works for both actual use and testing.

As this runs on the appliance itself, which already had parallel, the dependency on parallel has never really been an issue until now.

That's because we were never testing this behaviour within backup-utils itself so never encountered any problems until now. We don't use parallel for anything else within GitHub Enterprise either.

As discussed in Slack, I'm wrong here and misunderstood the behaviour. Will revert that change.