internode lockArgs should use messagepack (#13329)

it would seem like using `bufio.Scan()` is very
slow for heavy concurrent I/O, ie. when r.Body
is slow , instead use a proper
binary exchange format, to marshal and unmarshal
the LockArgs datastructure in a cleaner way.

this PR increases performance of the locking
sub-system for tiny repeated read lock requests
on same object.

```
BenchmarkLockArgs
BenchmarkLockArgs-4              6417609               185.7 ns/op            56 B/op          2 allocs/op
BenchmarkLockArgsOld
BenchmarkLockArgsOld-4           1187368              1015 ns/op            4096 B/op          1 allocs/op
```

cmd/lock-rest-client.go

@@ -22,7 +22,6 @@ import (
 	"context"
 	"io"
 	"net/url"
-	"strconv"
 
 	"github.com/minio/minio/internal/dsync"
 	"github.com/minio/minio/internal/http"
@@ -89,17 +88,13 @@ func (client *lockRESTClient) Close() error {
 
 // restCall makes a call to the lock REST server.
 func (client *lockRESTClient) restCall(ctx context.Context, call string, args dsync.LockArgs) (reply bool, err error) {
-	values := url.Values{}
-	values.Set(lockRESTUID, args.UID)
-	values.Set(lockRESTOwner, args.Owner)
-	values.Set(lockRESTSource, args.Source)
-	values.Set(lockRESTQuorum, strconv.Itoa(args.Quorum))
-	var buffer bytes.Buffer
-	for _, resource := range args.Resources {
-		buffer.WriteString(resource)
-		buffer.WriteString("\n")
+	argsBytes, err := args.MarshalMsg(metaDataPoolGet()[:0])
+	if err != nil {
+		return false, err
 	}
-	respBody, err := client.callWithContext(ctx, call, values, &buffer, -1)
+	defer metaDataPoolPut(argsBytes)
+	body := bytes.NewReader(argsBytes)
+	respBody, err := client.callWithContext(ctx, call, nil, body, body.Size())
 	defer http.DrainBody(respBody)
 	switch err {
 	case nil:

cmd/lock-rest-server-common.go

@@ -22,7 +22,7 @@ import (
 )
 
 const (
-	lockRESTVersion       = "v6" // Add Refresh API
+	lockRESTVersion       = "v7" // Add msgp for lockArgs
 	lockRESTVersionPrefix = SlashSeparator + lockRESTVersion
 	lockRESTPrefix        = minioReservedBucketPath + "/lock"
 )
@@ -35,20 +35,6 @@ const (
 	lockRESTMethodUnlock      = "/unlock"
 	lockRESTMethodRUnlock     = "/runlock"
 	lockRESTMethodForceUnlock = "/force-unlock"
-
-	// lockRESTOwner represents owner UUID
-	lockRESTOwner = "owner"
-
-	// Unique ID of lock/unlock request.
-	lockRESTUID = "uid"
-
-	// Source contains the line number, function and file name of the code
-	// on the client node that requested the lock.
-	lockRESTSource = "source"
-
-	// Quroum value to be saved along lock requester info, useful
-	// in verifying stale locks
-	lockRESTQuorum = "quorum"
 )
 
 var (

cmd/lock-rest-server.go

@@ -18,14 +18,13 @@
 package cmd
 
 import (
-	"bufio"
 	"context"
 	"errors"
+	"io"
 	"net/http"
-	"sort"
-	"strconv"
 	"time"
 
+	"github.com/dustin/go-humanize"
 	"github.com/gorilla/mux"
 	"github.com/minio/minio/internal/dsync"
 )
@@ -63,32 +62,10 @@ func (l *lockRESTServer) IsValid(w http.ResponseWriter, r *http.Request) bool {
 }
 
 func getLockArgs(r *http.Request) (args dsync.LockArgs, err error) {
-	values := r.Form
-	quorum, err := strconv.Atoi(values.Get(lockRESTQuorum))
-	if err != nil {
-		return args, err
-	}
-
-	args = dsync.LockArgs{
-		Owner:  values.Get(lockRESTOwner),
-		UID:    values.Get(lockRESTUID),
-		Source: values.Get(lockRESTSource),
-		Quorum: quorum,
-	}
-
-	var resources []string
-	bio := bufio.NewScanner(r.Body)
-	for bio.Scan() {
-		resources = append(resources, bio.Text())
-	}
-
-	if err := bio.Err(); err != nil {
-		return args, err
-	}
-
-	sort.Strings(resources)
-	args.Resources = resources
-	return args, nil
+	dec := msgpNewReader(io.LimitReader(r.Body, 1000*humanize.KiByte))
+	defer readMsgpReaderPool.Put(dec)
+	err = args.DecodeMsg(dec)
+	return args, err
 }
 
 // HealthHandler returns success if request is authenticated.

cmd/lock-rest-server_test.go

@@ -0,0 +1,104 @@
+// Copyright (c) 2015-2021 MinIO, Inc.
+//
+// This file is part of MinIO Object Storage stack
+//
+// This program is free software: you can redistribute it and/or modify
+// it under the terms of the GNU Affero General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU Affero General Public License for more details.
+//
+// You should have received a copy of the GNU Affero General Public License
+// along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+package cmd
+
+import (
+	"bufio"
+	"bytes"
+	"io/ioutil"
+	"net/http"
+	"net/url"
+	"sort"
+	"strconv"
+	"testing"
+
+	"github.com/minio/minio/internal/dsync"
+)
+
+func BenchmarkLockArgs(b *testing.B) {
+	args := dsync.LockArgs{
+		Owner:     "minio",
+		UID:       "uid",
+		Source:    "lockArgs.go",
+		Quorum:    3,
+		Resources: []string{"obj.txt"},
+	}
+
+	argBytes, err := args.MarshalMsg(nil)
+	if err != nil {
+		b.Fatal(err)
+	}
+
+	req := &http.Request{
+		Body: ioutil.NopCloser(bytes.NewReader(argBytes)),
+	}
+
+	b.ReportAllocs()
+	b.ResetTimer()
+	for i := 0; i < b.N; i++ {
+		getLockArgs(req)
+	}
+}
+
+func BenchmarkLockArgsOld(b *testing.B) {
+	values := url.Values{}
+	values.Set("owner", "minio")
+	values.Set("uid", "uid")
+	values.Set("source", "lockArgs.go")
+	values.Set("quorum", "3")
+
+	req := &http.Request{
+		Form: values,
+		Body: ioutil.NopCloser(bytes.NewReader([]byte(`obj.txt`))),
+	}
+
+	b.ReportAllocs()
+	b.ResetTimer()
+	for i := 0; i < b.N; i++ {
+		getLockArgsOld(req)
+	}
+}
+
+func getLockArgsOld(r *http.Request) (args dsync.LockArgs, err error) {
+	values := r.Form
+	quorum, err := strconv.Atoi(values.Get("quorum"))
+	if err != nil {
+		return args, err
+	}
+
+	args = dsync.LockArgs{
+		Owner:  values.Get("onwer"),
+		UID:    values.Get("uid"),
+		Source: values.Get("source"),
+		Quorum: quorum,
+	}
+
+	var resources []string
+	bio := bufio.NewScanner(r.Body)
+	for bio.Scan() {
+		resources = append(resources, bio.Text())
+	}
+
+	if err := bio.Err(); err != nil {
+		return args, err
+	}
+
+	sort.Strings(resources)
+	args.Resources = resources
+	return args, nil
+}

cmd/storage-errors.go

@@ -157,7 +157,10 @@ func osErrToFileErr(err error) error {
 		return errFaultyDisk
 	}
 	if isSysErrInvalidArg(err) {
-		return errUnsupportedDisk
+		// For some odd calls with O_DIRECT reads
+		// filesystems can return EINVAL, handle
+		// these as FileNotFound instead.
+		return errFileNotFound
 	}
 	if isSysErrNoSpace(err) {
 		return errDiskFull