scan server metrics from #4458 based on #4459#4461
Conversation
|
@EdColeman I added commit d8b4c66 to #4458 that could be merged into this |
EdColeman
force-pushed
the
kt_scan_server_metrics
branch
from
aeecc7b to
29f5f72
Compare
EdColeman
force-pushed
the
kt_scan_server_metrics
branch
from
ac6ec88 to
ad34474
Compare
|
@keith-turner I believe this is up to date with your additions, if not we can add them before merge, This was rebased with the other metric changes so the change-set is more reasonable to review. |
server/tserver/src/main/java/org/apache/accumulo/tserver/ScanServerMetrics.java
Outdated
Show resolved
Hide resolved
core/src/main/java/org/apache/accumulo/core/metrics/MetricsProducer.java
Outdated
Show resolved
Hide resolved
| // TODO this counter is a duplicate, already a metrics for this below the scan server... use | ||
| // that instead, but look into renaming existing one to indicate scan server | ||
| busyCounter = Counter.builder(MetricsProducer.METRICS_SSERVER_BUSY_COUNTER) |
There was a problem hiding this comment.
The process name in the common tags should differentiate between tserver and sserver.
core/src/main/java/org/apache/accumulo/core/metrics/MetricsProducer.java
Outdated
Show resolved
Hide resolved
core/src/main/java/org/apache/accumulo/core/metrics/MetricsProducer.java
Outdated
Show resolved
Hide resolved
core/src/main/java/org/apache/accumulo/core/metrics/MetricsProducer.java
Outdated
Show resolved
Hide resolved
|
Changes in 5dd3c2d
|
server/tserver/src/main/java/org/apache/accumulo/tserver/ScanServerMetrics.java
Outdated
Show resolved
Hide resolved
|
@EdColeman I was experimenting with these changes and I was not seeing the stats I expected from the cache. Finally figured out that stats have to be enabled when building the cache, so pushed a change to do that in 3660ecd. I was using MetricsIT to experiment with these changes when trying to figure this out, can see those changes here b3fdae9 if interested. |
server/tserver/src/main/java/org/apache/accumulo/tserver/ScanServerMetrics.java
Outdated
Show resolved
Hide resolved
4 participants
This PR is based on the changes proposed by @keith-turner in #4458, using the changes in #4459 as the base.
These changes have not be test yet.
These changes should not be merged until #4459 is complete.