From 8ea571c7f7f275bc2364259df624725cb9f03d92 Mon Sep 17 00:00:00 2001 From: Aditya Manthramurthy Date: Tue, 27 Sep 2016 14:35:43 -0700 Subject: [PATCH] Remove MINIO_DEBUG environment variable (#2794) Removes the unimplemented settings of MINIO_DEBUG=mem and makes MINIO_DEBUG=lock the default behaviour. --- cmd/control-mains_test.go | 3 - cmd/controller_test.go | 3 - cmd/globals.go | 16 ----- cmd/lock-instrument.go | 12 +--- cmd/lock-instrument_test.go | 12 ---- cmd/main.go | 3 - cmd/namespace-lock.go | 113 +++++++++++++++++++----------------- cmd/namespace-lock_test.go | 3 - 8 files changed, 62 insertions(+), 103 deletions(-) diff --git a/cmd/control-mains_test.go b/cmd/control-mains_test.go index 20ababe9a..e01ac5e80 100644 --- a/cmd/control-mains_test.go +++ b/cmd/control-mains_test.go @@ -59,14 +59,11 @@ func TestControlLockMain(t *testing.T) { // schedule cleanup at the end defer testServer.Stop() - // enabling lock instrumentation. - globalDebugLock = true // initializing the locks. initNSLock(false) // set debug lock info to `nil` so that other tests do not see // such modified env settings. defer func() { - globalDebugLock = false nsMutex.debugLockMap = nil }() diff --git a/cmd/controller_test.go b/cmd/controller_test.go index 4525d66c3..f815b1afb 100644 --- a/cmd/controller_test.go +++ b/cmd/controller_test.go @@ -66,14 +66,11 @@ func TestRPCControlLock(t *testing.T) { // Tests to validate the correctness of lock instrumentation control RPC end point. func (s *TestRPCControllerSuite) testRPCControlLock(c *testing.T) { - // enabling lock instrumentation. - globalDebugLock = true // initializing the locks. initNSLock(false) // set debug lock info to `nil` so that the next tests have to // initialize them again. defer func() { - globalDebugLock = false nsMutex.debugLockMap = nil }() diff --git a/cmd/globals.go b/cmd/globals.go index c79445521..488ad3483 100644 --- a/cmd/globals.go +++ b/cmd/globals.go @@ -21,7 +21,6 @@ import ( "github.com/fatih/color" "github.com/minio/minio/pkg/objcache" - "os" ) // Global constants for Minio. @@ -44,9 +43,6 @@ var ( globalQuiet = false // Quiet flag set via command line globalTrace = false // Trace flag set via environment setting. - globalDebug = false // Debug flag set to print debug info. - globalDebugLock = false // Lock debug info set via environment variable MINIO_DEBUG=lock . - globalDebugMemory = false // Memory debug info set via environment variable MINIO_DEBUG=mem // Add new global flags here. // Maximum connections handled per @@ -75,15 +71,3 @@ var ( colorBlue = color.New(color.FgBlue).SprintfFunc() colorBold = color.New(color.Bold).SprintFunc() ) - -// fetch from environment variables and set the global values related to locks. -func setGlobalsDebugFromEnv() { - debugEnv := os.Getenv("MINIO_DEBUG") - switch debugEnv { - case "lock": - globalDebugLock = true - case "mem": - globalDebugMemory = true - } - globalDebug = globalDebugLock || globalDebugMemory -} diff --git a/cmd/lock-instrument.go b/cmd/lock-instrument.go index 5b0bd139e..90f689409 100644 --- a/cmd/lock-instrument.go +++ b/cmd/lock-instrument.go @@ -271,13 +271,7 @@ func (n *nsLockMap) deleteLockInfoEntryForOps(param nsParam, operationID string) return nil } -// return randomly generated string ID if lock debug is enabled, -// else returns empty string -func getOpsID() (opsID string) { - // check if lock debug is enabled. - if globalDebugLock { - // generated random ID. - opsID = string(generateRequestID()) - } - return opsID +// return randomly generated string ID +func getOpsID() string { + return string(generateRequestID()) } diff --git a/cmd/lock-instrument_test.go b/cmd/lock-instrument_test.go index 966d48e2e..1fdb37e58 100644 --- a/cmd/lock-instrument_test.go +++ b/cmd/lock-instrument_test.go @@ -389,13 +389,10 @@ func TestNsLockMapStatusBlockedToRunning(t *testing.T) { t.Fatalf("Errors mismatch: Expected: \"%s\", got: \"%s\"", expectedBlockErr, actualErr) } - // enabling lock instrumentation. - globalDebugLock = true // initializing the locks. initNSLock(false) // set debug lock info to `nil` so that the next tests have to initialize them again. defer func() { - globalDebugLock = false nsMutex.debugLockMap = nil }() // Iterate over the cases and assert the result. @@ -531,13 +528,10 @@ func TestNsLockMapStatusNoneToBlocked(t *testing.T) { if actualErr != expectedNilErr { t.Fatalf("Errors mismatch: Expected \"%s\", got \"%s\"", expectedNilErr, actualErr) } - // enabling lock instrumentation. - globalDebugLock = true // initializing the locks. initNSLock(false) // set debug lock info to `nil` so that the next tests have to initialize them again. defer func() { - globalDebugLock = false nsMutex.debugLockMap = nil }() // Iterate over the cases and assert the result. @@ -580,13 +574,10 @@ func TestNsLockMapDeleteLockInfoEntryForOps(t *testing.T) { t.Fatalf("Errors mismatch: Expected \"%s\", got \"%s\"", expectedNilErr, actualErr) } - // enabling lock instrumentation. - globalDebugLock = true // initializing the locks. initNSLock(false) // set debug lock info to `nil` so that the next tests have to initialize them again. defer func() { - globalDebugLock = false nsMutex.debugLockMap = nil }() // case - 2. @@ -681,13 +672,10 @@ func TestNsLockMapDeleteLockInfoEntryForVolumePath(t *testing.T) { t.Fatalf("Errors mismatch: Expected \"%s\", got \"%s\"", expectedNilErr, actualErr) } - // enabling lock instrumentation. - globalDebugLock = true // initializing the locks. initNSLock(false) // set debug lock info to `nil` so that the next tests have to initialize them again. defer func() { - globalDebugLock = false nsMutex.debugLockMap = nil }() // case - 2. diff --git a/cmd/main.go b/cmd/main.go index 0235065d7..e607dce40 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -73,9 +73,6 @@ func init() { // Set global trace flag. globalTrace = os.Getenv("MINIO_TRACE") == "1" - - // Set all the debug flags from ENV if any. - setGlobalsDebugFromEnv() } func migrate() { diff --git a/cmd/namespace-lock.go b/cmd/namespace-lock.go index e35dceb9f..4b3cdae0e 100644 --- a/cmd/namespace-lock.go +++ b/cmd/namespace-lock.go @@ -64,11 +64,11 @@ func initNSLock(isDist bool) { isDist: isDist, lockMap: make(map[nsParam]*nsLock), } - if globalDebugLock { - // lock Debugging enabed, initialize nsLockMap with entry for debugging information. - // entries of -> stateInfo of locks, for instrumentation purpose. - nsMutex.debugLockMap = make(map[nsParam]*debugLockInfoPerVolumePath) - } + + // Initialize nsLockMap with entry for instrumentation + // information. + // Entries of -> stateInfo of locks + nsMutex.debugLockMap = make(map[nsParam]*debugLockInfoPerVolumePath) } func (n *nsLockMap) initLockInfoForVolumePath(param nsParam) { @@ -131,13 +131,12 @@ func (n *nsLockMap) lock(volume, path string, lockOrigin, opsID string, readLock } nsLk.ref++ // Update ref count here to avoid multiple races. - if globalDebugLock { - // Change the state of the lock to be blocked for the given pair of - // and till the lock unblocks. The lock for accessing `nsMutex` is - // held inside the function itself. - if err := n.statusNoneToBlocked(param, lockOrigin, opsID, readLock); err != nil { - errorIf(err, "Failed to set lock state to blocked.") - } + // Change the state of the lock to be blocked for the given + // pair of and till the lock + // unblocks. The lock for accessing `nsMutex` is held inside + // the function itself. + if err := n.statusNoneToBlocked(param, lockOrigin, opsID, readLock); err != nil { + errorIf(err, "Failed to set lock state to blocked.") } // Unlock map before Locking NS which might block. @@ -150,20 +149,19 @@ func (n *nsLockMap) lock(volume, path string, lockOrigin, opsID string, readLock nsLk.Lock() } - // Check if lock debugging enabled. - if globalDebugLock { - // Changing the status of the operation from blocked to running. - // change the state of the lock to be running (from blocked) for - // the given pair of and . - if err := n.statusBlockedToRunning(param, lockOrigin, opsID, readLock); err != nil { - errorIf(err, "Failed to set the lock state to running.") - } + // Changing the status of the operation from blocked to + // running. change the state of the lock to be running (from + // blocked) for the given pair of and + // . + if err := n.statusBlockedToRunning(param, lockOrigin, opsID, readLock); err != nil { + errorIf(err, "Failed to set the lock state to running.") } } // Unlock the namespace resource. func (n *nsLockMap) unlock(volume, path, opsID string, readLock bool) { - // nsLk.Unlock() will not block, hence locking the map for the entire function is fine. + // nsLk.Unlock() will not block, hence locking the map for the + // entire function is fine. n.lockMapMutex.Lock() defer n.lockMapMutex.Unlock() @@ -175,28 +173,27 @@ func (n *nsLockMap) unlock(volume, path, opsID string, readLock bool) { nsLk.Unlock() } if nsLk.ref == 0 { - errorIf(errors.New("Namespace reference count cannot be 0."), "Invalid reference count detected.") + errorIf(errors.New("Namespace reference count cannot be 0."), + "Invalid reference count detected.") } if nsLk.ref != 0 { nsLk.ref-- - // Locking debug enabled, delete the lock state entry for given operation ID. - if globalDebugLock { - err := n.deleteLockInfoEntryForOps(param, opsID) - if err != nil { - errorIf(err, "Failed to delete lock info entry.") - } + + // delete the lock state entry for given operation ID. + err := n.deleteLockInfoEntryForOps(param, opsID) + if err != nil { + errorIf(err, "Failed to delete lock info entry.") } } if nsLk.ref == 0 { // Remove from the map if there are no more references. delete(n.lockMap, param) - // Locking debug enabled, delete the lock state entry for given pair. - if globalDebugLock { - err := n.deleteLockInfoEntryForVolumePath(param) - if err != nil { - errorIf(err, "Failed to delete lock info entry.") - } + // delete the lock state entry for given + // pair. + err := n.deleteLockInfoEntryForVolumePath(param) + if err != nil { + errorIf(err, "Failed to delete lock info entry.") } } } @@ -206,17 +203,21 @@ func (n *nsLockMap) unlock(volume, path, opsID string, readLock bool) { // allocated name space lock or initializing a new one. func (n *nsLockMap) Lock(volume, path, opsID string) { var lockOrigin string - // Lock debugging enabled. The caller information of the lock held has - // been obtained here before calling any other function. - if globalDebugLock { - // Fetching the package, function name and the line number of the caller from the runtime. - // here is an example https://play.golang.org/p/perrmNRI9_ . - pc, fn, line, success := runtime.Caller(1) - if !success { - errorIf(errors.New("Couldn't get caller info."), "Fetching caller info form runtime failed.") - } - lockOrigin = fmt.Sprintf("[lock held] in %s[%s:%d]", runtime.FuncForPC(pc).Name(), fn, line) + + // The caller information of the lock held has been obtained + // here before calling any other function. + + // Fetching the package, function name and the line number of + // the caller from the runtime. here is an example + // https://play.golang.org/p/perrmNRI9_ . + pc, fn, line, success := runtime.Caller(1) + if !success { + errorIf(errors.New("Couldn't get caller info."), + "Fetching caller info form runtime failed.") } + lockOrigin = fmt.Sprintf("[lock held] in %s[%s:%d]", + runtime.FuncForPC(pc).Name(), fn, line) + readLock := false n.lock(volume, path, lockOrigin, opsID, readLock) } @@ -231,17 +232,21 @@ func (n *nsLockMap) Unlock(volume, path, opsID string) { func (n *nsLockMap) RLock(volume, path, opsID string) { var lockOrigin string readLock := true - // Lock debugging enabled. The caller information of the lock held has - // been obtained here before calling any other function. - if globalDebugLock { - // Fetching the package, function name and the line number of the - // caller from the runtime. Here is an example https://play.golang.org/p/perrmNRI9_ . - pc, fn, line, success := runtime.Caller(1) - if !success { - errorIf(errors.New("Couldn't get caller info."), "Fetching caller info form runtime failed.") - } - lockOrigin = fmt.Sprintf("[lock held] in %s[%s:%d]", runtime.FuncForPC(pc).Name(), fn, line) + + // The caller information of the lock held has been obtained + // here before calling any other function. + + // Fetching the package, function name and the line number of + // the caller from the runtime. Here is an example + // https://play.golang.org/p/perrmNRI9_ . + pc, fn, line, success := runtime.Caller(1) + if !success { + errorIf(errors.New("Couldn't get caller info."), + "Fetching caller info form runtime failed.") } + lockOrigin = fmt.Sprintf("[lock held] in %s[%s:%d]", + runtime.FuncForPC(pc).Name(), fn, line) + n.lock(volume, path, lockOrigin, opsID, readLock) } diff --git a/cmd/namespace-lock_test.go b/cmd/namespace-lock_test.go index b2e359659..7e715be24 100644 --- a/cmd/namespace-lock_test.go +++ b/cmd/namespace-lock_test.go @@ -298,14 +298,11 @@ func TestLockStats(t *testing.T) { }, } var wg sync.WaitGroup - // enabling lock instrumentation. - globalDebugLock = true // initializing the locks. initNSLock(false) // set debug lock info to `nil` so that the next tests have to initialize them again. defer func() { - globalDebugLock = false nsMutex.debugLockMap = nil }()