Why does check_deleted_edge only checks for 2 edge histories?

[hbina]

Hi, just a passerby reading through the commits and trying to understand how this all works.
I was reading this commit 636f10e and the surrounding code, then I noticed that the check_deleted_edge have the same issue that the commit was trying fix.

ternfs/kmod/metadata.c

Lines 305 to 324 in 636f10e

	if (
	likely(ctx.err == 0) &&
	i == 1 && // this is the old edge
	(
	edge_current.x ||
	TERNFS_GET_EXTRA(edge_target.x) != owned ||
	TERNFS_GET_EXTRA_ID(edge_target.x) != target ||
	edge_creation_time.x != creation_time
	)
	) {
	good = false;
	}
	if (
	likely(ctx.err == 0) &&
	i == 0 && // this is the deleted edge
	TERNFS_GET_EXTRA_ID(edge_target.x) != 0
	) {
	good = false;
	}
	}

In particular, the check assumes that you are the only person trying to delete the edge.
Therefore, you assume that you made the last 2 changes.
I think deletion have the same idempotent property as renaming.
That is, a delete is successful if the latest edge is a delete and my target_id exists somewhere in the history.

P.S. I might have botched the terminologies completely.

[bitonic]

You are correct, it's equally relevant. These failures are very rare and do not compromise the integrity of the filesystem, so we addressed them on a need-to basis, and some new usage pattern happened to trigger this rename case, but covering the delete case as well would be good.

[mcrnic]

You are right and here is some additional info why it was not added for deletions.
Even with the solution in this commit this is still racy as garbage collector can get to snapshot edges before this check. There are no plans at the moment to address it or add this check to deletions.
The chance of deletions race is actually much higher as we have high churn folders with ActiveDeleteAfterVersions directory policy set to 0. This makes snapshot edges immediately available for collection by gc and we also have a feature where this collection is done from the client (kernel module) directly so snapshots are not really a reliable check for idempotency but they are good enought for now.