vfs: MemFS Link erroneously shares refcount

[tbg]

vfs.Link¹ reuses the same *memNode for the old and new location. In particular, this means that its refs field² is now shared. This means that a hardlinked file cannot be deleted if the original is currently open (or vice versa):

func TestFoo(t *testing.T) {
	fs := vfs.NewMem()
	{
		// Write SST somewhere.
		f, err := fs.Create("file.txt")
		require.NoError(t, err)
		_, err = f.Write([]byte("hi"))
		require.NoError(t, err)
		require.NoError(t, f.Close())
	}
	// Ingest SST into LSM.
	require.NoError(t, fs.Link("file.txt", "file2.txt"))
	// Pebble does something with the SST, like compact it. Won't close it until test ends.
	{
		f, err := fs.Open("file2.txt")
		require.NoError(t, err)
		defer func() {
			require.NoError(t, f.Close())
		}()
	}

	// Try to remove original file.
	require.NoError(t, fs.Remove("file.txt")) // <-- will fail: "invalid argument"
}

We should not share the atomic. The hard-link should get its own atomic counter.

When this is fixed, this work-around can probably be removed:

pebble/ingest.go

Lines 282 to 298 in 5a2941f

	if _, ok := opts.FS.(*vfs.MemFS); ok && opts.DebugCheck != nil {
	// The combination of MemFS+Ingest+DebugCheck produces awkwardness around
	// the subsequent deletion of files. The problem is that MemFS implements
	// the Windows semantics of disallowing removal of an open file. This is
	// desirable because it helps catch bugs where we violate the
	// requirements of the Windows semantics. The normal practice for Ingest
	// is for the caller to remove the source files after the ingest
	// completes successfully. Unfortunately, Options.DebugCheck causes
	// ingest to run DB.CheckLevels() before the ingest finishes, and
	// DB.CheckLevels() populates the table cache with the newly ingested
	// files.
	//
	// The combination of MemFS+Ingest+DebugCheck is primarily used in
	// tests. As a workaround, disable hard linking this combination
	// occurs. See https://github.com/cockroachdb/pebble/issues/495.
	err = vfs.Copy(fs, paths[i], target)
	} else {

Found while investigating a CRDB log truncation test³.

See also #1236

Footnotes

1. https://github.com/cockroachdb/pebble/blob/b82ab1300c3a5320a68b1607df271d310b737086/vfs/mem_fs.go#L208-L248 ↩

2. https://github.com/cockroachdb/pebble/blob/b82ab1300c3a5320a68b1607df271d310b737086/vfs/mem_fs.go#L498 ↩

3. https://github.com/cockroachdb/cockroach/pull/89632#issuecomment-1293415352 ↩