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

@@ -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
 	}()
 

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
 	}()
 

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
-}

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,
+// return randomly generated string ID
-// else returns empty string
+func getOpsID() string {
-func getOpsID() (opsID string) {
+	return string(generateRequestID())
-	// check if lock debug is enabled.
-	if globalDebugLock {
-		// generated random ID.
-		opsID = string(generateRequestID())
-	}
-	return opsID
 }

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.

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() {

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.
+	// Initialize nsLockMap with entry for instrumentation
-		// entries of <volume,path> -> stateInfo of locks, for instrumentation purpose.
+	// information.
+	// Entries of <volume,path> -> stateInfo of locks
 	nsMutex.debugLockMap = make(map[nsParam]*debugLockInfoPerVolumePath)
-	}
 }
 
 func (n *nsLockMap) initLockInfoForVolumePath(param nsParam) {
@@ -131,14 +131,13 @@ 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
-		// Change the state of the lock to be blocked for the given pair of <volume, path>
+	// pair of <volume, path> and <OperationID> till the lock
-		// and <OperationID> till the lock unblocks. The lock for accessing `nsMutex` is
+	// unblocks. The lock for accessing `nsMutex` is held inside
-		// held inside the function itself.
+	// 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.
 	n.lockMapMutex.Unlock()
@@ -150,20 +149,19 @@ func (n *nsLockMap) lock(volume, path string, lockOrigin, opsID string, readLock
 		nsLk.Lock()
 	}
 
-	// Check if lock debugging enabled.
+	// Changing the status of the operation from blocked to
-	if globalDebugLock {
+	// running.  change the state of the lock to be running (from
-		// Changing the status of the operation from blocked to running.
+	// blocked) for the given pair of <volume, path> and
-		// change the state of the lock to be  running (from blocked) for
+	// <OperationID>.
-		// the given pair of <volume, path> and <OperationID>.
 	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,48 +173,51 @@ 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 {
+			// 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 <volume, path> pair.
+			// delete the lock state entry for given
-			if globalDebugLock {
+			// <volume, path> pair.
 			err := n.deleteLockInfoEntryForVolumePath(param)
 			if err != nil {
 				errorIf(err, "Failed to delete lock info entry.")
 			}
 		}
 	}
-	}
 }
 
 // Lock - locks the given resource for writes, using a previously
 // 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.
+	// The caller information of the lock held has been obtained
-	if globalDebugLock {
+	// 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_ .
+	// 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.")
+		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)
 	}
+	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.
+	// The caller information of the lock held has been obtained
-	if globalDebugLock {
+	// 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_ .
+	// 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.")
+		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)
 	}
+	lockOrigin = fmt.Sprintf("[lock held] in %s[%s:%d]",
+		runtime.FuncForPC(pc).Name(), fn, line)
+
 	n.lock(volume, path, lockOrigin, opsID, readLock)
 }
 

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
 	}()