Add backoff, retry and other metrics for GCS JSON API #1228
Conversation
|
/gcbrun |
|
/gcbrun |
util/src/main/java/com/google/cloud/hadoop/util/GcsJsonApiEvent.java
Outdated
Show resolved
Hide resolved
util/src/main/java/com/google/cloud/hadoop/util/GcsJsonApiEvent.java
Outdated
Show resolved
Hide resolved
gcs/src/main/java/com/google/cloud/hadoop/fs/gcs/GhfsGlobalStorageStatistics.java
Show resolved
Hide resolved
gcs/src/test/java/com/google/cloud/hadoop/fs/gcs/GoogleCloudStorageStatisticsTest.java
Show resolved
Hide resolved
gcsio/src/main/java/com/google/cloud/hadoop/gcsio/GoogleCloudStorageStatistics.java
Outdated
Show resolved
Hide resolved
util/src/main/java/com/google/cloud/hadoop/util/GcsJsonApiEvent.java
Outdated
Show resolved
Hide resolved
| @@ -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. | |||
There was a problem hiding this comment.
is this moved to some other place?
There was a problem hiding this comment.
Yes, this will be done in the event subscriber.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Subscriber is incrementing the metric. HttpResonsecod event is not required. That information is present in the event and it will increment the status code.
util/src/main/java/com/google/cloud/hadoop/util/interceptors/InvocationIdInterceptor.java
Show resolved
Hide resolved
|
/gcbrun |
Give some warmup time for logging the outliers
|
/gcbrun |
|
/gcbrun |
| @@ -2424,6 +2422,273 @@ public void testHnBucketDeleteOperationOnNonExistingFolder() throws Exception { | |||
| } | |||
| } | |||
|
|
|||
| @Test | |||
There was a problem hiding this comment.
this is too big for unit test, can't it be refactored?
There was a problem hiding this comment.
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() { |
There was a problem hiding this comment.
you mean GcsGrpcAPIMetrics?
There was a problem hiding this comment.
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
util/src/main/java/com/google/cloud/hadoop/util/GcsJsonApiEvent.java
Outdated
Show resolved
Hide resolved
| @@ -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. | |||
There was a problem hiding this comment.
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?
util/src/main/java/com/google/cloud/hadoop/util/interceptors/InvocationIdInterceptor.java
Show resolved
Hide resolved
|
/gcbrun |
7d92934
into
GoogleCloudDataproc:branch-3.0.x
No description provided.