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

Add backoff, retry and other metrics for GCS JSON API #1228

Conversation

arunkumarchacko
Copy link
Contributor

No description provided.

@arunkumarchacko
Copy link
Contributor Author

/gcbrun

@arunkumarchacko
Copy link
Contributor Author

/gcbrun

@@ -196,9 +197,6 @@ public boolean handleResponse(HttpRequest request, HttpResponse response, boolea
}

private void logResponseCode(HttpRequest request, HttpResponse response) {
// Incrementing GCS Static Statistics using status code of response.
Copy link
Contributor

Choose a reason for hiding this comment

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

is this moved to some other place?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, this will be done in the event subscriber.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't understand. This is the place from where we are publishing an event in EventBus which will be consumed by subscriber to update metrics. How a subscriber is publishing event?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Subscriber is incrementing the metric. HttpResonsecod event is not required. That information is present in the event and it will increment the status code.

@arunkumarchacko
Copy link
Contributor Author

/gcbrun

Give some warmup time for logging the outliers
@arunkumarchacko
Copy link
Contributor Author

/gcbrun

@arunkumarchacko
Copy link
Contributor Author

/gcbrun

@@ -2424,6 +2422,273 @@ public void testHnBucketDeleteOperationOnNonExistingFolder() throws Exception {
}
}

@Test
Copy link
Contributor

Choose a reason for hiding this comment

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

this is too big for unit test, can't it be refactored?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree.

The length is coming from the "expected" results. Refactoring it might make it less readable.

I thought about separating it. But this is having a full lifecycle of an object. i.e create, read, rename and lastly delete. Hence choose to having this as one test.


@Ignore
@Test
public void testGcsJsonAPIMetrics() {
Copy link
Contributor

Choose a reason for hiding this comment

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

you mean GcsGrpcAPIMetrics?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

after gRPC we will have to update this tests. I am not sure if we can get the full parity for gRPC. If not, we might need to have two tests. one for gRPC and another for JSON

@@ -196,9 +197,6 @@ public boolean handleResponse(HttpRequest request, HttpResponse response, boolea
}

private void logResponseCode(HttpRequest request, HttpResponse response) {
// Incrementing GCS Static Statistics using status code of response.
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't understand. This is the place from where we are publishing an event in EventBus which will be consumed by subscriber to update metrics. How a subscriber is publishing event?

@arunkumarchacko
Copy link
Contributor Author

/gcbrun

@arunkumarchacko arunkumarchacko merged commit 7d92934 into GoogleCloudDataproc:branch-3.0.x Aug 1, 2024
4 checks passed
@arunkumarchacko arunkumarchacko deleted the add_metrics_3_0 branch August 1, 2024 04:10
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants