-
Notifications
You must be signed in to change notification settings - Fork 20k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
cmd, dashboard, ethdb, vendor: send iostats to dashboard #16277
Conversation
b815ccb
to
76b6d28
Compare
ethdb/database.go
Outdated
func (db *LDBDatabase) meter(refresh time.Duration) { | ||
// Create the counters to store current and previous values | ||
counters := make([][]float64, 2) | ||
for i := 0; i < 2; i++ { | ||
counters[i] = make([]float64, 3) | ||
} | ||
// Create storage for iostats. | ||
var curr, prev [2]float64 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You only need to track the previous values, there's no point to pre-declare a curr
array too. Also previously we only gathered compaction stats, so names like counters
was good enough, since there wasn't ambiguity. With the added iostats, counters
and curr
and prev
all of a sudden don't mean anything.
Please rename counters
to compactions
and curr/prev
to iostats
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor ambiguity issue in the stats gathering, otherwise lgtm.
76b6d28
to
7fabe6b
Compare
ethdb/database.go
Outdated
curr[0], _ = strconv.ParseFloat(strings.Split(parts[0], ":")[1], 64) | ||
curr[1], _ = strconv.ParseFloat(strings.Split(parts[1], ":")[1], 64) | ||
read, _ := strconv.ParseFloat(strings.Split(parts[0], ":")[1], 64) | ||
write, _ := strconv.ParseFloat(strings.Split(parts[1], ":")[1], 64) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please handle any error, otherwise this will blow up the stats.
7fabe6b
to
3d0181b
Compare
db.log.Error("Read entry parsing failed", "err", err) | ||
return | ||
} | ||
w := strings.Split(parts[1], ":") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will panic if len(parts) == 1
ethdb/database.go
Outdated
return | ||
} | ||
w := strings.Split(parts[1], ":") | ||
if len(r) < 1 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
r
-> w
034caea
to
6a5af0a
Compare
ethdb/database.go
Outdated
return | ||
} | ||
w := strings.Split(parts[1], ":") | ||
if len(r) < 2 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
w
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
* cmd, dashboard, ethdb, vendor: send iostats to dashboard * ethdb: change names * ethdb: handle parsing errors * ethdb: handle iostats syntax error * ethdb: r -> w
* cmd, dashboard, ethdb, vendor: send iostats to dashboard * ethdb: change names * ethdb: handle parsing errors * ethdb: handle iostats syntax error * ethdb: r -> w
* cmd, dashboard, ethdb, vendor: send iostats to dashboard * ethdb: change names * ethdb: handle parsing errors * ethdb: handle iostats syntax error * ethdb: r -> w
* cmd, dashboard, ethdb, vendor: send iostats to dashboard * ethdb: change names * ethdb: handle parsing errors * ethdb: handle iostats syntax error * ethdb: r -> w
Update the
goleveldb
vendor package.Collect the effective amount of disk RW and send it to the dashboard.
Remove the old RW metrics.
Only the last commit is relevant, the previous ones are part of this PR #16263