have you experimented with more radical splitting factors? 4? 8? 16? 32?

The counter seems to be not correlated at all with the ExecuteSafe method. I'd replace lock(_lock) with Interlocked.Add for both, Increment and Decrement methods. Unless in Decrement you want to be sure that no ExecuteSafe is happening at the moment

So this is the one that allows to pass on all the actions and is treated as the default. Some comment would be nice I believe.

What @LukaszRozmej said above. What if you call IncrementStorageRangeLockIfExists you get no lockInfo and immediately it's followed by IncrementStorageRangeLock that does the update? Does it mean that something is broken here?

Why key is needed here? Should it not be captured in the ctor of the lock, so that we just Decrement(bool removeFromOwner). You get the lock by key so the key should not be needed here I think.

Why increment returns the lock object? My feeling is that it's to satisfy the ConcurrentDictionary. I'd keep it simple and make it void.

Why do we need to take care of whether a lock should be removed or not? Should it not be the case that when the counter hits 0, we always remove?

I'd just use a sharded lock to avoid such complications.
Also, you can just use _pivot.UpdatedStorages.Add(pathWithAccount.Path); to the splitted storage so that healing will fix it. But it limits the number of storage that can be splitted though.

Main change here!

The main point of having this complexity around locking was to lock only when necessary - that is when split is done and there us a chance of actually having parallel processing of storage ranges for same account. If the overhead is negligible, I'm all for it - it's the simplest.

On having a conversation with @Scooletz yesterday, there could be a way to optimize it easily, if there is significant overhead. Stitching only happens when there are boundary proof nodes - so can lock only if that happens.

lock is super cheap compared to I/O so locking even when not necessary shouldn't degrade performance. And you still paid for ConcurrentDictionary in your solution, so it might be comparable

Should be fine. That said, this code path is run quite a lot, might wannna double check if the lock is significant.

If there is no contention then lock is very fast, if there is contention, well we still have to lock. Keep in mind in the previous solution we locked on potential contention while also accessed ConcurrentDictionary at least one time even with no contention. https://stackoverflow.com/a/72479230

I don't like the account based locking and using the object that might be elsewhere locked or used for different purposes. Locking should have specific scoping and should be reasoned about locally.

Another take is that this lock is needed, correct me @damian-orzechowski if I'm wrong, only if we know that there's no pending requests. We could extract the check from StitchBoundaries and assert it explicitly before the lock.

we could create artificial object and pass it around, but I don't really see the point other than increasing the memory pressure and complicating the code. We added comments! :)

Done is probably better than perfect here.
If we want to refactor is to have parent request directly available when child request is being created. So the connection is obvious and not hidden in multiple method calls.

Done some refactoring here #7930. The problem is that passing around the whole StorageRange request means also passing an index of the actual account processed (accounts are processed one at a time), so it doesn't look very nice.