Re: [PR] Add metrics for scan server reservation write out time and collisions [accumulo]
DomGarguilo merged PR #4577: URL: https://github.com/apache/accumulo/pull/4577 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@accumulo.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Add metrics for scan server reservation write out time and collisions [accumulo]
DomGarguilo commented on code in PR #4577:
URL: https://github.com/apache/accumulo/pull/4577#discussion_r1610068107
##
server/tserver/src/main/java/org/apache/accumulo/tserver/ScanServer.java:
##
@@ -601,7 +602,13 @@ private Map
reserveFilesInner(Collection ex
}
if (!filesToReserve.isEmpty()) {
-getContext().getAmple().putScanServerFileReferences(refs);
+long reservationWriteStart = System.nanoTime();
+try {
+ getContext().getAmple().putScanServerFileReferences(refs);
Review Comment:
I think it does make things shorter and cleaner. I'll make this change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscr...@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Add metrics for scan server reservation write out time and collisions [accumulo]
keith-turner commented on code in PR #4577:
URL: https://github.com/apache/accumulo/pull/4577#discussion_r1609029345
##
server/tserver/src/main/java/org/apache/accumulo/tserver/ScanServer.java:
##
@@ -601,7 +602,13 @@ private Map
reserveFilesInner(Collection ex
}
if (!filesToReserve.isEmpty()) {
-getContext().getAmple().putScanServerFileReferences(refs);
+long reservationWriteStart = System.nanoTime();
+try {
+ getContext().getAmple().putScanServerFileReferences(refs);
Review Comment:
Micrometer Timer meters have a `record(Runnable)` method that could possibly
be used here by passing it a lambda that calls `putScanServerFileReferences()`.
Not sure if that is shorter or better, this is stylistic comment feel free to
ignore it.
##
server/tserver/src/main/java/org/apache/accumulo/tserver/ScanServerMetrics.java:
##
@@ -43,20 +49,38 @@ public ScanServerMetrics(final
LoadingCache tabletMeta
@Override
public void registerMetrics(MeterRegistry registry) {
-reservationTimer =
Timer.builder(MetricsProducer.METRICS_SCAN_RESERVATION_TIMER)
+totalReservationTimer =
Timer.builder(MetricsProducer.METRICS_SCAN_RESERVATION_TOTAL_TIMER)
.description("Time to reserve a tablets files for
scan").register(registry);
+writeOutReservationTimer =
+Timer.builder(MetricsProducer.METRICS_SCAN_RESERVATION_WRITEOUT_TIMER)
+.description("Time to write out a tablets files for
scan").register(registry);
Review Comment:
```suggestion
.description("Time to write out a tablets file reservations for
scan").register(registry);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscr...@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Add metrics for scan server reservation write out time and collisions [accumulo]
DomGarguilo commented on PR #4577:
URL: https://github.com/apache/accumulo/pull/4577#issuecomment-2123388801
@keith-turner, I reimplemented things based on your feedback. Was able to
see these values when metrics were set to be logged:
```
METRICS: 2024-05-21T16:23:46,854,
accumulo.scan.reservation.total.timer{host=localhost,instance.name=uno,port=9996,process.name=sserver,resource.group=default}
throughput=0.1/s mean=0.102909686s max=0.102909686s
METRICS: 2024-05-21T16:23:46,855,
accumulo.scan.reservation.writeout.timer{host=localhost,instance.name=uno,port=9996,process.name=sserver,resource.group=default}
throughput=0.1/s mean=0.025735159s max=0.025735159s
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscr...@accumulo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
Re: [PR] Add metrics for scan server reservation write out time and collisions [accumulo]
keith-turner commented on PR #4577: URL: https://github.com/apache/accumulo/pull/4577#issuecomment-2121468480 @DomGarguilo sorry my writeup was awful for #4509. For the write time was looking to track how long it takes to call ` getContext().getAmple().putScanServerFileReferences(refs);` [here](https://github.com/apache/accumulo/blob/dd61442925f9236fda1bc176394052c30547156f/server/tserver/src/main/java/org/apache/accumulo/tserver/ScanServer.java#L604). This is the time it take write out scan server refs to the metadata table. For collisions, my terminology was really bad. The event I wanted to track was the case where tablets files changed during the process of attempting to reserve files. That could be done by incrementing a metric immediately before returning null [here](https://github.com/apache/accumulo/blob/dd61442925f9236fda1bc176394052c30547156f/server/tserver/src/main/java/org/apache/accumulo/tserver/ScanServer.java#L638). The [log message](https://github.com/apache/accumulo/blob/dd61442925f9236fda1bc176394052c30547156f/server/tserver/src/main/java/org/apache/accumulo/tserver/ScanServer.java#L635) a few lines up describes what is happening. Not sure colliosn is the best single word for the metric name that describes `tablet files changed while attempting to reference files`, but can not think of something more succinct. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@accumulo.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Add metrics for scan server reservation write out time and collisions [accumulo]
DomGarguilo commented on PR #4541: URL: https://github.com/apache/accumulo/pull/4541#issuecomment-2120632234 Closing as superseded by #4577 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@accumulo.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Add metrics for scan server reservation write out time and collisions [accumulo]
DomGarguilo closed pull request #4541: Add metrics for scan server reservation write out time and collisions URL: https://github.com/apache/accumulo/pull/4541 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@accumulo.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
