[ARVADOS] updated: daa3cc42efa55ad17df2a151aebaac515d10cc52

git at public.curoverse.com git at public.curoverse.com
Tue Jan 26 10:41:10 EST 2016


Summary of changes:
 services/crunchstat/crunchstat.go                  |   4 +-
 services/keepstore/azure_blob_volume.go            |   5 +
 services/keepstore/handlers.go                     |  15 +-
 services/keepstore/s3_volume.go                    |   3 +
 services/keepstore/volume_test.go                  |   2 +-
 services/keepstore/volume_unix.go                  |   3 +
 tools/crunchstat-summary/bin/crunchstat-summary    |   9 +-
 .../crunchstat_summary/__init__.py                 |   4 +
 .../crunchstat_summary/chartjs.js                  |  27 ++
 .../crunchstat_summary/chartjs.py                  |  70 +++++
 .../crunchstat_summary/command.py                  |  47 +++
 .../crunchstat_summary/summarizer.py               | 336 ++++++++++++++++++---
 tools/crunchstat-summary/setup.py                  |   2 +
 .../tests/crunchstat_error_messages.txt            |   9 +
 .../tests/logfile_20151204190335.txt.gz.report     |   3 +
 .../tests/logfile_20151210063411.txt.gz.report     |   2 +
 .../tests/logfile_20151210063439.txt.gz.report     |   2 +
 tools/crunchstat-summary/tests/test_examples.py    | 158 +++++++++-
 18 files changed, 630 insertions(+), 71 deletions(-)
 create mode 100644 tools/crunchstat-summary/crunchstat_summary/chartjs.js
 create mode 100644 tools/crunchstat-summary/crunchstat_summary/chartjs.py
 create mode 100644 tools/crunchstat-summary/tests/crunchstat_error_messages.txt

       via  daa3cc42efa55ad17df2a151aebaac515d10cc52 (commit)
       via  ce669fb969b7669dc1f672214f96404fcd141455 (commit)
       via  0d4bee15a0e69b08799bff869eb3f1ca122f107f (commit)
       via  b32e371420fff5763c13c79cd4327692e2a2e1bd (commit)
       via  7faa4aa742a7562c75e2fc2cabc7bf2ea8e856a8 (commit)
       via  fc3b155fb7ad82cf97cdb81a02a9cfaebe4389ff (commit)
       via  a4b58ddc7b8ec80010dfa30dc6c4ac1ce69858da (commit)
       via  fdd28d6f009c605c61f4444dc2d9d142c3a1d395 (commit)
       via  d9ab654450fd23c043beab5c19c99806a301e51c (commit)
       via  56922b056a746e79c53b43186dbeff7e8a856546 (commit)
       via  e001d5f3e5cece6429c9b1305f98fdad89f451d0 (commit)
       via  b59cedb067aa5cf27045d1ad15fe9cea49502520 (commit)
       via  aa720c29abcf039a965231decb6a40d00e479437 (commit)
       via  72e1f8eebd7eb9ad5f6b4d0923f06cfc2b57b9d4 (commit)
       via  12b1501e8ee2edc932ceb908ab69189712a761b4 (commit)
       via  0d3411abc645a8cd6fef4b960070ecf054ac16f2 (commit)
       via  20fe67d073424f5c277fbd13557ffe5ae2b15fd9 (commit)
       via  0343fe988fdc50bdfb1ac34204c1a4998fc7c446 (commit)
       via  4b78fb11974d8bcb0b9e4ecd0162d6a938026c73 (commit)
       via  b8fbd409f84d647726a25ee7fd2d73660a2bfa82 (commit)
       via  d9b556e32bd91635a57d5f7ab0844eb3ad12de2b (commit)
       via  87a3cc47267fad4cc769d9d866754c072a83c35e (commit)
       via  a82be0d9ab0438fa778e80c0aea3a07e8ba6a929 (commit)
      from  3b31f110db82c93c3aade294f50bbb0218c74697 (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 daa3cc42efa55ad17df2a151aebaac515d10cc52
Merge: ce669fb 3b31f11
Author: radhika <radhika at curoverse.com>
Date:   Tue Jan 26 10:41:00 2016 -0500

    Merge branch '8178-keepstore-trash-interface' of git.curoverse.com:arvados into 8178-keepstore-trash-interface
    
    Conflicts:
    	services/keepstore/handlers.go
    	services/keepstore/volume_test.go


commit ce669fb969b7669dc1f672214f96404fcd141455
Author: radhika <radhika at curoverse.com>
Date:   Tue Jan 26 10:38:28 2016 -0500

    8178: untrash should fail when ErrNotImplemented is returned.

diff --git a/services/keepstore/azure_blob_volume.go b/services/keepstore/azure_blob_volume.go
index 3681230..7dfb84d 100644
--- a/services/keepstore/azure_blob_volume.go
+++ b/services/keepstore/azure_blob_volume.go
@@ -320,6 +320,11 @@ func (v *AzureBlobVolume) Trash(loc string) error {
 	if v.readonly {
 		return MethodDisabledError
 	}
+
+	if trashLifetime != 0 {
+		return ErrNotImplemented
+	}
+
 	// Ideally we would use If-Unmodified-Since, but that
 	// particular condition seems to be ignored by Azure. Instead,
 	// we get the Etag before checking Mtime, and use If-Match to
diff --git a/services/keepstore/handlers.go b/services/keepstore/handlers.go
index e52dba8..043ab69 100644
--- a/services/keepstore/handlers.go
+++ b/services/keepstore/handlers.go
@@ -453,16 +453,15 @@ func UntrashHandler(resp http.ResponseWriter, req *http.Request) {
 	var numNotFound int
 	for _, vol := range KeepVM.AllWritable() {
 		err := vol.Untrash(hash)
-		if err == nil || err == ErrNotImplemented {
+
+		if os.IsNotExist(err) {
+			numNotFound++
+		} else if err != nil {
+			log.Printf("Error untrashing %v on volume %v", hash, vol.String())
+			failedOn = append(failedOn, vol.String())
+		} else {
 			log.Printf("Untrashed %v on volume %v", hash, vol.String())
 			untrashedOn = append(untrashedOn, vol.String())
-		} else {
-			if os.IsNotExist(err) {
-				numNotFound++
-			} else {
-				log.Printf("Error untrashing %v on volume %v", hash, vol.String())
-				failedOn = append(failedOn, vol.String())
-			}
 		}
 	}
 
diff --git a/services/keepstore/s3_volume.go b/services/keepstore/s3_volume.go
index 1df8f78..7d9ba8a 100644
--- a/services/keepstore/s3_volume.go
+++ b/services/keepstore/s3_volume.go
@@ -264,6 +264,9 @@ func (v *S3Volume) Trash(loc string) error {
 	if v.readonly {
 		return MethodDisabledError
 	}
+	if trashLifetime != 0 {
+		return ErrNotImplemented
+	}
 	if t, err := v.Mtime(loc); err != nil {
 		return err
 	} else if time.Since(t) < blobSignatureTTL {
diff --git a/services/keepstore/volume_test.go b/services/keepstore/volume_test.go
index 2af1ad6..53ffeef 100644
--- a/services/keepstore/volume_test.go
+++ b/services/keepstore/volume_test.go
@@ -201,7 +201,7 @@ func (v *MockVolume) Trash(loc string) error {
 
 // TBD
 func (v *MockVolume) Untrash(loc string) error {
-	return ErrNotImplemented
+	return nil
 }
 
 func (v *MockVolume) Status() *VolumeStatus {
diff --git a/services/keepstore/volume_unix.go b/services/keepstore/volume_unix.go
index d71e175..0dd1d82 100644
--- a/services/keepstore/volume_unix.go
+++ b/services/keepstore/volume_unix.go
@@ -378,6 +378,9 @@ func (v *UnixVolume) Trash(loc string) error {
 	if v.readonly {
 		return MethodDisabledError
 	}
+	if trashLifetime != 0 {
+		return ErrNotImplemented
+	}
 	if v.locker != nil {
 		v.locker.Lock()
 		defer v.locker.Unlock()

commit 0d4bee15a0e69b08799bff869eb3f1ca122f107f
Author: radhika <radhika at curoverse.com>
Date:   Fri Jan 22 17:37:15 2016 -0500

    8178: (for now) all volumes must return ErrNotImplemented if trash-lifetime != 0

diff --git a/services/keepstore/azure_blob_volume.go b/services/keepstore/azure_blob_volume.go
index f135835..3681230 100644
--- a/services/keepstore/azure_blob_volume.go
+++ b/services/keepstore/azure_blob_volume.go
@@ -43,8 +43,7 @@ type azureVolumeAdder struct {
 }
 
 func (s *azureVolumeAdder) Set(containerName string) error {
-	if trashLifetime <= 0 {
-		log.Print("Missing required configuration parameter: trash-lifetime")
+	if trashLifetime != 0 {
 		return ErrNotImplemented
 	}
 
@@ -343,7 +342,7 @@ func (v *AzureBlobVolume) Trash(loc string) error {
 // Untrash a Keep block.
 // TBD
 func (v *AzureBlobVolume) Untrash(loc string) error {
-	return nil
+	return ErrNotImplemented
 }
 
 // Status returns a VolumeStatus struct with placeholder data.
diff --git a/services/keepstore/handler_test.go b/services/keepstore/handler_test.go
index 9526c3c..a7675fb 100644
--- a/services/keepstore/handler_test.go
+++ b/services/keepstore/handler_test.go
@@ -971,7 +971,7 @@ func TestPutReplicationHeader(t *testing.T) {
 	}
 }
 
-func TestUndeleteHandler(t *testing.T) {
+func TestUntrashHandler(t *testing.T) {
 	defer teardown()
 
 	// Set up Keep volumes
@@ -980,73 +980,96 @@ func TestUndeleteHandler(t *testing.T) {
 	vols := KeepVM.AllWritable()
 	vols[0].Put(TestHash, TestBlock)
 
-	// Only the datamanager user should be allowed to undelete blocks
 	dataManagerToken = "DATA MANAGER TOKEN"
 
 	// unauthenticatedReq => UnauthorizedError
 	unauthenticatedReq := &RequestTester{
 		method: "PUT",
-		uri:    "/undelete/" + TestHash,
+		uri:    "/untrash/" + TestHash,
 	}
 	response := IssueRequest(unauthenticatedReq)
 	ExpectStatusCode(t,
-		"enforcePermissions on, unauthenticated request",
+		"Unauthenticated request",
 		UnauthorizedError.HTTPCode,
 		response)
 
 	// notDataManagerReq => UnauthorizedError
 	notDataManagerReq := &RequestTester{
 		method:   "PUT",
-		uri:      "/undelete/" + TestHash,
+		uri:      "/untrash/" + TestHash,
 		apiToken: knownToken,
 	}
 
 	response = IssueRequest(notDataManagerReq)
 	ExpectStatusCode(t,
-		"permissions on, unauthenticated /index/prefix request",
+		"Non-datamanager token",
 		UnauthorizedError.HTTPCode,
 		response)
 
 	// datamanagerWithBadHashReq => StatusBadRequest
 	datamanagerWithBadHashReq := &RequestTester{
 		method:   "PUT",
-		uri:      "/undelete/thisisnotalocator",
+		uri:      "/untrash/thisisnotalocator",
 		apiToken: dataManagerToken,
 	}
 	response = IssueRequest(datamanagerWithBadHashReq)
 	ExpectStatusCode(t,
-		"permissions on, authenticated request, non-superuser",
+		"Bad locator in untrash request",
 		http.StatusBadRequest,
 		response)
 
 	// datamanagerWrongMethodReq => StatusBadRequest
 	datamanagerWrongMethodReq := &RequestTester{
 		method:   "GET",
-		uri:      "/undelete/" + TestHash,
+		uri:      "/untrash/" + TestHash,
 		apiToken: dataManagerToken,
 	}
 	response = IssueRequest(datamanagerWrongMethodReq)
 	ExpectStatusCode(t,
-		"permissions on, authenticated request, non-superuser",
+		"Only PUT method is supported for untrash",
 		http.StatusBadRequest,
 		response)
 
 	// datamanagerReq => StatusOK
 	datamanagerReq := &RequestTester{
 		method:   "PUT",
-		uri:      "/undelete/" + TestHash,
+		uri:      "/untrash/" + TestHash,
 		apiToken: dataManagerToken,
 	}
 	response = IssueRequest(datamanagerReq)
 	ExpectStatusCode(t,
-		"permissions on, authenticated request, non-superuser",
+		"",
 		http.StatusOK,
 		response)
-	expected := `Untrashed on volume`
-	match, _ := regexp.MatchString(expected, response.Body.String())
-	if !match {
+	expected := "Successfully untrashed on: [MockVolume],[MockVolume]"
+	if response.Body.String() != expected {
 		t.Errorf(
-			"Undelete response mismatched: expected %s, got:\n%s",
+			"Untrash response mismatched: expected %s, got:\n%s",
 			expected, response.Body.String())
 	}
 }
+
+func TestUntrashHandlerWithNoWritableVolumes(t *testing.T) {
+	defer teardown()
+
+	// Set up readonly Keep volumes
+	vols := []*MockVolume{CreateMockVolume(), CreateMockVolume()}
+	vols[0].Readonly = true
+	vols[1].Readonly = true
+	KeepVM = MakeRRVolumeManager([]Volume{vols[0], vols[1]})
+	defer KeepVM.Close()
+
+	dataManagerToken = "DATA MANAGER TOKEN"
+
+	// datamanagerReq => StatusOK
+	datamanagerReq := &RequestTester{
+		method:   "PUT",
+		uri:      "/untrash/" + TestHash,
+		apiToken: dataManagerToken,
+	}
+	response := IssueRequest(datamanagerReq)
+	ExpectStatusCode(t,
+		"No writable volumes",
+		http.StatusNotFound,
+		response)
+}
diff --git a/services/keepstore/handlers.go b/services/keepstore/handlers.go
index f8e4d71..e52dba8 100644
--- a/services/keepstore/handlers.go
+++ b/services/keepstore/handlers.go
@@ -20,6 +20,7 @@ import (
 	"regexp"
 	"runtime"
 	"strconv"
+	"strings"
 	"sync"
 	"time"
 )
@@ -53,8 +54,8 @@ func MakeRESTRouter() *mux.Router {
 	// Replace the current trash queue.
 	rest.HandleFunc(`/trash`, TrashHandler).Methods("PUT")
 
-	// Undelete moves blocks from trash back into store
-	rest.HandleFunc(`/undelete/{hash:[0-9a-f]{32}}`, UndeleteHandler).Methods("PUT")
+	// Untrash moves blocks from trash back into store
+	rest.HandleFunc(`/untrash/{hash:[0-9a-f]{32}}`, UntrashHandler).Methods("PUT")
 
 	// Any request which does not match any of these routes gets
 	// 400 Bad Request.
@@ -433,8 +434,8 @@ func TrashHandler(resp http.ResponseWriter, req *http.Request) {
 	trashq.ReplaceQueue(tlist)
 }
 
-// UndeleteHandler processes "PUT /undelete/{hash:[0-9a-f]{32}}" requests for the data manager.
-func UndeleteHandler(resp http.ResponseWriter, req *http.Request) {
+// UntrashHandler processes "PUT /untrash/{hash:[0-9a-f]{32}}" requests for the data manager.
+func UntrashHandler(resp http.ResponseWriter, req *http.Request) {
 	// Reject unauthorized requests.
 	if !IsDataManagerToken(GetApiToken(req)) {
 		http.Error(resp, UnauthorizedError.Error(), UnauthorizedError.HTTPCode)
@@ -442,25 +443,43 @@ func UndeleteHandler(resp http.ResponseWriter, req *http.Request) {
 	}
 
 	hash := mux.Vars(req)["hash"]
-	successResp := "Untrashed on volume: "
-	var st int
+
+	if len(KeepVM.AllWritable()) == 0 {
+		http.Error(resp, "No writable volumes", http.StatusNotFound)
+		return
+	}
+
+	var untrashedOn, failedOn []string
+	var numNotFound int
 	for _, vol := range KeepVM.AllWritable() {
-		if err := vol.Untrash(hash); err == nil {
+		err := vol.Untrash(hash)
+		if err == nil || err == ErrNotImplemented {
 			log.Printf("Untrashed %v on volume %v", hash, vol.String())
-			st = http.StatusOK
-			successResp += vol.String()
-			break
+			untrashedOn = append(untrashedOn, vol.String())
 		} else {
-			log.Printf("Error untrashing %v on volume %v: %v", hash, vol.String(), err)
-			st = 500
+			if os.IsNotExist(err) {
+				numNotFound++
+			} else {
+				log.Printf("Error untrashing %v on volume %v", hash, vol.String())
+				failedOn = append(failedOn, vol.String())
+			}
 		}
 	}
 
-	if st == http.StatusOK {
-		resp.Write([]byte(successResp))
+	if numNotFound == len(KeepVM.AllWritable()) {
+		http.Error(resp, "Block not found on any of the writable volumes", http.StatusNotFound)
+		return
 	}
 
-	resp.WriteHeader(st)
+	if len(failedOn) == len(KeepVM.AllWritable()) {
+		http.Error(resp, "Failed to untrash on all writable volumes", http.StatusInternalServerError)
+	} else {
+		respBody := "Successfully untrashed on: " + strings.Join(untrashedOn, ",")
+		if len(failedOn) > 0 {
+			respBody += "; Failed to untrash on: " + strings.Join(failedOn, ",")
+		}
+		resp.Write([]byte(respBody))
+	}
 }
 
 // ==============================
diff --git a/services/keepstore/keepstore.go b/services/keepstore/keepstore.go
index f6b1f6e..3850e99 100644
--- a/services/keepstore/keepstore.go
+++ b/services/keepstore/keepstore.go
@@ -55,6 +55,10 @@ var dataManagerToken string
 // actually deleting anything.
 var neverDelete = true
 
+// trashLifetime is the time duration after a block is trashed
+// during which it can be recovered using an /untrash request
+var trashLifetime time.Duration
+
 var maxBuffers = 128
 var bufs *bufferPool
 
@@ -114,7 +118,6 @@ var (
 	flagSerializeIO bool
 	flagReadonly    bool
 	volumes         volumeSet
-	trashLifetime   int
 )
 
 func (vs *volumeSet) String() string {
@@ -202,11 +205,11 @@ func main() {
 		"max-buffers",
 		maxBuffers,
 		fmt.Sprintf("Maximum RAM to use for data buffers, given in multiples of block size (%d MiB). When this limit is reached, HTTP requests requiring buffers (like GET and PUT) will wait for buffer space to be released.", BlockSize>>20))
-	flag.IntVar(
+	flag.DurationVar(
 		&trashLifetime,
 		"trash-lifetime",
-		0,
-		fmt.Sprintf("Trashed blocks will stay in trash for trash-lifetime interval before they are actually deleted by the system."))
+		0*time.Second,
+		"Interval after a block is trashed during which it can be recovered using an /untrash request")
 
 	flag.Parse()
 
diff --git a/services/keepstore/keepstore_test.go b/services/keepstore/keepstore_test.go
index 746d99e..2a1c3d2 100644
--- a/services/keepstore/keepstore_test.go
+++ b/services/keepstore/keepstore_test.go
@@ -335,7 +335,6 @@ func TestDiscoverTmpfs(t *testing.T) {
 	f.Close()
 	ProcMounts = f.Name()
 
-	trashLifetime = 24 * 60 * 60
 	resultVols := volumeSet{}
 	added := (&unixVolumeAdder{&resultVols}).Discover()
 
@@ -376,7 +375,6 @@ func TestDiscoverNone(t *testing.T) {
 	f.Close()
 	ProcMounts = f.Name()
 
-	trashLifetime = 24 * 60 * 60
 	resultVols := volumeSet{}
 	added := (&unixVolumeAdder{&resultVols}).Discover()
 	if added != 0 || len(resultVols) != 0 {
diff --git a/services/keepstore/s3_volume.go b/services/keepstore/s3_volume.go
index 17cc194..1df8f78 100644
--- a/services/keepstore/s3_volume.go
+++ b/services/keepstore/s3_volume.go
@@ -39,8 +39,7 @@ type s3VolumeAdder struct {
 }
 
 func (s *s3VolumeAdder) Set(bucketName string) error {
-	if trashLifetime <= 0 {
-		log.Print("Missing required configuration parameter: trash-lifetime")
+	if trashLifetime != 0 {
 		return ErrNotImplemented
 	}
 	if bucketName == "" {
@@ -278,7 +277,7 @@ func (v *S3Volume) Trash(loc string) error {
 
 // TBD
 func (v *S3Volume) Untrash(loc string) error {
-	return nil
+	return ErrNotImplemented
 }
 
 func (v *S3Volume) Status() *VolumeStatus {
diff --git a/services/keepstore/volume_test.go b/services/keepstore/volume_test.go
index 53ffeef..2af1ad6 100644
--- a/services/keepstore/volume_test.go
+++ b/services/keepstore/volume_test.go
@@ -201,7 +201,7 @@ func (v *MockVolume) Trash(loc string) error {
 
 // TBD
 func (v *MockVolume) Untrash(loc string) error {
-	return nil
+	return ErrNotImplemented
 }
 
 func (v *MockVolume) Status() *VolumeStatus {
diff --git a/services/keepstore/volume_unix.go b/services/keepstore/volume_unix.go
index 1be622c..d71e175 100644
--- a/services/keepstore/volume_unix.go
+++ b/services/keepstore/volume_unix.go
@@ -23,9 +23,8 @@ type unixVolumeAdder struct {
 }
 
 func (vs *unixVolumeAdder) Set(value string) error {
-	if trashLifetime <= 0 {
-		log.Print("Missing required configuration parameter: trash-lifetime")
-		//return ErrNotImplemented
+	if trashLifetime != 0 {
+		return ErrNotImplemented
 	}
 	if dirs := strings.Split(value, ","); len(dirs) > 1 {
 		log.Print("DEPRECATED: using comma-separated volume list.")
@@ -412,7 +411,7 @@ func (v *UnixVolume) Trash(loc string) error {
 // Untrash moves block from trash back into store
 // TBD
 func (v *UnixVolume) Untrash(loc string) error {
-	return nil
+	return ErrNotImplemented
 }
 
 // blockDir returns the fully qualified directory name for the directory

commit b32e371420fff5763c13c79cd4327692e2a2e1bd
Author: radhika <radhika at curoverse.com>
Date:   Thu Jan 21 15:25:06 2016 -0500

    8178: All three currently supported volumes return error when trash-lifetime period is not configured. azure blob and s3 volumes are updated to do so.
    Returning an error is causing test failures in unix volume and hence is still a work in progress.

diff --git a/services/keepstore/azure_blob_volume.go b/services/keepstore/azure_blob_volume.go
index 0071567..f135835 100644
--- a/services/keepstore/azure_blob_volume.go
+++ b/services/keepstore/azure_blob_volume.go
@@ -43,6 +43,11 @@ type azureVolumeAdder struct {
 }
 
 func (s *azureVolumeAdder) Set(containerName string) error {
+	if trashLifetime <= 0 {
+		log.Print("Missing required configuration parameter: trash-lifetime")
+		return ErrNotImplemented
+	}
+
 	if containerName == "" {
 		return errors.New("no container name given")
 	}
diff --git a/services/keepstore/keepstore_test.go b/services/keepstore/keepstore_test.go
index 2a1c3d2..746d99e 100644
--- a/services/keepstore/keepstore_test.go
+++ b/services/keepstore/keepstore_test.go
@@ -335,6 +335,7 @@ func TestDiscoverTmpfs(t *testing.T) {
 	f.Close()
 	ProcMounts = f.Name()
 
+	trashLifetime = 24 * 60 * 60
 	resultVols := volumeSet{}
 	added := (&unixVolumeAdder{&resultVols}).Discover()
 
@@ -375,6 +376,7 @@ func TestDiscoverNone(t *testing.T) {
 	f.Close()
 	ProcMounts = f.Name()
 
+	trashLifetime = 24 * 60 * 60
 	resultVols := volumeSet{}
 	added := (&unixVolumeAdder{&resultVols}).Discover()
 	if added != 0 || len(resultVols) != 0 {
diff --git a/services/keepstore/s3_volume.go b/services/keepstore/s3_volume.go
index 16afc32..17cc194 100644
--- a/services/keepstore/s3_volume.go
+++ b/services/keepstore/s3_volume.go
@@ -39,6 +39,10 @@ type s3VolumeAdder struct {
 }
 
 func (s *s3VolumeAdder) Set(bucketName string) error {
+	if trashLifetime <= 0 {
+		log.Print("Missing required configuration parameter: trash-lifetime")
+		return ErrNotImplemented
+	}
 	if bucketName == "" {
 		return fmt.Errorf("no container name given")
 	}
diff --git a/services/keepstore/volume_unix.go b/services/keepstore/volume_unix.go
index da1d390..1be622c 100644
--- a/services/keepstore/volume_unix.go
+++ b/services/keepstore/volume_unix.go
@@ -23,6 +23,10 @@ type unixVolumeAdder struct {
 }
 
 func (vs *unixVolumeAdder) Set(value string) error {
+	if trashLifetime <= 0 {
+		log.Print("Missing required configuration parameter: trash-lifetime")
+		//return ErrNotImplemented
+	}
 	if dirs := strings.Split(value, ","); len(dirs) > 1 {
 		log.Print("DEPRECATED: using comma-separated volume list.")
 		for _, dir := range dirs {

commit 7faa4aa742a7562c75e2fc2cabc7bf2ea8e856a8
Author: radhika <radhika at curoverse.com>
Date:   Thu Jan 21 13:59:36 2016 -0500

    8178: rename Delete api as Trash; add Untrash to volume interface; add UndeleteHandler and test for this endpoint.

diff --git a/services/keepstore/azure_blob_volume.go b/services/keepstore/azure_blob_volume.go
index c0033d9..0071567 100644
--- a/services/keepstore/azure_blob_volume.go
+++ b/services/keepstore/azure_blob_volume.go
@@ -311,8 +311,8 @@ func (v *AzureBlobVolume) IndexTo(prefix string, writer io.Writer) error {
 	}
 }
 
-// Delete a Keep block.
-func (v *AzureBlobVolume) Delete(loc string) error {
+// Trash a Keep block.
+func (v *AzureBlobVolume) Trash(loc string) error {
 	if v.readonly {
 		return MethodDisabledError
 	}
@@ -335,6 +335,12 @@ func (v *AzureBlobVolume) Delete(loc string) error {
 	})
 }
 
+// Untrash a Keep block.
+// TBD
+func (v *AzureBlobVolume) Untrash(loc string) error {
+	return nil
+}
+
 // Status returns a VolumeStatus struct with placeholder data.
 func (v *AzureBlobVolume) Status() *VolumeStatus {
 	return &VolumeStatus{
diff --git a/services/keepstore/handler_test.go b/services/keepstore/handler_test.go
index 3817ea1..9526c3c 100644
--- a/services/keepstore/handler_test.go
+++ b/services/keepstore/handler_test.go
@@ -970,3 +970,83 @@ func TestPutReplicationHeader(t *testing.T) {
 		t.Errorf("Got X-Keep-Replicas-Stored: %q, expected %q", r, "1")
 	}
 }
+
+func TestUndeleteHandler(t *testing.T) {
+	defer teardown()
+
+	// Set up Keep volumes
+	KeepVM = MakeTestVolumeManager(2)
+	defer KeepVM.Close()
+	vols := KeepVM.AllWritable()
+	vols[0].Put(TestHash, TestBlock)
+
+	// Only the datamanager user should be allowed to undelete blocks
+	dataManagerToken = "DATA MANAGER TOKEN"
+
+	// unauthenticatedReq => UnauthorizedError
+	unauthenticatedReq := &RequestTester{
+		method: "PUT",
+		uri:    "/undelete/" + TestHash,
+	}
+	response := IssueRequest(unauthenticatedReq)
+	ExpectStatusCode(t,
+		"enforcePermissions on, unauthenticated request",
+		UnauthorizedError.HTTPCode,
+		response)
+
+	// notDataManagerReq => UnauthorizedError
+	notDataManagerReq := &RequestTester{
+		method:   "PUT",
+		uri:      "/undelete/" + TestHash,
+		apiToken: knownToken,
+	}
+
+	response = IssueRequest(notDataManagerReq)
+	ExpectStatusCode(t,
+		"permissions on, unauthenticated /index/prefix request",
+		UnauthorizedError.HTTPCode,
+		response)
+
+	// datamanagerWithBadHashReq => StatusBadRequest
+	datamanagerWithBadHashReq := &RequestTester{
+		method:   "PUT",
+		uri:      "/undelete/thisisnotalocator",
+		apiToken: dataManagerToken,
+	}
+	response = IssueRequest(datamanagerWithBadHashReq)
+	ExpectStatusCode(t,
+		"permissions on, authenticated request, non-superuser",
+		http.StatusBadRequest,
+		response)
+
+	// datamanagerWrongMethodReq => StatusBadRequest
+	datamanagerWrongMethodReq := &RequestTester{
+		method:   "GET",
+		uri:      "/undelete/" + TestHash,
+		apiToken: dataManagerToken,
+	}
+	response = IssueRequest(datamanagerWrongMethodReq)
+	ExpectStatusCode(t,
+		"permissions on, authenticated request, non-superuser",
+		http.StatusBadRequest,
+		response)
+
+	// datamanagerReq => StatusOK
+	datamanagerReq := &RequestTester{
+		method:   "PUT",
+		uri:      "/undelete/" + TestHash,
+		apiToken: dataManagerToken,
+	}
+	response = IssueRequest(datamanagerReq)
+	ExpectStatusCode(t,
+		"permissions on, authenticated request, non-superuser",
+		http.StatusOK,
+		response)
+	expected := `Untrashed on volume`
+	match, _ := regexp.MatchString(expected, response.Body.String())
+	if !match {
+		t.Errorf(
+			"Undelete response mismatched: expected %s, got:\n%s",
+			expected, response.Body.String())
+	}
+}
diff --git a/services/keepstore/handlers.go b/services/keepstore/handlers.go
index 95af1b4..f8e4d71 100644
--- a/services/keepstore/handlers.go
+++ b/services/keepstore/handlers.go
@@ -53,6 +53,9 @@ func MakeRESTRouter() *mux.Router {
 	// Replace the current trash queue.
 	rest.HandleFunc(`/trash`, TrashHandler).Methods("PUT")
 
+	// Undelete moves blocks from trash back into store
+	rest.HandleFunc(`/undelete/{hash:[0-9a-f]{32}}`, UndeleteHandler).Methods("PUT")
+
 	// Any request which does not match any of these routes gets
 	// 400 Bad Request.
 	rest.NotFoundHandler = http.HandlerFunc(BadRequestHandler)
@@ -295,7 +298,7 @@ func DeleteHandler(resp http.ResponseWriter, req *http.Request) {
 		Failed  int `json:"copies_failed"`
 	}
 	for _, vol := range KeepVM.AllWritable() {
-		if err := vol.Delete(hash); err == nil {
+		if err := vol.Trash(hash); err == nil {
 			result.Deleted++
 		} else if os.IsNotExist(err) {
 			continue
@@ -430,6 +433,36 @@ func TrashHandler(resp http.ResponseWriter, req *http.Request) {
 	trashq.ReplaceQueue(tlist)
 }
 
+// UndeleteHandler processes "PUT /undelete/{hash:[0-9a-f]{32}}" requests for the data manager.
+func UndeleteHandler(resp http.ResponseWriter, req *http.Request) {
+	// Reject unauthorized requests.
+	if !IsDataManagerToken(GetApiToken(req)) {
+		http.Error(resp, UnauthorizedError.Error(), UnauthorizedError.HTTPCode)
+		return
+	}
+
+	hash := mux.Vars(req)["hash"]
+	successResp := "Untrashed on volume: "
+	var st int
+	for _, vol := range KeepVM.AllWritable() {
+		if err := vol.Untrash(hash); err == nil {
+			log.Printf("Untrashed %v on volume %v", hash, vol.String())
+			st = http.StatusOK
+			successResp += vol.String()
+			break
+		} else {
+			log.Printf("Error untrashing %v on volume %v: %v", hash, vol.String(), err)
+			st = 500
+		}
+	}
+
+	if st == http.StatusOK {
+		resp.Write([]byte(successResp))
+	}
+
+	resp.WriteHeader(st)
+}
+
 // ==============================
 // GetBlock and PutBlock implement lower-level code for handling
 // blocks by rooting through volumes connected to the local machine.
diff --git a/services/keepstore/keepstore.go b/services/keepstore/keepstore.go
index 96a887f..f6b1f6e 100644
--- a/services/keepstore/keepstore.go
+++ b/services/keepstore/keepstore.go
@@ -79,6 +79,7 @@ var (
 	SizeRequiredError   = &KeepError{411, "Missing Content-Length"}
 	TooLongError        = &KeepError{413, "Block is too large"}
 	MethodDisabledError = &KeepError{405, "Method disabled"}
+	ErrNotImplemented   = &KeepError{500, "Unsupported configuration"}
 )
 
 func (e *KeepError) Error() string {
@@ -113,6 +114,7 @@ var (
 	flagSerializeIO bool
 	flagReadonly    bool
 	volumes         volumeSet
+	trashLifetime   int
 )
 
 func (vs *volumeSet) String() string {
@@ -200,6 +202,11 @@ func main() {
 		"max-buffers",
 		maxBuffers,
 		fmt.Sprintf("Maximum RAM to use for data buffers, given in multiples of block size (%d MiB). When this limit is reached, HTTP requests requiring buffers (like GET and PUT) will wait for buffer space to be released.", BlockSize>>20))
+	flag.IntVar(
+		&trashLifetime,
+		"trash-lifetime",
+		0,
+		fmt.Sprintf("Trashed blocks will stay in trash for trash-lifetime interval before they are actually deleted by the system."))
 
 	flag.Parse()
 
diff --git a/services/keepstore/s3_volume.go b/services/keepstore/s3_volume.go
index 572ee46..16afc32 100644
--- a/services/keepstore/s3_volume.go
+++ b/services/keepstore/s3_volume.go
@@ -257,7 +257,7 @@ func (v *S3Volume) IndexTo(prefix string, writer io.Writer) error {
 	return nil
 }
 
-func (v *S3Volume) Delete(loc string) error {
+func (v *S3Volume) Trash(loc string) error {
 	if v.readonly {
 		return MethodDisabledError
 	}
@@ -272,6 +272,11 @@ func (v *S3Volume) Delete(loc string) error {
 	return v.Bucket.Del(loc)
 }
 
+// TBD
+func (v *S3Volume) Untrash(loc string) error {
+	return nil
+}
+
 func (v *S3Volume) Status() *VolumeStatus {
 	return &VolumeStatus{
 		DeviceNum: 1,
diff --git a/services/keepstore/trash_worker.go b/services/keepstore/trash_worker.go
index 65e3fbd..62f63d5 100644
--- a/services/keepstore/trash_worker.go
+++ b/services/keepstore/trash_worker.go
@@ -47,7 +47,7 @@ func TrashItem(trashRequest TrashRequest) {
 		if neverDelete {
 			err = errors.New("did not delete block because neverDelete is true")
 		} else {
-			err = volume.Delete(trashRequest.Locator)
+			err = volume.Trash(trashRequest.Locator)
 		}
 
 		if err != nil {
diff --git a/services/keepstore/volume.go b/services/keepstore/volume.go
index 7966c41..58710c0 100644
--- a/services/keepstore/volume.go
+++ b/services/keepstore/volume.go
@@ -144,20 +144,21 @@ type Volume interface {
 	// particular order.
 	IndexTo(prefix string, writer io.Writer) error
 
-	// Delete deletes the block data from the underlying storage
-	// device.
+	// Trash moves the block data from the underlying storage
+	// device to trash area. The block then stays in trash for
+	// -trash-lifetime interval before it is actually deleted.
 	//
 	// loc is as described in Get.
 	//
 	// If the timestamp for the given locator is newer than
-	// blobSignatureTTL, Delete must not delete the data.
+	// blobSignatureTTL, Trash must not trash the data.
 	//
-	// If a Delete operation overlaps with any Touch or Put
+	// If a Trash 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
+	//   - Trash does not trash the block, or
 	//   - Both of the above.
 	//
 	// If it is possible for the storage device to be accessed by
@@ -171,9 +172,12 @@ type Volume interface {
 	// reliably or fail outright.
 	//
 	// Corollary: A successful Touch or Put guarantees a block
-	// will not be deleted for at least blobSignatureTTL
+	// will not be trashed for at least blobSignatureTTL
 	// seconds.
-	Delete(loc string) error
+	Trash(loc string) error
+
+	// Untrash moves block from trash back into store
+	Untrash(loc string) error
 
 	// Status returns a *VolumeStatus representing the current
 	// in-use and available storage capacity and an
diff --git a/services/keepstore/volume_generic_test.go b/services/keepstore/volume_generic_test.go
index 7580a20..e168940 100644
--- a/services/keepstore/volume_generic_test.go
+++ b/services/keepstore/volume_generic_test.go
@@ -420,7 +420,7 @@ func testDeleteNewBlock(t TB, factory TestableVolumeFactory) {
 
 	v.Put(TestHash, TestBlock)
 
-	if err := v.Delete(TestHash); err != nil {
+	if err := v.Trash(TestHash); err != nil {
 		t.Error(err)
 	}
 	data, err := v.Get(TestHash)
@@ -449,7 +449,7 @@ func testDeleteOldBlock(t TB, factory TestableVolumeFactory) {
 	v.Put(TestHash, TestBlock)
 	v.TouchWithDate(TestHash, time.Now().Add(-2*blobSignatureTTL))
 
-	if err := v.Delete(TestHash); err != nil {
+	if err := v.Trash(TestHash); err != nil {
 		t.Error(err)
 	}
 	if _, err := v.Get(TestHash); err == nil || !os.IsNotExist(err) {
@@ -463,7 +463,7 @@ func testDeleteNoSuchBlock(t TB, factory TestableVolumeFactory) {
 	v := factory(t)
 	defer v.Teardown()
 
-	if err := v.Delete(TestHash2); err == nil {
+	if err := v.Trash(TestHash2); err == nil {
 		t.Errorf("Expected error when attempting to delete a non-existing block")
 	}
 }
@@ -535,7 +535,7 @@ func testUpdateReadOnly(t TB, factory TestableVolumeFactory) {
 	}
 
 	// Delete a block from a read-only volume should result in error
-	err = v.Delete(TestHash)
+	err = v.Trash(TestHash)
 	if err == nil {
 		t.Errorf("Expected error when deleting block from a read-only volume")
 	}
diff --git a/services/keepstore/volume_test.go b/services/keepstore/volume_test.go
index d671436..53ffeef 100644
--- a/services/keepstore/volume_test.go
+++ b/services/keepstore/volume_test.go
@@ -183,7 +183,7 @@ func (v *MockVolume) IndexTo(prefix string, w io.Writer) error {
 	return nil
 }
 
-func (v *MockVolume) Delete(loc string) error {
+func (v *MockVolume) Trash(loc string) error {
 	v.gotCall("Delete")
 	<-v.Gate
 	if v.Readonly {
@@ -199,6 +199,11 @@ func (v *MockVolume) Delete(loc string) error {
 	return os.ErrNotExist
 }
 
+// TBD
+func (v *MockVolume) Untrash(loc string) error {
+	return nil
+}
+
 func (v *MockVolume) Status() *VolumeStatus {
 	var used uint64
 	for _, block := range v.Store {
diff --git a/services/keepstore/volume_unix.go b/services/keepstore/volume_unix.go
index 910cc25..da1d390 100644
--- a/services/keepstore/volume_unix.go
+++ b/services/keepstore/volume_unix.go
@@ -363,7 +363,7 @@ func (v *UnixVolume) IndexTo(prefix string, w io.Writer) error {
 }
 
 // Delete deletes the block data from the unix storage
-func (v *UnixVolume) Delete(loc string) error {
+func (v *UnixVolume) Trash(loc string) error {
 	// Touch() must be called before calling Write() on a block.  Touch()
 	// also uses lockfile().  This avoids a race condition between Write()
 	// and Delete() because either (a) the file will be deleted and Touch()
@@ -405,6 +405,12 @@ func (v *UnixVolume) Delete(loc string) error {
 	return os.Remove(p)
 }
 
+// Untrash moves block from trash back into store
+// TBD
+func (v *UnixVolume) Untrash(loc string) error {
+	return nil
+}
+
 // blockDir returns the fully qualified directory name for the directory
 // where loc is (or would be) stored on this volume.
 func (v *UnixVolume) blockDir(loc string) string {
diff --git a/services/keepstore/volume_unix_test.go b/services/keepstore/volume_unix_test.go
index b216810..0775e89 100644
--- a/services/keepstore/volume_unix_test.go
+++ b/services/keepstore/volume_unix_test.go
@@ -166,7 +166,7 @@ func TestUnixVolumeReadonly(t *testing.T) {
 		t.Errorf("got err %v, expected MethodDisabledError", err)
 	}
 
-	err = v.Delete(TestHash)
+	err = v.Trash(TestHash)
 	if err != MethodDisabledError {
 		t.Errorf("got err %v, expected MethodDisabledError", err)
 	}

-----------------------------------------------------------------------


hooks/post-receive
-- 




More information about the arvados-commits mailing list