X-Git-Url: https://git.arvados.org/arvados.git/blobdiff_plain/35ea47144857dc16ab8b6b8a272a87af6d50cb88..ffe1fe1c77743147ee82aacdc50edde3672cd748:/services/keepstore/s3_volume_test.go

diff --git a/services/keepstore/s3_volume_test.go b/services/keepstore/s3_volume_test.go
index a40b84f88d..6ba390426f 100644
--- a/services/keepstore/s3_volume_test.go
+++ b/services/keepstore/s3_volume_test.go
@@ -122,7 +122,7 @@ func (s *StubbedS3Suite) TestBackendStates(c *check.C) {
 	v := NewTestableS3Volume(c, 5*time.Minute, false, 2)
 	var none time.Time
 
-	stubKey := func(t time.Time, key string, data []byte) {
+	putS3Obj := func(t time.Time, key string, data []byte) {
 		if t == none {
 			return
 		}
@@ -132,21 +132,23 @@ func (s *StubbedS3Suite) TestBackendStates(c *check.C) {
 
 	t0 := time.Now()
 	nextKey := 0
-	for _, test := range []struct {
+	for _, scenario := range []struct {
 		label               string
-		data                time.Time
-		recent              time.Time
-		trash               time.Time
+		dataT               time.Time
+		recentT             time.Time
+		trashT              time.Time
 		canGet              bool
 		canTrash            bool
 		canGetAfterTrash    bool
 		canUntrash          bool
 		haveTrashAfterEmpty bool
+		freshAfterEmpty     bool
 	}{
 		{
 			"No related objects",
 			none, none, none,
-			false, false, false, false, false},
+			false, false, false, false, false, false,
+		},
 		{
 			// Stored by older version, or there was a
 			// race between EmptyTrash and Put: Trash is a
@@ -154,97 +156,154 @@ func (s *StubbedS3Suite) TestBackendStates(c *check.C) {
 			// old
 			"No recent/X",
 			t0.Add(-48 * time.Hour), none, none,
-			true, true, true, false, false},
+			true, true, true, false, false, false,
+		},
 		{
-			"Not trash; old enough to trash",
+			"Not trash, but old enough to be eligible for trash",
 			t0.Add(-24 * time.Hour), t0.Add(-2 * time.Hour), none,
-			true, true, false, false, false},
+			true, true, false, false, false, false,
+		},
 		{
-			"Not trash; not old enough to trash",
+			"Not trash, and not old enough to be eligible for trash",
 			t0.Add(-24 * time.Hour), t0.Add(-30 * time.Minute), none,
-			true, true, true, false, false},
+			true, true, true, false, false, false,
+		},
 		{
-			"Trash + not-trash: recent race between Trash and Put",
+			"Trashed + untrashed copies exist, due to recent race between Trash and Put",
 			t0.Add(-24 * time.Hour), t0.Add(-3 * time.Minute), t0.Add(-2 * time.Minute),
-			true, true, true, true, true},
+			true, true, true, true, true, false,
+		},
 		{
-			"Trash + not-trash, nearly eligible for deletion, prone to Trash race",
+			"Trashed + untrashed copies exist, trash nearly eligible for deletion: prone to Trash race",
 			t0.Add(-24 * time.Hour), t0.Add(-12 * time.Hour), t0.Add(-59 * time.Minute),
-			true, false, true, true, true},
+			true, false, true, true, true, false,
+		},
 		{
-			"Trash + not-trash, eligible for deletion, prone to Trash race",
+			"Trashed + untrashed copies exist, trash is eligible for deletion: prone to Trash race",
 			t0.Add(-24 * time.Hour), t0.Add(-12 * time.Hour), t0.Add(-61 * time.Minute),
-			true, false, true, true, false},
-		// FIXME: old trash never gets deleted!
-		// {
-		// 	"Not trash; old race between Trash and Put, or incomplete Trash",
-		// 	t0.Add(-24 * time.Hour), t0.Add(-12 * time.Hour), t0.Add(-12 * time.Hour),
-		// 	true, false, true, true, false},
+			true, false, true, true, false, false,
+		},
 		{
-			"Trash operation was interrupted",
+			"Trashed + untrashed copies exist, due to old race between Put and unfinished Trash: emptying trash is unsafe",
+			t0.Add(-24 * time.Hour), t0.Add(-12 * time.Hour), t0.Add(-12 * time.Hour),
+			true, false, true, true, true, true,
+		},
+		{
+			"Trashed + untrashed copies exist, used to be unsafe to empty, but since made safe by fixRace+Touch",
+			t0.Add(-time.Second), t0.Add(-time.Second), t0.Add(-12 * time.Hour),
+			true, true, true, true, false, false,
+		},
+		{
+			"Trashed + untrashed copies exist because Trash operation was interrupted (no race)",
 			t0.Add(-24 * time.Hour), t0.Add(-24 * time.Hour), t0.Add(-12 * time.Hour),
-			true, false, true, true, false},
+			true, false, true, true, false, false,
+		},
 		{
 			"Trash, not yet eligible for deletion",
 			none, t0.Add(-12 * time.Hour), t0.Add(-time.Minute),
-			false, false, false, true, true},
+			false, false, false, true, true, false,
+		},
 		{
 			"Trash, not yet eligible for deletion, prone to races",
 			none, t0.Add(-12 * time.Hour), t0.Add(-59 * time.Minute),
-			false, false, false, true, true},
+			false, false, false, true, true, false,
+		},
 		{
 			"Trash, eligible for deletion",
 			none, t0.Add(-12 * time.Hour), t0.Add(-2 * time.Hour),
-			false, false, false, true, false},
+			false, false, false, true, false, false,
+		},
 		{
 			"Erroneously trashed during a race, detected before trashLifetime",
 			none, t0.Add(-30 * time.Minute), t0.Add(-29 * time.Minute),
-			true, false, true, true, true},
+			true, false, true, true, true, false,
+		},
 		{
 			"Erroneously trashed during a race, rescue during EmptyTrash despite reaching trashLifetime",
 			none, t0.Add(-90 * time.Minute), t0.Add(-89 * time.Minute),
-			true, false, true, true, true},
+			true, false, true, true, true, false,
+		},
+		{
+			"Trashed copy exists with no recent/* marker (cause unknown); repair by untrashing",
+			none, none, t0.Add(-time.Minute),
+			false, false, false, true, true, true,
+		},
 	} {
-		c.Log("Scenario: ", test.label)
-		var loc string
-		var blk []byte
+		c.Log("Scenario: ", scenario.label)
 
-		setup := func() {
+		// We have a few tests to run for each scenario, and
+		// the tests are expected to change state. By calling
+		// this setup func between tests, we (re)create the
+		// scenario as specified, using a new unique block
+		// locator to prevent interference from previous
+		// tests.
+
+		setupScenario := func() (string, []byte) {
 			nextKey++
-			blk = []byte(fmt.Sprintf("%d", nextKey))
-			loc = fmt.Sprintf("%x", md5.Sum(blk))
+			blk := []byte(fmt.Sprintf("%d", nextKey))
+			loc := fmt.Sprintf("%x", md5.Sum(blk))
 			c.Log("\t", loc)
-			stubKey(test.data, loc, blk)
-			stubKey(test.recent, "recent/"+loc, nil)
-			stubKey(test.trash, "trash/"+loc, blk)
+			putS3Obj(scenario.dataT, loc, blk)
+			putS3Obj(scenario.recentT, "recent/"+loc, nil)
+			putS3Obj(scenario.trashT, "trash/"+loc, blk)
 			v.serverClock.now = &t0
+			return loc, blk
 		}
 
-		setup()
+		// Check canGet
+		loc, blk := setupScenario()
 		buf := make([]byte, len(blk))
 		_, err := v.Get(loc, buf)
-		c.Check(err == nil, check.Equals, test.canGet)
+		c.Check(err == nil, check.Equals, scenario.canGet)
 		if err != nil {
 			c.Check(os.IsNotExist(err), check.Equals, true)
 		}
 
-		setup()
+		// Call Trash, then check canTrash and canGetAfterTrash
+		loc, blk = setupScenario()
 		err = v.Trash(loc)
-		c.Check(err == nil, check.Equals, test.canTrash)
+		c.Check(err == nil, check.Equals, scenario.canTrash)
 		_, err = v.Get(loc, buf)
-		c.Check(err == nil, check.Equals, test.canGetAfterTrash)
+		c.Check(err == nil, check.Equals, scenario.canGetAfterTrash)
 		if err != nil {
 			c.Check(os.IsNotExist(err), check.Equals, true)
 		}
 
-		setup()
+		// Call Untrash, then check canUntrash
+		loc, blk = setupScenario()
 		err = v.Untrash(loc)
-		c.Check(err == nil, check.Equals, test.canUntrash)
+		c.Check(err == nil, check.Equals, scenario.canUntrash)
+		if scenario.dataT != none || scenario.trashT != none {
+			// In all scenarios where the data exists, we
+			// should be able to Get after Untrash --
+			// regardless of timestamps, errors, race
+			// conditions, etc.
+			_, err = v.Get(loc, buf)
+			c.Check(err, check.IsNil)
+		}
 
-		setup()
+		// Call EmptyTrash, then check haveTrashAfterEmpty and
+		// freshAfterEmpty
+		loc, blk = setupScenario()
 		v.EmptyTrash()
 		_, err = v.Bucket.Head("trash/"+loc, nil)
-		c.Check(err == nil, check.Equals, test.haveTrashAfterEmpty)
+		c.Check(err == nil, check.Equals, scenario.haveTrashAfterEmpty)
+		if scenario.freshAfterEmpty {
+			t, err := v.Mtime(loc)
+			c.Check(err, check.IsNil)
+			// new mtime must be current (with an
+			// allowance for 1s timestamp precision)
+			c.Check(t.After(t0.Add(-time.Second)), check.Equals, true)
+		}
+
+		// Check for current Mtime after Put (applies to all
+		// scenarios)
+		loc, blk = setupScenario()
+		err = v.Put(loc, blk)
+		c.Check(err, check.IsNil)
+		t, err := v.Mtime(loc)
+		c.Check(err, check.IsNil)
+		c.Check(t.After(t0.Add(-time.Second)), check.Equals, true)
 	}
 }