From: Tom Clegg Date: Wed, 9 Sep 2015 21:41:57 +0000 (-0400) Subject: 7179: Improve comments. X-Git-Tag: 1.1.0~1359^2~1 X-Git-Url: https://git.arvados.org/arvados.git/commitdiff_plain/7327d3b601a50148abead0ae226176a40937e363 7179: Improve comments. --- diff --git a/services/keepstore/volume.go b/services/keepstore/volume.go index 60e4b8686c..abd23aa532 100644 --- a/services/keepstore/volume.go +++ b/services/keepstore/volume.go @@ -20,7 +20,7 @@ type Volume interface { // // Get should not verify the integrity of the returned data: // it should just return whatever was found in its backing - // store. + // store. (Integrity checking is the caller's responsibility.) // // If an error is encountered that prevents it from // retrieving the data, that error should be returned so the @@ -69,9 +69,25 @@ type Volume interface { // // loc is as described in Get. // - // Touch must return a non-nil error unless it can guarantee - // that a future call to Mtime() will return a timestamp newer - // than {now minus one second}. + // If invoked at time t0, Touch must guarantee that a + // subsequent call to Mtime will return a timestamp no older + // than {t0 minus one second}. For example, if Touch is called + // at 2015-07-07T01:23:45.67890123Z, it is acceptable for a + // subsequent Mtime to return any of the following: + // + // - 2015-07-07T01:23:45.00000000Z + // - 2015-07-07T01:23:45.67890123Z + // - 2015-07-07T01:23:46.67890123Z + // - 2015-07-08T00:00:00.00000000Z + // + // It is not acceptable for a subsequente Mtime to return + // either of the following: + // + // - 2015-07-07T00:00:00.00000000Z -- ERROR + // - 2015-07-07T01:23:44.00000000Z -- ERROR + // + // Touch must return a non-nil error if the timestamp cannot + // be updated. Touch(loc string) error // Mtime returns the stored timestamp for the given locator. @@ -172,19 +188,25 @@ type Volume interface { type VolumeManager interface { // AllReadable returns all volumes. AllReadable() []Volume + // AllWritable returns all volumes that aren't known to be in // a read-only state. (There is no guarantee that a write to // one will succeed, though.) AllWritable() []Volume + // NextWritable returns the volume where the next new block // should be written. A VolumeManager can select a volume in // order to distribute activity across spindles, fill up disks // with more free space, etc. NextWritable() Volume + // Close shuts down the volume manager cleanly. Close() } +// RRVolumeManager is a round-robin VolumeManager: the Nth call to +// NextWritable returns the (N % len(writables))th writable Volume +// (where writables are all Volumes v where v.Writable()==true). type RRVolumeManager struct { readables []Volume writables []Volume diff --git a/services/keepstore/volume_generic_test.go b/services/keepstore/volume_generic_test.go index 20faf8f08e..0c0629c2a1 100644 --- a/services/keepstore/volume_generic_test.go +++ b/services/keepstore/volume_generic_test.go @@ -7,13 +7,22 @@ import ( "time" ) +// A TestableVolumeFactory returns a new TestableVolume. The factory +// function, and the TestableVolume it returns, can use t to write +// logs, fail the current test, etc. type TestableVolumeFactory func(t *testing.T) TestableVolume +// DoGenericVolumeTests runs a set of tests that every TestableVolume +// is expected to pass. It calls factory to create a new +// TestableVolume for each test case, to avoid leaking state between +// tests. func DoGenericVolumeTests(t *testing.T, factory TestableVolumeFactory) { testDeleteNewBlock(t, factory) testDeleteOldBlock(t, factory) } +// Calling Delete() for a block immediately after writing it should +// neither delete the data nor return an error. func testDeleteNewBlock(t *testing.T, factory TestableVolumeFactory) { v := factory(t) defer v.Teardown() @@ -22,8 +31,6 @@ func testDeleteNewBlock(t *testing.T, factory TestableVolumeFactory) { if err := v.Delete(TEST_HASH); err != nil { t.Error(err) } - // This isn't reported as an error, but the block should not - // have been deleted: it's newer than blob_signature_ttl. if data, err := v.Get(TEST_HASH); err != nil { t.Error(err) } else if bytes.Compare(data, TEST_BLOCK) != 0 { @@ -31,6 +38,8 @@ func testDeleteNewBlock(t *testing.T, factory TestableVolumeFactory) { } } +// Calling Delete() for a block with a timestamp older than +// blob_signature_ttl seconds in the past should delete the data. func testDeleteOldBlock(t *testing.T, factory TestableVolumeFactory) { v := factory(t) defer v.Teardown() diff --git a/services/keepstore/volume_test.go b/services/keepstore/volume_test.go index 34dcbdc8cf..4f4ee03759 100644 --- a/services/keepstore/volume_test.go +++ b/services/keepstore/volume_test.go @@ -18,9 +18,11 @@ type TestableVolume interface { // [Over]write content for a locator with the given data, // bypassing all constraints like readonly and serialize. PutRaw(locator string, data []byte) + // Specify the value Mtime() should return, until the next // call to Touch, TouchWithDate, or Put. TouchWithDate(locator string, lastPut time.Time) + // Clean up, delete temporary files. Teardown() } @@ -29,14 +31,18 @@ type TestableVolume interface { type MockVolume struct { Store map[string][]byte Timestamps map[string]time.Time + // Bad volumes return an error for every operation. Bad bool + // Touchable volumes' Touch() method succeeds for a locator // that has been Put(). Touchable bool + // Readonly volumes return an error for Put, Delete, and // Touch. Readonly bool + // Gate is a "starting gate", allowing test cases to pause // volume operations long enough to inspect state. Every // operation (except Status) starts by receiving from @@ -45,6 +51,7 @@ type MockVolume struct { // closed channel, so all operations proceed without // blocking. See trash_worker_test.go for an example. Gate chan struct{} + called map[string]int mutex sync.Mutex }