mirror of
https://github.com/owncloud/ocis.git
synced 2026-10-23 23:29:02 +03:00
fix: purge revisions with whole-second timestamps (#13036)
Fixes #11167 Co-authored-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>
This commit is contained in:
Matt Van Horn
GitHub
13
changelog/unreleased/bugfix-purge-whole-second-revisions.md
Normal file
13
changelog/unreleased/bugfix-purge-whole-second-revisions.md
Normal file
@@ -0,0 +1,13 @@
|
||||
Bugfix: Purge revisions stored with whole-second timestamps
|
||||
|
||||
The revisions purge command reported a clean storage when revisions were still
|
||||
present. Revision filenames are the previous mtime formatted with
|
||||
time.RFC3339Nano, which omits fractional seconds on a whole second. Names such
|
||||
as `.REV.1601-01-01T00:00:00Z` never matched the purge pattern, so those
|
||||
revisions stayed on disk.
|
||||
|
||||
The matcher now treats the decimal point and fractional digits as optional.
|
||||
Whole-second revisions and their `.mpk` and `.mlock` companions are discovered
|
||||
and purged along with revisions that still carry a fractional timestamp.
|
||||
|
||||
https://github.com/owncloud/ocis/issues/11167
|
||||
@@ -3,7 +3,9 @@ package revisions
|
||||
import (
|
||||
"os"
|
||||
"path/filepath"
|
||||
"sort"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/owncloud/reva/v2/pkg/storage/utils/decomposedfs/node"
|
||||
"github.com/shamaton/msgpack/v2"
|
||||
@@ -53,3 +55,121 @@ func TestPurgeRevisionsDeletesBlobWithSpaceID(t *testing.T) {
|
||||
require.Equal(t, blobID, bs.deleted[0].BlobID)
|
||||
require.Equal(t, spaceID, bs.deleted[0].SpaceID, "SpaceID must be passed to the blobstore, otherwise the blob is orphaned")
|
||||
}
|
||||
|
||||
// TestPurgeRevisionsMixedWholeSecondAndFractional purges a decomposed tree that
|
||||
// mixes whole-second and fractional revision triplets. Each triplet is the
|
||||
// revision node plus its .mpk and .mlock companion: three files, one blob, and
|
||||
// one revision. Dry-run keeps every file and does not call the blobstore.
|
||||
func TestPurgeRevisionsMixedWholeSecondAndFractional(t *testing.T) {
|
||||
for _, d := range revisionDiscoverers() {
|
||||
t.Run(d.name, func(t *testing.T) {
|
||||
for _, dryRun := range []bool{true, false} {
|
||||
name := "delete"
|
||||
if dryRun {
|
||||
name = "dry-run"
|
||||
}
|
||||
t.Run(name, func(t *testing.T) {
|
||||
tree := newDecomposedRevisionTree(t)
|
||||
bs := &recordingBlobstore{}
|
||||
|
||||
files, blobs, revs := PurgeRevisions(d.find(tree.root), bs, dryRun, false)
|
||||
|
||||
require.Equal(t, tree.revisions*3, files, "files")
|
||||
require.Equal(t, tree.revisions, blobs, "blobs")
|
||||
require.Equal(t, tree.revisions, revs, "revisions")
|
||||
requirePresent(t, tree.preservedPaths)
|
||||
|
||||
if dryRun {
|
||||
require.Empty(t, bs.deleted)
|
||||
requirePresent(t, tree.revisionPaths)
|
||||
return
|
||||
}
|
||||
|
||||
require.Equal(t, tree.blobs, deletedBlobs(bs))
|
||||
requireAbsent(t, tree.revisionPaths)
|
||||
})
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestPurgeRevisionsRetainsInvalidWholeSecondMessagePack keeps a whole-second
|
||||
// .mpk whose metadata is not MessagePack. A sibling whole-second triplet is
|
||||
// purged in the same call, which shows the retained file was recognized as a
|
||||
// revision and then left in place by the existing read error path. That path
|
||||
// also skips the blob delete.
|
||||
func TestPurgeRevisionsRetainsInvalidWholeSecondMessagePack(t *testing.T) {
|
||||
root := t.TempDir()
|
||||
nodePath := decomposedNodePath(root, revisionFixtureSpaceID, revisionFixtureNodeID)
|
||||
whole := time.Date(2024, 5, 22, 7, 32, 53, 0, time.UTC).UTC().Format(time.RFC3339Nano)
|
||||
ancient := time.Date(1601, 1, 1, 0, 0, 0, 0, time.UTC).UTC().Format(time.RFC3339Nano)
|
||||
require.Equal(t, "2024-05-22T07:32:53Z", whole)
|
||||
require.Equal(t, "1601-01-01T00:00:00Z", ancient)
|
||||
|
||||
validBase := nodePath + ".REV." + whole
|
||||
badMPK := nodePath + ".REV." + ancient + ".mpk"
|
||||
currentMPK := nodePath + ".mpk"
|
||||
|
||||
writeFixtureFile(t, nodePath, nil)
|
||||
writeFixtureFile(t, currentMPK, revisionBlobMPK(t, "blob-current"))
|
||||
writeFixtureFile(t, validBase, nil)
|
||||
writeFixtureFile(t, validBase+".mpk", revisionBlobMPK(t, "blob-valid"))
|
||||
writeFixtureFile(t, validBase+".mlock", nil)
|
||||
writeFixtureFile(t, badMPK, []byte("not-msgpack"))
|
||||
|
||||
nodes := make(chan string, 6)
|
||||
for _, p := range []string{
|
||||
badMPK,
|
||||
validBase,
|
||||
validBase + ".mpk",
|
||||
validBase + ".mlock",
|
||||
nodePath,
|
||||
currentMPK,
|
||||
} {
|
||||
nodes <- p
|
||||
}
|
||||
close(nodes)
|
||||
|
||||
bs := &recordingBlobstore{}
|
||||
files, blobs, revs := PurgeRevisions(nodes, bs, false, false)
|
||||
|
||||
require.Equal(t, 3, files, "files")
|
||||
require.Equal(t, 1, blobs, "blobs")
|
||||
require.Equal(t, 1, revs, "revisions")
|
||||
require.Equal(t, []purgedBlob{{
|
||||
spaceID: revisionFixtureSpaceID,
|
||||
blobID: "blob-valid",
|
||||
}}, deletedBlobs(bs))
|
||||
requireAbsent(t, []string{validBase, validBase + ".mpk", validBase + ".mlock"})
|
||||
requirePresent(t, []string{badMPK, nodePath, currentMPK})
|
||||
}
|
||||
|
||||
func deletedBlobs(bs *recordingBlobstore) []purgedBlob {
|
||||
got := make([]purgedBlob, 0, len(bs.deleted))
|
||||
for _, n := range bs.deleted {
|
||||
got = append(got, purgedBlob{spaceID: n.SpaceID, blobID: n.BlobID})
|
||||
}
|
||||
sort.Slice(got, func(i, j int) bool {
|
||||
if got[i].blobID != got[j].blobID {
|
||||
return got[i].blobID < got[j].blobID
|
||||
}
|
||||
return got[i].spaceID < got[j].spaceID
|
||||
})
|
||||
return got
|
||||
}
|
||||
|
||||
func requirePresent(t *testing.T, paths []string) {
|
||||
t.Helper()
|
||||
for _, p := range paths {
|
||||
_, err := os.Stat(p)
|
||||
require.NoError(t, err, p)
|
||||
}
|
||||
}
|
||||
|
||||
func requireAbsent(t *testing.T, paths []string) {
|
||||
t.Helper()
|
||||
for _, p := range paths {
|
||||
_, err := os.Stat(p)
|
||||
require.True(t, os.IsNotExist(err), "%s still present: %v", p, err)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -18,7 +18,13 @@ var (
|
||||
// 9113a718-8285-4b32-9042-f930f1a58ac2.REV.2024-05-22T07:32:53.89969726Z
|
||||
// 9113a718-8285-4b32-9042-f930f1a58ac2.REV.2024-05-22T07:32:53.89969726Z.mpk
|
||||
// 9113a718-8285-4b32-9042-f930f1a58ac2.REV.2024-05-22T07:32:53.89969726Z.mlock
|
||||
_versionRegex = regexp.MustCompile(`\.REV\.[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2}\.[0-9]+Z*`)
|
||||
// 9113a718-8285-4b32-9042-f930f1a58ac2.REV.2024-05-22T07:32:53Z
|
||||
// 9113a718-8285-4b32-9042-f930f1a58ac2.REV.1601-01-01T00:00:00Z
|
||||
//
|
||||
// Fractional seconds are optional because decomposedfs formats the revision
|
||||
// id with time.RFC3339Nano, which drops them when the mtime is a whole
|
||||
// second. The pattern stays unanchored so .mpk and .mlock companions match.
|
||||
_versionRegex = regexp.MustCompile(`\.REV\.[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2}(\.[0-9]+)?Z*`)
|
||||
)
|
||||
|
||||
// DelBlobstore is the interface for a blobstore that can delete blobs.
|
||||
|
||||
@@ -5,11 +5,14 @@ import (
|
||||
"io/fs"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"sort"
|
||||
"strconv"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/google/uuid"
|
||||
"github.com/owncloud/reva/v2/pkg/storage/utils/decomposedfs/lookup"
|
||||
"github.com/shamaton/msgpack/v2"
|
||||
"github.com/test-go/testify/require"
|
||||
)
|
||||
|
||||
@@ -168,3 +171,208 @@ func initialize(numNodes int, numRevisions int) string {
|
||||
}
|
||||
return base
|
||||
}
|
||||
|
||||
// revisionTimestampCase is a timestamp decomposedfs can write as a revision name.
|
||||
// The stamp comes from time.Time.UTC().Format(time.RFC3339Nano), which omits the
|
||||
// fractional part when the mtime falls on a whole second.
|
||||
type revisionTimestampCase struct {
|
||||
name string
|
||||
at time.Time
|
||||
want string
|
||||
blobID string
|
||||
}
|
||||
|
||||
func revisionTimestampCases() []revisionTimestampCase {
|
||||
return []revisionTimestampCase{
|
||||
{
|
||||
name: "contemporary whole second",
|
||||
at: time.Date(2024, 5, 22, 7, 32, 53, 0, time.UTC),
|
||||
want: "2024-05-22T07:32:53Z",
|
||||
blobID: "blob-contemporary",
|
||||
},
|
||||
{
|
||||
name: "year 1601 whole second",
|
||||
at: time.Date(1601, 1, 1, 0, 0, 0, 0, time.UTC),
|
||||
want: "1601-01-01T00:00:00Z",
|
||||
blobID: "blob-1601",
|
||||
},
|
||||
{
|
||||
name: "one fractional digit",
|
||||
at: time.Date(2024, 6, 1, 12, 0, 0, 100000000, time.UTC),
|
||||
want: "2024-06-01T12:00:00.1Z",
|
||||
blobID: "blob-frac-1",
|
||||
},
|
||||
{
|
||||
name: "nine fractional digits",
|
||||
at: time.Date(2024, 6, 1, 12, 0, 1, 123456789, time.UTC),
|
||||
want: "2024-06-01T12:00:01.123456789Z",
|
||||
blobID: "blob-frac-9",
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
func TestRevisionTimestampsFollowWriterContract(t *testing.T) {
|
||||
for _, c := range revisionTimestampCases() {
|
||||
t.Run(c.name, func(t *testing.T) {
|
||||
require.Equal(t, c.want, c.at.UTC().Format(time.RFC3339Nano))
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
const (
|
||||
revisionFixtureSpaceID = "8f638374-6ea8-4f0d-80c4-66d9b49830a5"
|
||||
revisionFixtureNodeID = "aa111111-1111-4111-8111-111111111111"
|
||||
revisionFixtureOtherID = "bb222222-2222-4222-8222-222222222222"
|
||||
)
|
||||
|
||||
// purgedBlob is the SpaceID and BlobID pair passed to the blobstore.
|
||||
type purgedBlob struct {
|
||||
spaceID string
|
||||
blobID string
|
||||
}
|
||||
|
||||
// decomposedRevisionTree is a temporary decomposedfs tree:
|
||||
//
|
||||
// <root>/spaces/<ss>/<space>/nodes/<n0>/<n1>/<n2>/<n3>/<node>
|
||||
// <root>/spaces/<ss>/<space>/nodes/<n0>/<n1>/<n2>/<n3>/<node>.REV.<RFC3339Nano>
|
||||
//
|
||||
// plus the .mpk and .mlock companion of every revision.
|
||||
type decomposedRevisionTree struct {
|
||||
root string
|
||||
revisions int
|
||||
revisionPaths []string
|
||||
preservedPaths []string
|
||||
blobs []purgedBlob
|
||||
}
|
||||
|
||||
type revisionDiscoverer struct {
|
||||
name string
|
||||
find func(root string) <-chan string
|
||||
}
|
||||
|
||||
// revisionDiscoverers matches the mechanisms revisions purge can run, plus Walk.
|
||||
func revisionDiscoverers() []revisionDiscoverer {
|
||||
return []revisionDiscoverer{
|
||||
{
|
||||
name: "glob",
|
||||
find: func(root string) <-chan string {
|
||||
return Glob(filepath.Join(root, "spaces", "*", "*", "nodes", "*", "*", "*", "*", "*"))
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "globWorkers",
|
||||
find: func(root string) <-chan string {
|
||||
return GlobWorkers(filepath.Join(root, "spaces", "*", "*", "nodes"), "/*", "/*/*/*/*")
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "walk",
|
||||
find: func(root string) <-chan string {
|
||||
return Walk(root)
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "list",
|
||||
find: func(root string) <-chan string {
|
||||
return List(filepath.Join(root, "spaces"), 10)
|
||||
},
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
func decomposedNodePath(root, spaceID, nodeID string) string {
|
||||
return filepath.Join(root, "spaces", lookup.Pathify(spaceID, 1, 2), "nodes", lookup.Pathify(nodeID, 4, 2))
|
||||
}
|
||||
|
||||
func revisionBlobMPK(t *testing.T, blobID string) []byte {
|
||||
t.Helper()
|
||||
b, err := msgpack.Marshal(map[string][]byte{"user.ocis.blobid": []byte(blobID)})
|
||||
require.NoError(t, err)
|
||||
return b
|
||||
}
|
||||
|
||||
func writeFixtureFile(t *testing.T, path string, content []byte) {
|
||||
t.Helper()
|
||||
require.NoError(t, os.MkdirAll(filepath.Dir(path), 0o755))
|
||||
require.NoError(t, os.WriteFile(path, content, 0o644))
|
||||
}
|
||||
|
||||
// newDecomposedRevisionTree builds one space with a current node, an unrelated
|
||||
// node, ordinary files, and one revision triplet per writer-contract timestamp.
|
||||
func newDecomposedRevisionTree(t *testing.T) decomposedRevisionTree {
|
||||
t.Helper()
|
||||
|
||||
root := t.TempDir()
|
||||
nodePath := decomposedNodePath(root, revisionFixtureSpaceID, revisionFixtureNodeID)
|
||||
otherPath := decomposedNodePath(root, revisionFixtureSpaceID, revisionFixtureOtherID)
|
||||
|
||||
writeFixtureFile(t, nodePath, nil)
|
||||
writeFixtureFile(t, nodePath+".mpk", revisionBlobMPK(t, "blob-current"))
|
||||
writeFixtureFile(t, otherPath, nil)
|
||||
|
||||
notes := filepath.Join(filepath.Dir(nodePath), "notes.txt")
|
||||
writeFixtureFile(t, notes, []byte("unrelated"))
|
||||
trashStamp := time.Date(2024, 5, 22, 7, 32, 53, 0, time.UTC).UTC().Format(time.RFC3339Nano)
|
||||
trash := nodePath + ".T." + trashStamp
|
||||
writeFixtureFile(t, trash, nil)
|
||||
readme := filepath.Join(root, "README.txt")
|
||||
writeFixtureFile(t, readme, []byte("unrelated"))
|
||||
ignored := filepath.Join(root, "spaces", "ignored.txt")
|
||||
writeFixtureFile(t, ignored, []byte("unrelated"))
|
||||
|
||||
tree := decomposedRevisionTree{
|
||||
root: root,
|
||||
preservedPaths: []string{
|
||||
nodePath,
|
||||
nodePath + ".mpk",
|
||||
otherPath,
|
||||
notes,
|
||||
trash,
|
||||
readme,
|
||||
ignored,
|
||||
},
|
||||
}
|
||||
|
||||
for _, c := range revisionTimestampCases() {
|
||||
stamp := c.at.UTC().Format(time.RFC3339Nano)
|
||||
base := nodePath + ".REV." + stamp
|
||||
writeFixtureFile(t, base, nil)
|
||||
writeFixtureFile(t, base+".mpk", revisionBlobMPK(t, c.blobID))
|
||||
writeFixtureFile(t, base+".mlock", nil)
|
||||
|
||||
tree.revisions++
|
||||
tree.revisionPaths = append(tree.revisionPaths, base, base+".mpk", base+".mlock")
|
||||
tree.blobs = append(tree.blobs, purgedBlob{spaceID: revisionFixtureSpaceID, blobID: c.blobID})
|
||||
}
|
||||
|
||||
sort.Strings(tree.revisionPaths)
|
||||
sort.Slice(tree.blobs, func(i, j int) bool {
|
||||
if tree.blobs[i].blobID != tree.blobs[j].blobID {
|
||||
return tree.blobs[i].blobID < tree.blobs[j].blobID
|
||||
}
|
||||
return tree.blobs[i].spaceID < tree.blobs[j].spaceID
|
||||
})
|
||||
return tree
|
||||
}
|
||||
|
||||
// drainRevisionPaths reads a discovery channel to completion. GlobWorkers and
|
||||
// List send from several goroutines, so callers compare the sorted result.
|
||||
func drainRevisionPaths(ch <-chan string) []string {
|
||||
paths := make([]string, 0)
|
||||
for p := range ch {
|
||||
paths = append(paths, p)
|
||||
}
|
||||
sort.Strings(paths)
|
||||
return paths
|
||||
}
|
||||
|
||||
func TestDiscoverRevisionsWholeSecondAndFractional(t *testing.T) {
|
||||
tree := newDecomposedRevisionTree(t)
|
||||
|
||||
for _, d := range revisionDiscoverers() {
|
||||
t.Run(d.name, func(t *testing.T) {
|
||||
got := drainRevisionPaths(d.find(tree.root))
|
||||
require.Equal(t, tree.revisionPaths, got)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user