change Read* calls over net/http to move to http.MethodGet (#20173)

- ReadVersion
- ReadFile
- ReadXL

Further changes include to

- Compact internode resource RPC paths
- Compact internode query params

To optimize on parsing by gorilla/mux as the
length of this string increases latency in
gorilla/mux - reduce to a meaningful string.
This commit is contained in:
Harshavardhana 2024-07-29 01:00:12 -07:00 committed by GitHub
parent c87a489514
commit 3ae104edae
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 68 additions and 74 deletions

View File

@ -173,14 +173,28 @@ func (client *storageRESTClient) GetDiskLoc() (poolIdx, setIdx, diskIdx int) {
return client.endpoint.PoolIdx, client.endpoint.SetIdx, client.endpoint.DiskIdx return client.endpoint.PoolIdx, client.endpoint.SetIdx, client.endpoint.DiskIdx
} }
// Wrapper to restClient.Call to handle network errors, in case of network error the connection is disconnected // Wrapper to restClient.CallWithMethod to handle network errors, in case of network error the connection is disconnected
// and a healthcheck routine gets invoked that would reconnect. // and a healthcheck routine gets invoked that would reconnect.
func (client *storageRESTClient) call(ctx context.Context, method string, values url.Values, body io.Reader, length int64) (io.ReadCloser, error) { func (client *storageRESTClient) callGet(ctx context.Context, rpcMethod string, values url.Values, body io.Reader, length int64) (io.ReadCloser, error) {
if values == nil { if values == nil {
values = make(url.Values) values = make(url.Values)
} }
values.Set(storageRESTDiskID, *client.diskID.Load()) values.Set(storageRESTDiskID, *client.diskID.Load())
respBody, err := client.restClient.Call(ctx, method, values, body, length) respBody, err := client.restClient.CallWithHTTPMethod(ctx, http.MethodGet, rpcMethod, values, body, length)
if err != nil {
return nil, toStorageErr(err)
}
return respBody, nil
}
// Wrapper to restClient.Call to handle network errors, in case of network error the connection is disconnected
// and a healthcheck routine gets invoked that would reconnect.
func (client *storageRESTClient) call(ctx context.Context, rpcMethod string, values url.Values, body io.Reader, length int64) (io.ReadCloser, error) {
if values == nil {
values = make(url.Values)
}
values.Set(storageRESTDiskID, *client.diskID.Load())
respBody, err := client.restClient.CallWithHTTPMethod(ctx, http.MethodPost, rpcMethod, values, body, length)
if err != nil { if err != nil {
return nil, toStorageErr(err) return nil, toStorageErr(err)
} }
@ -526,7 +540,6 @@ func (client *storageRESTClient) ReadVersion(ctx context.Context, origvolume, vo
storageRESTFilePath: path, storageRESTFilePath: path,
storageRESTVersionID: versionID, storageRESTVersionID: versionID,
storageRESTInclFreeVersions: strconv.FormatBool(opts.InclFreeVersions), storageRESTInclFreeVersions: strconv.FormatBool(opts.InclFreeVersions),
storageRESTReadData: strconv.FormatBool(opts.ReadData),
storageRESTHealing: strconv.FormatBool(opts.Healing), storageRESTHealing: strconv.FormatBool(opts.Healing),
})) }))
if err != nil { if err != nil {
@ -541,10 +554,9 @@ func (client *storageRESTClient) ReadVersion(ctx context.Context, origvolume, vo
values.Set(storageRESTFilePath, path) values.Set(storageRESTFilePath, path)
values.Set(storageRESTVersionID, versionID) values.Set(storageRESTVersionID, versionID)
values.Set(storageRESTInclFreeVersions, strconv.FormatBool(opts.InclFreeVersions)) values.Set(storageRESTInclFreeVersions, strconv.FormatBool(opts.InclFreeVersions))
values.Set(storageRESTReadData, strconv.FormatBool(opts.ReadData))
values.Set(storageRESTHealing, strconv.FormatBool(opts.Healing)) values.Set(storageRESTHealing, strconv.FormatBool(opts.Healing))
respBody, err := client.call(ctx, storageRESTMethodReadVersion, values, nil, -1) respBody, err := client.callGet(ctx, storageRESTMethodReadVersion, values, nil, -1)
if err != nil { if err != nil {
return fi, err return fi, err
} }
@ -568,7 +580,6 @@ func (client *storageRESTClient) ReadXL(ctx context.Context, volume string, path
storageRESTDiskID: *client.diskID.Load(), storageRESTDiskID: *client.diskID.Load(),
storageRESTVolume: volume, storageRESTVolume: volume,
storageRESTFilePath: path, storageRESTFilePath: path,
storageRESTReadData: "false",
})) }))
if err != nil { if err != nil {
return rf, toStorageErr(err) return rf, toStorageErr(err)
@ -579,8 +590,8 @@ func (client *storageRESTClient) ReadXL(ctx context.Context, volume string, path
values := make(url.Values) values := make(url.Values)
values.Set(storageRESTVolume, volume) values.Set(storageRESTVolume, volume)
values.Set(storageRESTFilePath, path) values.Set(storageRESTFilePath, path)
values.Set(storageRESTReadData, strconv.FormatBool(readData))
respBody, err := client.call(ctx, storageRESTMethodReadXL, values, nil, -1) respBody, err := client.callGet(ctx, storageRESTMethodReadXL, values, nil, -1)
if err != nil { if err != nil {
return rf, toStorageErr(err) return rf, toStorageErr(err)
} }
@ -617,9 +628,8 @@ func (client *storageRESTClient) ReadFileStream(ctx context.Context, volume, pat
values.Set(storageRESTFilePath, path) values.Set(storageRESTFilePath, path)
values.Set(storageRESTOffset, strconv.Itoa(int(offset))) values.Set(storageRESTOffset, strconv.Itoa(int(offset)))
values.Set(storageRESTLength, strconv.Itoa(int(length))) values.Set(storageRESTLength, strconv.Itoa(int(length)))
values.Set(storageRESTDiskID, *client.diskID.Load())
respBody, err := client.restClient.CallWithHTTPMethod(ctx, http.MethodGet, storageRESTMethodReadFileStream, values, nil, -1) respBody, err := client.callGet(ctx, storageRESTMethodReadFileStream, values, nil, -1)
if err != nil { if err != nil {
return nil, toStorageErr(err) return nil, toStorageErr(err)
} }
@ -640,7 +650,7 @@ func (client *storageRESTClient) ReadFile(ctx context.Context, volume string, pa
values.Set(storageRESTBitrotAlgo, "") values.Set(storageRESTBitrotAlgo, "")
values.Set(storageRESTBitrotHash, "") values.Set(storageRESTBitrotHash, "")
} }
respBody, err := client.call(ctx, storageRESTMethodReadFile, values, nil, -1) respBody, err := client.callGet(ctx, storageRESTMethodReadFile, values, nil, -1)
if err != nil { if err != nil {
return 0, err return 0, err
} }

View File

@ -20,7 +20,7 @@ package cmd
//go:generate msgp -file $GOFILE -unexported //go:generate msgp -file $GOFILE -unexported
const ( const (
storageRESTVersion = "v60" // ReadFileStream now uses http.MethodGet storageRESTVersion = "v61" // Move all Read* calls to http.MethodGet, compact handlers and query params fields
storageRESTVersionPrefix = SlashSeparator + storageRESTVersion storageRESTVersionPrefix = SlashSeparator + storageRESTVersion
storageRESTPrefix = minioReservedBucketPath + "/storage" storageRESTPrefix = minioReservedBucketPath + "/storage"
) )
@ -28,48 +28,48 @@ const (
const ( const (
storageRESTMethodHealth = "/health" storageRESTMethodHealth = "/health"
storageRESTMethodAppendFile = "/appendfile" storageRESTMethodAppendFile = "/afile"
storageRESTMethodCreateFile = "/createfile" storageRESTMethodCreateFile = "/cfile"
storageRESTMethodWriteAll = "/writeall" storageRESTMethodWriteAll = "/wall"
storageRESTMethodReadVersion = "/readversion" storageRESTMethodReadVersion = "/rver"
storageRESTMethodReadXL = "/readxl" storageRESTMethodReadXL = "/rxl"
storageRESTMethodReadAll = "/readall" storageRESTMethodReadAll = "/rall"
storageRESTMethodReadFile = "/readfile" storageRESTMethodReadFile = "/rfile"
storageRESTMethodReadFileStream = "/readfilestream" storageRESTMethodReadFileStream = "/rfilest"
storageRESTMethodListDir = "/listdir" storageRESTMethodListDir = "/ls"
storageRESTMethodDeleteVersions = "/deleteverions" storageRESTMethodDeleteVersions = "/dvers"
storageRESTMethodRenameFile = "/renamefile" storageRESTMethodRenameFile = "/rfile"
storageRESTMethodVerifyFile = "/verifyfile" storageRESTMethodVerifyFile = "/vfile"
storageRESTMethodStatInfoFile = "/statfile" storageRESTMethodStatInfoFile = "/sfile"
storageRESTMethodReadMultiple = "/readmultiple" storageRESTMethodReadMultiple = "/rmpl"
storageRESTMethodCleanAbandoned = "/cleanabandoned" storageRESTMethodCleanAbandoned = "/cln"
) )
const ( const (
storageRESTVolume = "volume" storageRESTVolume = "vol"
storageRESTVolumes = "volumes" storageRESTVolumes = "vols"
storageRESTDirPath = "dir-path" storageRESTDirPath = "dpath"
storageRESTFilePath = "file-path" storageRESTFilePath = "fp"
storageRESTVersionID = "version-id" storageRESTVersionID = "vid"
storageRESTReadData = "read-data" storageRESTHealing = "heal"
storageRESTHealing = "healing" storageRESTTotalVersions = "tvers"
storageRESTTotalVersions = "total-versions" storageRESTSrcVolume = "svol"
storageRESTSrcVolume = "source-volume" storageRESTSrcPath = "spath"
storageRESTSrcPath = "source-path" storageRESTDstVolume = "dvol"
storageRESTDstVolume = "destination-volume" storageRESTDstPath = "dpath"
storageRESTDstPath = "destination-path"
storageRESTOffset = "offset" storageRESTOffset = "offset"
storageRESTLength = "length" storageRESTLength = "length"
storageRESTCount = "count" storageRESTCount = "count"
storageRESTBitrotAlgo = "bitrot-algo" storageRESTBitrotAlgo = "balg"
storageRESTBitrotHash = "bitrot-hash" storageRESTBitrotHash = "bhash"
storageRESTDiskID = "disk-id" storageRESTDiskID = "did"
storageRESTForceDelete = "force-delete" storageRESTForceDelete = "fdel"
storageRESTGlob = "glob" storageRESTGlob = "glob"
storageRESTMetrics = "metrics" storageRESTMetrics = "metrics"
storageRESTDriveQuorum = "drive-quorum" storageRESTDriveQuorum = "dquorum"
storageRESTOrigVolume = "orig-volume" storageRESTOrigVolume = "ovol"
storageRESTInclFreeVersions = "incl-free-versions" storageRESTInclFreeVersions = "incl-fv"
storageRESTRange = "rng"
) )
type nsScannerOptions struct { type nsScannerOptions struct {

View File

@ -375,10 +375,6 @@ func (s *storageRESTServer) ReadVersionHandlerWS(params *grid.MSS) (*FileInfo, *
volume := params.Get(storageRESTVolume) volume := params.Get(storageRESTVolume)
filePath := params.Get(storageRESTFilePath) filePath := params.Get(storageRESTFilePath)
versionID := params.Get(storageRESTVersionID) versionID := params.Get(storageRESTVersionID)
readData, err := strconv.ParseBool(params.Get(storageRESTReadData))
if err != nil {
return nil, grid.NewRemoteErr(err)
}
healing, err := strconv.ParseBool(params.Get(storageRESTHealing)) healing, err := strconv.ParseBool(params.Get(storageRESTHealing))
if err != nil { if err != nil {
@ -392,7 +388,7 @@ func (s *storageRESTServer) ReadVersionHandlerWS(params *grid.MSS) (*FileInfo, *
fi, err := s.getStorage().ReadVersion(context.Background(), origvolume, volume, filePath, versionID, ReadOptions{ fi, err := s.getStorage().ReadVersion(context.Background(), origvolume, volume, filePath, versionID, ReadOptions{
InclFreeVersions: inclFreeVersions, InclFreeVersions: inclFreeVersions,
ReadData: readData, ReadData: false,
Healing: healing, Healing: healing,
}) })
if err != nil { if err != nil {
@ -410,11 +406,6 @@ func (s *storageRESTServer) ReadVersionHandler(w http.ResponseWriter, r *http.Re
volume := r.Form.Get(storageRESTVolume) volume := r.Form.Get(storageRESTVolume)
filePath := r.Form.Get(storageRESTFilePath) filePath := r.Form.Get(storageRESTFilePath)
versionID := r.Form.Get(storageRESTVersionID) versionID := r.Form.Get(storageRESTVersionID)
readData, err := strconv.ParseBool(r.Form.Get(storageRESTReadData))
if err != nil {
s.writeErrorResponse(w, err)
return
}
healing, err := strconv.ParseBool(r.Form.Get(storageRESTHealing)) healing, err := strconv.ParseBool(r.Form.Get(storageRESTHealing))
if err != nil { if err != nil {
s.writeErrorResponse(w, err) s.writeErrorResponse(w, err)
@ -429,7 +420,7 @@ func (s *storageRESTServer) ReadVersionHandler(w http.ResponseWriter, r *http.Re
fi, err := s.getStorage().ReadVersion(r.Context(), origvolume, volume, filePath, versionID, ReadOptions{ fi, err := s.getStorage().ReadVersion(r.Context(), origvolume, volume, filePath, versionID, ReadOptions{
InclFreeVersions: inclFreeVersions, InclFreeVersions: inclFreeVersions,
ReadData: readData, ReadData: true,
Healing: healing, Healing: healing,
}) })
if err != nil { if err != nil {
@ -506,15 +497,11 @@ func (s *storageRESTServer) ReadXLHandler(w http.ResponseWriter, r *http.Request
if !s.IsValid(w, r) { if !s.IsValid(w, r) {
return return
} }
volume := r.Form.Get(storageRESTVolume) volume := r.Form.Get(storageRESTVolume)
filePath := r.Form.Get(storageRESTFilePath) filePath := r.Form.Get(storageRESTFilePath)
readData, err := strconv.ParseBool(r.Form.Get(storageRESTReadData))
if err != nil {
s.writeErrorResponse(w, err)
return
}
rf, err := s.getStorage().ReadXL(r.Context(), volume, filePath, readData) rf, err := s.getStorage().ReadXL(r.Context(), volume, filePath, true)
if err != nil { if err != nil {
s.writeErrorResponse(w, err) s.writeErrorResponse(w, err)
return return
@ -528,14 +515,10 @@ func (s *storageRESTServer) ReadXLHandlerWS(params *grid.MSS) (*RawFileInfo, *gr
if !s.checkID(params.Get(storageRESTDiskID)) { if !s.checkID(params.Get(storageRESTDiskID)) {
return nil, grid.NewRemoteErr(errDiskNotFound) return nil, grid.NewRemoteErr(errDiskNotFound)
} }
volume := params.Get(storageRESTVolume) volume := params.Get(storageRESTVolume)
filePath := params.Get(storageRESTFilePath) filePath := params.Get(storageRESTFilePath)
readData, err := strconv.ParseBool(params.Get(storageRESTReadData)) rf, err := s.getStorage().ReadXL(context.Background(), volume, filePath, false)
if err != nil {
return nil, grid.NewRemoteErr(err)
}
rf, err := s.getStorage().ReadXL(context.Background(), volume, filePath, readData)
if err != nil { if err != nil {
return nil, grid.NewRemoteErr(err) return nil, grid.NewRemoteErr(err)
} }
@ -1336,17 +1319,18 @@ func registerStorageRESTHandlers(router *mux.Router, endpointServerPools Endpoin
subrouter.Methods(http.MethodPost).Path(storageRESTVersionPrefix + storageRESTMethodHealth).HandlerFunc(h(server.HealthHandler)) subrouter.Methods(http.MethodPost).Path(storageRESTVersionPrefix + storageRESTMethodHealth).HandlerFunc(h(server.HealthHandler))
subrouter.Methods(http.MethodPost).Path(storageRESTVersionPrefix + storageRESTMethodAppendFile).HandlerFunc(h(server.AppendFileHandler)) subrouter.Methods(http.MethodPost).Path(storageRESTVersionPrefix + storageRESTMethodAppendFile).HandlerFunc(h(server.AppendFileHandler))
subrouter.Methods(http.MethodPost).Path(storageRESTVersionPrefix + storageRESTMethodReadVersion).HandlerFunc(h(server.ReadVersionHandler))
subrouter.Methods(http.MethodPost).Path(storageRESTVersionPrefix + storageRESTMethodReadXL).HandlerFunc(h(server.ReadXLHandler))
subrouter.Methods(http.MethodPost).Path(storageRESTVersionPrefix + storageRESTMethodCreateFile).HandlerFunc(h(server.CreateFileHandler)) subrouter.Methods(http.MethodPost).Path(storageRESTVersionPrefix + storageRESTMethodCreateFile).HandlerFunc(h(server.CreateFileHandler))
subrouter.Methods(http.MethodPost).Path(storageRESTVersionPrefix + storageRESTMethodReadFile).HandlerFunc(h(server.ReadFileHandler))
subrouter.Methods(http.MethodPost).Path(storageRESTVersionPrefix + storageRESTMethodReadFileStream).HandlerFunc(h(server.ReadFileStreamHandler))
subrouter.Methods(http.MethodPost).Path(storageRESTVersionPrefix + storageRESTMethodDeleteVersions).HandlerFunc(h(server.DeleteVersionsHandler)) subrouter.Methods(http.MethodPost).Path(storageRESTVersionPrefix + storageRESTMethodDeleteVersions).HandlerFunc(h(server.DeleteVersionsHandler))
subrouter.Methods(http.MethodPost).Path(storageRESTVersionPrefix + storageRESTMethodVerifyFile).HandlerFunc(h(server.VerifyFileHandler)) subrouter.Methods(http.MethodPost).Path(storageRESTVersionPrefix + storageRESTMethodVerifyFile).HandlerFunc(h(server.VerifyFileHandler))
subrouter.Methods(http.MethodPost).Path(storageRESTVersionPrefix + storageRESTMethodStatInfoFile).HandlerFunc(h(server.StatInfoFile)) subrouter.Methods(http.MethodPost).Path(storageRESTVersionPrefix + storageRESTMethodStatInfoFile).HandlerFunc(h(server.StatInfoFile))
subrouter.Methods(http.MethodPost).Path(storageRESTVersionPrefix + storageRESTMethodReadMultiple).HandlerFunc(h(server.ReadMultiple)) subrouter.Methods(http.MethodPost).Path(storageRESTVersionPrefix + storageRESTMethodReadMultiple).HandlerFunc(h(server.ReadMultiple))
subrouter.Methods(http.MethodPost).Path(storageRESTVersionPrefix + storageRESTMethodCleanAbandoned).HandlerFunc(h(server.CleanAbandonedDataHandler)) subrouter.Methods(http.MethodPost).Path(storageRESTVersionPrefix + storageRESTMethodCleanAbandoned).HandlerFunc(h(server.CleanAbandonedDataHandler))
subrouter.Methods(http.MethodGet).Path(storageRESTVersionPrefix + storageRESTMethodReadFileStream).HandlerFunc(h(server.ReadFileStreamHandler)) subrouter.Methods(http.MethodGet).Path(storageRESTVersionPrefix + storageRESTMethodReadFileStream).HandlerFunc(h(server.ReadFileStreamHandler))
subrouter.Methods(http.MethodGet).Path(storageRESTVersionPrefix + storageRESTMethodReadVersion).HandlerFunc(h(server.ReadVersionHandler))
subrouter.Methods(http.MethodGet).Path(storageRESTVersionPrefix + storageRESTMethodReadXL).HandlerFunc(h(server.ReadXLHandler))
subrouter.Methods(http.MethodGet).Path(storageRESTVersionPrefix + storageRESTMethodReadFile).HandlerFunc(h(server.ReadFileHandler))
logger.FatalIf(storageListDirRPC.RegisterNoInput(gm, server.ListDirHandler, endpoint.Path), "unable to register handler") logger.FatalIf(storageListDirRPC.RegisterNoInput(gm, server.ListDirHandler, endpoint.Path), "unable to register handler")
logger.FatalIf(storageReadAllRPC.Register(gm, server.ReadAllHandler, endpoint.Path), "unable to register handler") logger.FatalIf(storageReadAllRPC.Register(gm, server.ReadAllHandler, endpoint.Path), "unable to register handler")
logger.FatalIf(storageWriteAllRPC.Register(gm, server.WriteAllHandler, endpoint.Path), "unable to register handler") logger.FatalIf(storageWriteAllRPC.Register(gm, server.WriteAllHandler, endpoint.Path), "unable to register handler")