Skip to content
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

Merged
merged 3 commits into from
Mar 18, 2024
Merged

Adding Statistics #1128

merged 3 commits into from
Mar 18, 2024

Conversation

guljain
Copy link
Contributor

@guljain guljain commented Mar 14, 2024

Adding following statistics:

  1. Directories_deleted
  2. op_get_list_status_result_size

Also changing LATENCY_LOGGING_THRESHOLD_MS to 300ms

@guljain
Copy link
Contributor Author

guljain commented Mar 14, 2024

/gcbrun

* @param strBuilder
*/
@Subscribe
private void subscriberOnString(StringBuilder strBuilder) {
Copy link
Contributor

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();
Copy link
Contributor

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?

Copy link
Contributor Author

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);
Copy link
Contributor

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above.

@guljain
Copy link
Contributor Author

guljain commented Mar 18, 2024

/gcbrun

* @param strType
*/
@Subscribe
private void subscriberOnStatisticsType(StatisticsType strType) {
Copy link
Contributor

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;
Copy link
Contributor

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?

@guljain guljain merged commit 6698080 into GoogleCloudDataproc:master Mar 18, 2024
2 of 4 checks passed
guljain added a commit to guljain/hadoop-connectors that referenced this pull request Jun 14, 2024
* Adding Statistics

* Adding statistics

(cherry picked from commit 6698080)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants