From c3d24fb26d1972617c89a81f02b28b87383d6a36 Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Thu, 4 Nov 2021 12:11:52 -0700 Subject: [PATCH] use single encoder for sending speedtest results (#13579) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bonus: if runs have PUT higher then capture it anyways to display an unexpected result, which provides a way to understand what might be slowing things down on the system. For example on a Data24 WDC setup it is clearly visible there is a bug in the hardware. ``` ./mc admin speedtest wdc/ ⠧ Running speedtest (With 64 MiB object size, 32 concurrency) PUT: 31 GiB/s GET: 24 GiB/s ⠹ Running speedtest (With 64 MiB object size, 48 concurrency) PUT: 38 GiB/s GET: 24 GiB/s MinIO 2021-11-04T06:08:33Z, 6 servers, 48 drives PUT: 38 GiB/s, 605 objs/s GET: 24 GiB/s, 383 objs/s ``` Reads are almost 14GiB/sec slower than Writes which is practically not possible. --- cmd/admin-handlers.go | 21 ++++++++++++++++----- cmd/utils.go | 37 ++++++++++++++++++++++++++----------- 2 files changed, 42 insertions(+), 16 deletions(-) diff --git a/cmd/admin-handlers.go b/cmd/admin-handlers.go index 8dd47664b..adb5ecc2b 100644 --- a/cmd/admin-handlers.go +++ b/cmd/admin-handlers.go @@ -951,9 +951,18 @@ func (a adminAPIHandlers) SpeedtestHandler(w http.ResponseWriter, r *http.Reques duration = time.Second * 10 } + deleteBucket := func() { + loc := pathJoin(minioMetaSpeedTestBucket, minioMetaSpeedTestBucketPrefix) + objectAPI.DeleteBucket(context.Background(), loc, DeleteBucketOptions{ + Force: true, + NoRecreate: true, + }) + } + keepAliveTicker := time.NewTicker(500 * time.Millisecond) defer keepAliveTicker.Stop() + enc := json.NewEncoder(w) ch := speedTest(ctx, size, concurrent, duration, autotune) for { select { @@ -961,19 +970,19 @@ func (a adminAPIHandlers) SpeedtestHandler(w http.ResponseWriter, r *http.Reques return case <-keepAliveTicker.C: // Write a blank entry to prevent client from disconnecting - if err := json.NewEncoder(w).Encode(madmin.SpeedTestResult{}); err != nil { + if err := enc.Encode(madmin.SpeedTestResult{}); err != nil { return } w.(http.Flusher).Flush() case result, ok := <-ch: if !ok { - defer objectAPI.DeleteBucket(context.Background(), pathJoin(minioMetaSpeedTestBucket, minioMetaSpeedTestBucketPrefix), DeleteBucketOptions{Force: true, NoRecreate: true}) + deleteBucket() return } - if err := json.NewEncoder(w).Encode(result); err != nil { - writeErrorResponseJSON(ctx, w, toAdminAPIErr(ctx, err), r.URL) + if err := enc.Encode(result); err != nil { return } + w.(http.Flusher).Flush() } } } @@ -1920,13 +1929,15 @@ func (a adminAPIHandlers) BandwidthMonitorHandler(w http.ResponseWriter, r *http } } }() + + enc := json.NewEncoder(w) for { select { case report, ok := <-reportCh: if !ok { return } - if err := json.NewEncoder(w).Encode(report); err != nil { + if err := enc.Encode(report); err != nil { writeErrorResponseJSON(ctx, w, toAPIError(ctx, err), r.URL) return } diff --git a/cmd/utils.go b/cmd/utils.go index f3eb2f3be..644bc730d 100644 --- a/cmd/utils.go +++ b/cmd/utils.go @@ -987,7 +987,7 @@ func speedTest(ctx context.Context, throughputSize, concurrencyStart int, durati throughputHighestGet := uint64(0) throughputHighestPut := uint64(0) - var throughputHighestGetResults []SpeedtestResult + var throughputHighestResults []SpeedtestResult sendResult := func() { var result madmin.SpeedTestResult @@ -998,21 +998,21 @@ func speedTest(ctx context.Context, throughputSize, concurrencyStart int, durati result.GETStats.ObjectsPerSec = throughputHighestGet / uint64(throughputSize) / uint64(durationSecs) result.PUTStats.ThroughputPerSec = throughputHighestPut / uint64(durationSecs) result.PUTStats.ObjectsPerSec = throughputHighestPut / uint64(throughputSize) / uint64(durationSecs) - for i := 0; i < len(throughputHighestGetResults); i++ { + for i := 0; i < len(throughputHighestResults); i++ { errStr := "" - if throughputHighestGetResults[i].Error != "" { - errStr = throughputHighestGetResults[i].Error + if throughputHighestResults[i].Error != "" { + errStr = throughputHighestResults[i].Error } result.PUTStats.Servers = append(result.PUTStats.Servers, madmin.SpeedTestStatServer{ - Endpoint: throughputHighestGetResults[i].Endpoint, - ThroughputPerSec: throughputHighestGetResults[i].Uploads / uint64(durationSecs), - ObjectsPerSec: throughputHighestGetResults[i].Uploads / uint64(throughputSize) / uint64(durationSecs), + Endpoint: throughputHighestResults[i].Endpoint, + ThroughputPerSec: throughputHighestResults[i].Uploads / uint64(durationSecs), + ObjectsPerSec: throughputHighestResults[i].Uploads / uint64(throughputSize) / uint64(durationSecs), Err: errStr, }) result.GETStats.Servers = append(result.GETStats.Servers, madmin.SpeedTestStatServer{ - Endpoint: throughputHighestGetResults[i].Endpoint, - ThroughputPerSec: throughputHighestGetResults[i].Downloads / uint64(durationSecs), - ObjectsPerSec: throughputHighestGetResults[i].Downloads / uint64(throughputSize) / uint64(durationSecs), + Endpoint: throughputHighestResults[i].Endpoint, + ThroughputPerSec: throughputHighestResults[i].Downloads / uint64(durationSecs), + ObjectsPerSec: throughputHighestResults[i].Downloads / uint64(throughputSize) / uint64(durationSecs), Err: errStr, }) } @@ -1052,6 +1052,21 @@ func speedTest(ctx context.Context, throughputSize, concurrencyStart int, durati } if totalGet < throughputHighestGet { + // Following check is for situations + // when Writes() scale higher than Reads() + // - practically speaking this never happens + // and should never happen - however it has + // been seen recently due to hardware issues + // causes Reads() to go slower than Writes(). + // + // Send such results anyways as this shall + // expose a problem underneath. + if totalPut > throughputHighestPut { + throughputHighestResults = results + throughputHighestPut = totalPut + // let the client see lower value as well + throughputHighestGet = totalGet + } sendResult() break } @@ -1062,7 +1077,7 @@ func speedTest(ctx context.Context, throughputSize, concurrencyStart int, durati } throughputHighestGet = totalGet - throughputHighestGetResults = results + throughputHighestResults = results throughputHighestPut = totalPut if doBreak {