swarm/shed: add metrics to each shed db #1029
Conversation
swarm/shed/db.go
Outdated
| } | ||
|
|
||
| // NewDB constructs a new DB and validates the schema | ||
| // if it exists in database on the given path. | ||
| func NewDB(path string) (db *DB, err error) { | ||
| // prefix is used for metrics collection for the given DB. | ||
| func NewDB(path string, prefix string) (db *DB, err error) { |
There was a problem hiding this comment.
I would just rename prefix to metricsPrefix as it is more clear what is it for.
It is also ok to have metrics optional. Not to call db.Meter in NewBD, but to explicitly call it only in implementation when it is needed. In that case prefix argument is not needed in NewDB.
There was a problem hiding this comment.
I think there is no harm calling it for every single LevelDB, as metrics are gathered only once every 10sec. and AFAIK LevelDB also collects them always.
| // Meter configures the database metrics collectors | ||
| func (db *DB) Meter(prefix string) { | ||
| // Initialize all the metrics collector at the requested prefix | ||
| db.compTimeMeter = metrics.NewRegisteredMeter(prefix+"compact/time", nil) |
There was a problem hiding this comment.
Shouldn't *Meter fields assignments be under a lock?
There was a problem hiding this comment.
I don't think so. Why would they be under a lock? Meter is called once, when you initialise the DB, why would you initialise the same DB under different threads?
There was a problem hiding this comment.
As Meter is exported, it can be called any number of times, this is why I suggested not to export it, in the commend bellow.
There was a problem hiding this comment.
You're right, Meter should not be exported. It should not be called a number of times, but only once.
| } | ||
| } | ||
| // Update all the requested meters | ||
| if db.compTimeMeter != nil { |
There was a problem hiding this comment.
All *Meter fields checks should be under a lock.
I have a suggestion, not to export Meter method, just to initialize metrics in NewDB, making them "immutable" from "outside" and to remove the lock completely.
There was a problem hiding this comment.
Yes, the problem is that Meter is exported.
|
merged upstream as ethereum/go-ethereum#18277 |
Currently it is kind of difficult to measure Swarm's disk usage, due to the very different setups we use. It'd be nice if we have a consistent view of the disk usage at least from the point of view of the Swarm process.