Added Time-Specific Metrics #867
Conversation
This is disabled by default behind <fs.gs.tracelog.enable> flag
Change the tests to add a log handler to verify the logs
Original PR: GoogleCloudDataproc#819
|
/gcbrun |
Codecov ReportBase: 80.44% // Head: 80.34% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #867 +/- ##
============================================
- Coverage 80.44% 80.34% -0.10%
- Complexity 2009 2024 +15
============================================
Files 137 137
Lines 9000 9041 +41
Branches 1036 1035 -1
============================================
+ Hits 7240 7264 +24
- Misses 1340 1358 +18
+ Partials 420 419 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
| @@ -569,7 +573,6 @@ public void seekForwards(long skipped) { | |||
| if (skipped > 0) { | |||
| bytesSkippedOnSeek.addAndGet(skipped); | |||
| } | |||
| seekOperations.incrementAndGet(); | |||
There was a problem hiding this comment.
Why we are removing this? Similar comment fir seekBackwards too.
nit: update the java doc of this function.
gcs/src/main/java/com/google/cloud/hadoop/fs/gcs/GhfsStorageStatistics.java
Show resolved
Hide resolved
| * | ||
| * @return current map of minimums | ||
| */ | ||
| private Map<String, Long> minimums() { |
There was a problem hiding this comment.
Do any of these private functions adding any value?
There was a problem hiding this comment.
We could have inlined this. Having this method is not bad either.
| streamStatistics.bytesRead(max(response, 0)); | ||
| streamStatistics.readOperationCompleted(length, max(response, 0)); | ||
| return response; | ||
| return trackDuration( |
There was a problem hiding this comment.
It would be much cleaner if we can use AOP and annotation to capture the "duration" of each API.
There was a problem hiding this comment.
Using the pattern from hadoop. Adding AOP will be more work and I do no think it is worth the effort.
|
/gcbrun |
|
/gcbrun |
|
/gcbrun |
Original PR: #819