[ARVADOS] updated: 9a34f5ed292fb8d2f262e4c23e758cd689d81db7
git at public.curoverse.com
git at public.curoverse.com
Wed Sep 9 20:51:23 EDT 2015
Summary of changes:
services/keepstore/volume.go | 173 ++++++++++++++++++++++++++++-
services/keepstore/volume_generic_test.go | 55 ++++++++++
services/keepstore/volume_test.go | 22 ++++
services/keepstore/volume_unix_test.go | 177 +++++++++++++++---------------
4 files changed, 336 insertions(+), 91 deletions(-)
create mode 100644 services/keepstore/volume_generic_test.go
via 9a34f5ed292fb8d2f262e4c23e758cd689d81db7 (commit)
via 52cca9dc7c50ef8c54c9dc83a2ccf0129c27485e (commit)
via 7327d3b601a50148abead0ae226176a40937e363 (commit)
via 5dd88c53ca3357f96bb98ad286d0fb0a52ef5f54 (commit)
via eeeb4aef6e54d5cd3290bdeba91a8009f3e261bc (commit)
via 20abe07b6780abb3a67292af4f4f8ffab988f9ca (commit)
via 8dd54802b2ada6da4fd6faac6e4fba2e82d2fb11 (commit)
from c5f81922eff0f35eb2bcf8f9da7b9da1a7f86e90 (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
commit 9a34f5ed292fb8d2f262e4c23e758cd689d81db7
Merge: c5f8192 52cca9d
Author: Tom Clegg <tom at curoverse.com>
Date: Wed Sep 9 20:49:30 2015 -0400
Merge branch '7179-test-mocks' refs #7179
commit 52cca9dc7c50ef8c54c9dc83a2ccf0129c27485e
Merge: 7327d3b c5f8192
Author: Tom Clegg <tom at curoverse.com>
Date: Wed Sep 9 20:48:53 2015 -0400
Merge branch 'master' into 7179-test-mocks
Conflicts:
services/keepstore/volume.go
services/keepstore/volume_unix_test.go
diff --cc services/keepstore/volume.go
index abd23aa,bda7d87..f57df24
--- a/services/keepstore/volume.go
+++ b/services/keepstore/volume.go
@@@ -13,173 -13,23 +13,182 @@@ import
type Volume interface {
// Get a block. IFF the returned error is nil, the caller must
// put the returned slice back into the buffer pool when it's
- // finished with it.
+ // finished with it. (Otherwise, the buffer pool will be
+ // depleted and eventually -- when all available buffers are
+ // used and not returned -- operations will reach deadlock.)
+ //
+ // loc is guaranteed to consist of 32 or more lowercase hex
+ // digits.
+ //
+ // Get should not verify the integrity of the returned data:
+ // it should just return whatever was found in its backing
+ // 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
+ // caller can log (and send to the client) a more useful
+ // message.
+ //
+ // If the error is "not found", and there's no particular
+ // reason to expect the block to be found (other than that a
+ // caller is asking for it), the returned error should satisfy
+ // os.IsNotExist(err): this is a normal condition and will not
+ // be logged as an error (except that a 404 will appear in the
+ // access log if the block is not found on any other volumes
+ // either).
+ //
+ // If the data in the backing store is bigger than BLOCKSIZE,
+ // Get is permitted to return an error without reading any of
+ // the data.
Get(loc string) ([]byte, error)
- // Confirm Get() would return a buffer with exactly the same
- // content as buf. If so, return nil. If not, return
+
++ // Compare the given data with the stored data (i.e., what Get
++ // would return). If equal, return nil. If not, return
+ // CollisionError or DiskHashError (depending on whether the
+ // data on disk matches the expected hash), or whatever error
- // was encountered opening/reading the file.
++ // was encountered opening/reading the stored data.
+ Compare(loc string, data []byte) error
- Put(loc string, data []byte) error
++
+ // Put writes a block to an underlying storage device.
+ //
+ // loc is as described in Get.
+ //
+ // len(block) is guaranteed to be between 0 and BLOCKSIZE.
+ //
+ // If a block is already stored under the same name (loc) with
+ // different content, Put must either overwrite the existing
+ // data with the new data or return a non-nil error.
+ //
+ // Put also sets the timestamp for the given locator to the
+ // current time.
+ //
+ // Put must return a non-nil error unless it can guarantee
+ // that the entire block has been written and flushed to
+ // persistent storage, and that its timestamp is current. Of
+ // course, this guarantee is only as good as the underlying
+ // storage device, but it is Put's responsibility to at least
+ // get whatever guarantee is offered by the storage device.
+ //
+ // Put should not verify that loc==hash(block): this is the
+ // caller's responsibility.
+ Put(loc string, block []byte) error
+
+ // Touch sets the timestamp for the given locator to the
+ // current time.
+ //
+ // loc is as described in Get.
+ //
+ // 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.
+ //
+ // loc is as described in Get.
+ //
+ // Mtime must return a non-nil error if the given block is not
+ // found or the timestamp could not be retrieved.
Mtime(loc string) (time.Time, error)
+
+ // IndexTo writes a complete list of locators with the given
+ // prefix for which Get() can retrieve data.
+ //
+ // prefix consists of zero or more lowercase hexadecimal
+ // digits.
+ //
+ // Each locator must be written to the given writer using the
+ // following format:
+ //
+ // loc "+" size " " timestamp "\n"
+ //
+ // where:
+ //
+ // - size is the number of bytes of content, given as a
+ // decimal number with one or more digits
+ //
+ // - timestamp is the timestamp stored for the locator,
+ // given as a decimal number of seconds after January 1,
+ // 1970 UTC.
+ //
+ // IndexTo must not write any other data to writer: for
+ // example, it must not write any blank lines.
+ //
+ // If an error makes it impossible to provide a complete
+ // index, IndexTo must return a non-nil error. It is
+ // acceptable to return a non-nil error after writing a
+ // partial index to writer.
+ //
+ // The resulting index is not expected to be sorted in any
+ // particular order.
IndexTo(prefix string, writer io.Writer) error
+
+ // Delete deletes the block data from the underlying storage
+ // device.
+ //
+ // loc is as described in Get.
+ //
+ // If the timestamp for the given locator is newer than
+ // blob_signature_ttl, Delete must not delete the data.
+ //
+ // If a Delete operation overlaps with any Touch or Put
+ // operations on the same locator, the implementation must
+ // ensure one of the following outcomes:
+ //
+ // - Touch and Put return a non-nil error, or
+ // - Delete does not delete the block, or
+ // - Both of the above.
+ //
+ // If it is possible for the storage device to be accessed by
+ // a different process or host, the synchronization mechanism
+ // should also guard against races with other processes and
+ // hosts. If such a mechanism is not available, there must be
+ // a mechanism for detecting unsafe configurations, alerting
+ // the operator, and aborting or falling back to a read-only
+ // state. In other words, running multiple keepstore processes
+ // with the same underlying storage device must either work
+ // reliably or fail outright.
+ //
+ // Corollary: A successful Touch or Put guarantees a block
+ // will not be deleted for at least blob_signature_ttl
+ // seconds.
Delete(loc string) error
+
+ // Status returns a *VolumeStatus representing the current
+ // in-use and available storage capacity and an
+ // implementation-specific volume identifier (e.g., "mount
+ // point" for a UnixVolume).
Status() *VolumeStatus
+
+ // String returns an identifying label for this volume,
+ // suitable for including in log messages. It should contain
+ // enough information to uniquely identify the underlying
+ // storage device, but should not contain any credentials or
+ // secrets.
String() string
+
+ // Writable returns false if all future Put, Mtime, and Delete
+ // calls are expected to fail.
+ //
+ // If the volume is only temporarily unwritable -- or if Put
+ // will fail because it is full, but Mtime or Delete can
+ // succeed -- then Writable should return false.
Writable() bool
}
diff --cc services/keepstore/volume_unix_test.go
index b47bedb,08ca31c..9f37042
--- a/services/keepstore/volume_unix_test.go
+++ b/services/keepstore/volume_unix_test.go
@@@ -23,13 -21,14 +26,17 @@@ func NewTestableUnixVolume(t *testing.T
if err != nil {
t.Fatal(err)
}
+ var locker sync.Locker
+ if serialize {
+ locker = &sync.Mutex{}
+ }
- return &UnixVolume{
- root: d,
- locker: locker,
- readonly: readonly,
+ return &TestableUnixVolume{
+ UnixVolume: UnixVolume{
- root: d,
- serialize: serialize,
- readonly: readonly,
++ root: d,
++ locker: locker,
++ readonly: readonly,
+ },
+ t: t,
}
}
@@@ -388,3 -392,98 +395,98 @@@ func TestNodeStatus(t *testing.T)
t.Errorf("uninitialized bytes_used in %v", volinfo)
}
}
+
+ func TestUnixVolumeGetFuncWorkerError(t *testing.T) {
- v := TempUnixVolume(t, false, false)
- defer _teardown(v)
++ v := NewTestableUnixVolume(t, false, false)
++ defer v.Teardown()
+
+ v.Put(TEST_HASH, TEST_BLOCK)
+ mockErr := errors.New("Mock error")
+ err := v.getFunc(v.blockPath(TEST_HASH), func(rdr io.Reader) error {
+ return mockErr
+ })
+ if err != mockErr {
+ t.Errorf("Got %v, expected %v", err, mockErr)
+ }
+ }
+
+ func TestUnixVolumeGetFuncFileError(t *testing.T) {
- v := TempUnixVolume(t, false, false)
- defer _teardown(v)
++ v := NewTestableUnixVolume(t, false, false)
++ defer v.Teardown()
+
+ funcCalled := false
+ err := v.getFunc(v.blockPath(TEST_HASH), func(rdr io.Reader) error {
+ funcCalled = true
+ return nil
+ })
+ if err == nil {
+ t.Errorf("Expected error opening non-existent file")
+ }
+ if funcCalled {
+ t.Errorf("Worker func should not have been called")
+ }
+ }
+
+ func TestUnixVolumeGetFuncWorkerWaitsOnMutex(t *testing.T) {
- v := TempUnixVolume(t, false, false)
- defer _teardown(v)
++ v := NewTestableUnixVolume(t, false, false)
++ defer v.Teardown()
+
+ v.Put(TEST_HASH, TEST_BLOCK)
+
+ mtx := NewMockMutex()
+ v.locker = mtx
+
+ funcCalled := make(chan struct{})
+ go v.getFunc(v.blockPath(TEST_HASH), func(rdr io.Reader) error {
+ funcCalled <- struct{}{}
+ return nil
+ })
+ select {
+ case mtx.AllowLock <- struct{}{}:
+ case <-funcCalled:
+ t.Fatal("Function was called before mutex was acquired")
+ case <-time.After(5 * time.Second):
+ t.Fatal("Timed out before mutex was acquired")
+ }
+ select {
+ case <-funcCalled:
+ case mtx.AllowUnlock <- struct{}{}:
+ t.Fatal("Mutex was released before function was called")
+ case <-time.After(5 * time.Second):
+ t.Fatal("Timed out waiting for funcCalled")
+ }
+ select {
+ case mtx.AllowUnlock <- struct{}{}:
+ case <-time.After(5 * time.Second):
+ t.Fatal("Timed out waiting for getFunc() to release mutex")
+ }
+ }
+
+ func TestUnixVolumeCompare(t *testing.T) {
- v := TempUnixVolume(t, false, false)
- defer _teardown(v)
++ v := NewTestableUnixVolume(t, false, false)
++ defer v.Teardown()
+
+ v.Put(TEST_HASH, TEST_BLOCK)
+ err := v.Compare(TEST_HASH, TEST_BLOCK)
+ if err != nil {
+ t.Errorf("Got err %q, expected nil", err)
+ }
+
+ err = v.Compare(TEST_HASH, []byte("baddata"))
+ if err != CollisionError {
+ t.Errorf("Got err %q, expected %q", err, CollisionError)
+ }
+
- _store(t, v, TEST_HASH, []byte("baddata"))
++ v.Put(TEST_HASH, []byte("baddata"))
+ err = v.Compare(TEST_HASH, TEST_BLOCK)
+ if err != DiskHashError {
+ t.Errorf("Got err %q, expected %q", err, DiskHashError)
+ }
+
+ p := fmt.Sprintf("%s/%s/%s", v.root, TEST_HASH[:3], TEST_HASH)
+ os.Chmod(p, 000)
+ err = v.Compare(TEST_HASH, TEST_BLOCK)
+ if err == nil || strings.Index(err.Error(), "permission denied") < 0 {
+ t.Errorf("Got err %q, expected %q", err, "permission denied")
+ }
+ }
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list