{"action":"created","issue":{"url":"https://api.github.com/repos/minio/minio/issues/3203","repository_url":"https://api.github.com/repos/minio/minio","labels_url":"https://api.github.com/repos/minio/minio/issues/3203/labels{/name}","comments_url":"https://api.github.com/repos/minio/minio/issues/3203/comments","events_url":"https://api.github.com/repos/minio/minio/issues/3203/events","html_url":"https://github.com/minio/minio/pull/3203","id":187901931,"number":3203,"title":"[WIP] Improve namespace lock API","user":{"login":"donatello","id":200059,"avatar_url":"https://avatars.githubusercontent.com/u/200059?v=3","gravatar_id":"","url":"https://api.github.com/users/donatello","html_url":"https://github.com/donatello","followers_url":"https://api.github.com/users/donatello/followers","following_url":"https://api.github.com/users/donatello/following{/other_user}","gists_url":"https://api.github.com/users/donatello/gists{/gist_id}","starred_url":"https://api.github.com/users/donatello/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/donatello/subscriptions","organizations_url":"https://api.github.com/users/donatello/orgs","repos_url":"https://api.github.com/users/donatello/repos","events_url":"https://api.github.com/users/donatello/events{/privacy}","received_events_url":"https://api.github.com/users/donatello/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":9,"created_at":"2016-11-08T04:55:16Z","updated_at":"2016-11-09T02:03:04Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/minio/minio/pulls/3203","html_url":"https://github.com/minio/minio/pull/3203","diff_url":"https://github.com/minio/minio/pull/3203.diff","patch_url":"https://github.com/minio/minio/pull/3203.patch"},"body":"<!--- Provide a general summary of your changes in the Title above -->\r\n\r\n## Description\r\n<!--- Describe your changes in detail -->\r\nImprove lock interface:\r\n\r\n- abstract out instrumentation information.\r\n\r\n- make code more readable\r\n\r\n- TODO: (DO NOT MERGE YET!) update all namespace lock calls to use this\r\n  interface\r\n\r\n\r\n## Motivation and Context\r\n<!--- Why is this change required? What problem does it solve? -->\r\n<!--- If it fixes an open issue, please link to the issue here. -->\r\n\r\nCurrently, all namespace locking code is littered with a call to generate an opsID for instrumentation and is not very readable, for e.g. this snippet from `xl-v1-multipart.go` in `PutObjectPart`:\r\n\r\n```go\r\n\t// get a random ID for lock instrumentation.\r\n\topsID := getOpsID()\r\n\r\n\tnsMutex.RLock(minioMetaBucket, uploadIDPath, opsID)\r\n\t// Validates if upload ID exists.\r\n\tif !xl.isUploadIDExists(bucket, object, uploadID) {\r\n\t\tnsMutex.RUnlock(minioMetaBucket, uploadIDPath, opsID)\r\n\t\treturn \"\", traceError(InvalidUploadID{UploadID: uploadID})\r\n\t}\r\n\t// Read metadata associated with the object from all disks.\r\n\tpartsMetadata, errs = readAllXLMetadata(xl.storageDisks, minioMetaBucket,\r\n\t\tuploadIDPath)\r\n\tif !isDiskQuorum(errs, xl.writeQuorum) {\r\n\t\tnsMutex.RUnlock(minioMetaBucket, uploadIDPath, opsID)\r\n\t\treturn \"\", toObjectErr(traceError(errXLWriteQuorum), bucket, object)\r\n\t}\r\n\tnsMutex.RUnlock(minioMetaBucket, uploadIDPath, opsID)\r\n\r\n```\r\n\r\nWith the proposed interface change (please refer to the content of the commit in this PR), we could write this snippet as:\r\n\r\n```go\r\n\t// initialize uploadIdPath lock\r\n\tuploadIDLock := nsMutex.NewNSLock(minioMetaBucket, uploadIDPath)\r\n\r\n\tuploadIDLock.RLock()\r\n\t// Validates if upload ID exists.\r\n\tif !xl.isUploadIDExists(bucket, object, uploadID) {\r\n\t\tuploadIDLock.RUnlock()\r\n\t\treturn \"\", traceError(InvalidUploadID{UploadID: uploadID})\r\n\t}\r\n\t// Read metadata associated with the object from all disks.\r\n\tpartsMetadata, errs = readAllXLMetadata(xl.storageDisks, minioMetaBucket,\r\n\t\tuploadIDPath)\r\n\tif !isDiskQuorum(errs, xl.writeQuorum) {\r\n\t\tuploadIDLock.RUnlock()\r\n\t\treturn \"\", toObjectErr(traceError(errXLWriteQuorum), bucket, object)\r\n\t}\r\n\tuploadIDLock.RUnlock()\r\n\r\n```\r\n\r\nTagging @harshavardhana @abperiasamy @krishnasrinivas @fwessels @hackintoshrao for inputs.\r\n\r\n## Types of changes\r\n<!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: -->\r\n- [ ] Bug fix (non-breaking change which fixes an issue)\r\n- [ ] New feature (non-breaking change which adds functionality)\r\n- [ ] Breaking change (fix or feature that would cause existing functionality to change)\r\n- [x] Code enhancement\r\n\r\n## Checklist:\r\n<!--- Go over all the following points, and put an `x` in all the boxes that apply. -->\r\n<!--- If you're unsure about any of these, don't hesitate to ask. We're here to help! -->\r\n- [x] My code follows the code style of this project.\r\n- [ ] My change requires a change to the documentation.\r\n- [ ] I have updated the documentation accordingly.\r\n- [ ] I have added tests to cover my changes.\r\n- [ ] All new and existing tests passed.\r\n"},"comment":{"url":"https://api.github.com/repos/minio/minio/issues/comments/259316721","html_url":"https://github.com/minio/minio/pull/3203#issuecomment-259316721","issue_url":"https://api.github.com/repos/minio/minio/issues/3203","id":259316721,"user":{"login":"harshavardhana","id":622699,"avatar_url":"https://avatars.githubusercontent.com/u/622699?v=3","gravatar_id":"","url":"https://api.github.com/users/harshavardhana","html_url":"https://github.com/harshavardhana","followers_url":"https://api.github.com/users/harshavardhana/followers","following_url":"https://api.github.com/users/harshavardhana/following{/other_user}","gists_url":"https://api.github.com/users/harshavardhana/gists{/gist_id}","starred_url":"https://api.github.com/users/harshavardhana/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/harshavardhana/subscriptions","organizations_url":"https://api.github.com/users/harshavardhana/orgs","repos_url":"https://api.github.com/users/harshavardhana/repos","events_url":"https://api.github.com/users/harshavardhana/events{/privacy}","received_events_url":"https://api.github.com/users/harshavardhana/received_events","type":"User","site_admin":false},"created_at":"2016-11-09T02:03:04Z","updated_at":"2016-11-09T02:03:04Z","body":"Can you also paste the output of 'minio control lock' after this change? want see if it affected the printing. "}}
{"actor":{"display_login":"harshavardhana"}}