Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 24 additions & 5 deletions go/vt/vtgate/executor_scatter_stats.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,25 @@ type statsResultItem struct {
Count uint64
}

func castToFloat64(input interface{}) float64 {
switch input.(type) {
case int64:
return float64(input.(int64))
case uint64:
return float64(input.(uint64))
case time.Duration:
return float64(input.(time.Duration))
}
return 0
}

func divideOrReturnZero(dividend, divisor interface{}) float64 {
if castToFloat64(divisor) == 0 {
return 0
}
return castToFloat64(dividend) / castToFloat64(divisor)
}

func (e *Executor) gatherScatterStats() (statsResults, error) {
scatterExecTime := time.Duration(0)
readOnlyTime := time.Duration(0)
Expand Down Expand Up @@ -91,11 +110,11 @@ func (e *Executor) gatherScatterStats() (statsResults, error) {
route := routes[i]
resultItems[i] = &statsResultItem{
Query: plan.Original,
AvgTimePerQuery: time.Duration(plan.ExecTime.Nanoseconds() / int64(plan.ExecCount)),
PercentTimeOfReads: float64(100 * plan.ExecTime / readOnlyTime),
PercentTimeOfScatters: float64(100 * plan.ExecTime / scatterExecTime),
PercentCountOfReads: float64(100 * plan.ExecCount / readOnlyCount),
PercentCountOfScatters: float64(100 * plan.ExecCount / scatterCount),
AvgTimePerQuery: time.Duration(divideOrReturnZero(plan.ExecTime.Nanoseconds(), plan.ExecCount)),
PercentTimeOfReads: 100 * divideOrReturnZero(plan.ExecTime, readOnlyTime),
Copy link
Copy Markdown
Collaborator

@dweitzman dweitzman Mar 7, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Except for AvgTimePerQuery, seems like all of these are float64. You could just convert the arguments to float64 prior to division instead of after, which would avoid the panic since float64 has a concept of infinity and not-a-number.

If you do that, then for AvgTimePerQuery, maybe just add a one if-statement to check for a zero divisor inside this for-loop instead of a whole helper function?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the comment. Let me close this one and simply make another one-line change for AvgTimePerQuery

PercentTimeOfScatters: 100 * divideOrReturnZero(plan.ExecTime, scatterExecTime),
PercentCountOfReads: 100 * divideOrReturnZero(plan.ExecCount, readOnlyCount),
PercentCountOfScatters: 100 * divideOrReturnZero(plan.ExecCount, scatterCount),
From: route.Keyspace.Name + "." + route.TableName,
Count: plan.ExecCount,
}
Expand Down
37 changes: 37 additions & 0 deletions go/vt/vtgate/executor_scatter_stats_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ package vtgate
import (
"context"
"testing"
"time"

"github.com/stretchr/testify/assert"

"net/http/httptest"

Expand Down Expand Up @@ -76,3 +79,37 @@ func TestScatterStatsHttpWriting(t *testing.T) {
require.Contains(t, recorder.Body.String(), query4)
require.NoError(t, err)
}

func TestDivideOrReturnZero(t *testing.T) {
// Expected types
timeDurationZero := time.Duration(0)
unitZero := uint64(0)
intZero := int64(0)
timeDurationOne := time.Duration(1)
uintOne := uint64(1)
intOne := int64(1)

resultZero := float64(0)
resultOne := float64(1)

assert.Equal(t, divideOrReturnZero(timeDurationOne, timeDurationZero), resultZero)
assert.Equal(t, divideOrReturnZero(timeDurationOne, unitZero), resultZero)
assert.Equal(t, divideOrReturnZero(timeDurationOne, intZero), resultZero)
assert.Equal(t, divideOrReturnZero(timeDurationOne, timeDurationOne), resultOne)
assert.Equal(t, divideOrReturnZero(timeDurationOne, uintOne), resultOne)
assert.Equal(t, divideOrReturnZero(timeDurationOne, intOne), resultOne)
assert.Equal(t, divideOrReturnZero(uintOne, timeDurationOne), resultOne)
assert.Equal(t, divideOrReturnZero(uintOne, uintOne), resultOne)
assert.Equal(t, divideOrReturnZero(uintOne, intOne), resultOne)
assert.Equal(t, divideOrReturnZero(intOne, timeDurationOne), resultOne)
assert.Equal(t, divideOrReturnZero(intOne, uintOne), resultOne)
assert.Equal(t, divideOrReturnZero(intOne, intOne), resultOne)

// Unexpected type
int16 := int16(1)
intArray := [2]int64{1, 2}

assert.Equal(t, divideOrReturnZero(uintOne, int16), resultZero)
assert.Equal(t, divideOrReturnZero(uintOne, intArray), resultZero)

}