-
Notifications
You must be signed in to change notification settings - Fork 231
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
Adding Statistics #1128
Adding Statistics #1128
Conversation
/gcbrun |
* @param strBuilder | ||
*/ | ||
@Subscribe | ||
private void subscriberOnString(StringBuilder strBuilder) { |
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.
Let us create a a new type in utils instead of using String type. Strong typing is better.
@@ -593,8 +593,8 @@ public FSDataOutputStream create( | |||
.build(), | |||
statistics), | |||
statistics); | |||
globalStorageStatistics.filesCreated(); | |||
instrumentation.fileCreated(); |
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.
why are we removing this?
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.
These functions hamper the usage of incrementStatistics() function. I think we should have a single function where we increment both FS based and global counter simultaneously. Also improves code readability
@@ -726,8 +726,8 @@ public boolean delete(Path hadoopPath, boolean recursive) throws IOException { | |||
"delete(hadoopPath: %s, recursive: %b): true", hadoopPath, recursive); | |||
} | |||
response = result; | |||
globalStorageStatistics.fileDeleted(1); | |||
instrumentation.fileDeleted(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.
why are we removing this?
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.
same as above.
/gcbrun |
* @param strType | ||
*/ | ||
@Subscribe | ||
private void subscriberOnStatisticsType(StatisticsType strType) { |
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.
rename strType
@@ -61,7 +59,7 @@ public class GhfsGlobalStorageStatistics extends StorageStatistics { | |||
/** {@value} The key that stores all the registered metrics */ | |||
public static final String NAME = "GhfsStorageStatistics"; | |||
|
|||
public static final int LATENCY_LOGGING_THRESHOLD_MS = 150; | |||
public static final int LATENCY_LOGGING_THRESHOLD_MS = 300; |
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.
Can you also add the logic to not log this if it is the first request?
* Adding Statistics * Adding statistics (cherry picked from commit 6698080)
Adding following statistics:
Also changing LATENCY_LOGGING_THRESHOLD_MS to 300ms