expose HttpRequest.setInterceptor to allow for throttled uploads

[srosenberg]

We have a multi-threaded uploader which transfers large files from local disk to GCS. The uploader is co-located with our low-latency app. inside the JVM. To control latency, we must throttle the uploader. Our best workaround involves resorting to reflection in order to install HttpRequest interceptor which effectively wraps HttpContent with a throttled OutputStream; e.g.,

Object storageRpc = FieldUtils.getField(storage.getClass(), "storageRpc", true).get(storage);
Object serviceStorage = FieldUtils.getField(storageRpc.getClass(), "storage", true).get(storageRpc);
Object requestFactory = FieldUtils.getField(serviceStorage.getClass(), "requestFactory", true).get(serviceStorage);
Field initializerField = FieldUtils.getField(requestFactory.getClass(), "initializer", true);
Object initializer = initializerField.get(requestFactory);
initializerField.set(requestFactory, new InterceptingHttpRequestInitializer((HttpRequestInitializer) initializer));

[yihanzhen]

This would require work in google-http-java-client. cc/ @chingor13

[chingor13]

Is there a reason this needs to be modifiable after construction?

It looks like you can accomplish wrapping an additional HttpRequestInitializer by extending the HttpTransportOptions class and wrapping your InterceptingHttpRequestInitializer around the parent one:

class MyTransportOptions extends HttpTransportOptions {
  // TODO: make your own constructor or builder
  @Override
  public HttpRequestInitializer getHttpRequestInitializer(final ServiceOptions<?, ?> serviceOptions) {
    return new InterceptingHttpRequestInitializer(super.getHttpRequestInitiallizer(serviceOptions));
  }
}

StorageOptions options = StorageOptions
  .newBuilder()
  .setTransportOptions(new MyTransportOptions())
  .build();
Storage storage = options.getService();

[srosenberg]

@chingor13 That works for our use-case, thanks. I must have missed that HttpTransportOptions is used by default (and getHttpRequestInitializer can be overridden); much better than using reflection hack.

I can't think of another use-case which would require modifying RequestInitializer after construction; in fact, it seems like a good practice to make it immutable; e.g., modifying it while request is in progress is a risky proposition. Feel free to close the issue.

[ajaaym]

closing this issue as its resolved.

[srosenberg]

@chingor13 We're only now revisiting this issue... what you suggested is not feasible (statically) because HttpTransportOptions has a private constructor,

https://github.com/googleapis/google-cloud-java/blob/master/google-cloud-clients/google-cloud-core-http/src/main/java/com/google/cloud/http/HttpTransportOptions.java#L123

@ajaaym could you please reopen the issue unless there is a less hacky workaround as I'd originally suggested?

[andrey-qlogic]

@srosenberg , @sduskis what is new feature might be needed from the api: storage now by this feature request?

[srosenberg]

@ajaaym Recently, we had to also throttle downloads to minimize GC disturbances during (data) reload. I was unable to find a clean way to do this via existing API. Instead, the following workaround was implemented which suffers from using reflection (see second snippet) and extending HttpURLConnection. Is it possible to provide a hook for intercepting InputStream?

protected Storage installHttpThrottler(StorageOptions.Builder storageOptions) {
        return storageOptions
                .setTransportOptions(HttpTransportOptions.newBuilder().setHttpTransportFactory(new ThrottledHttpTransportFactory()).build())
                .build()
                .getService();
    }

class ThrottledHttpTransportFactory implements HttpTransportFactory {
        public HttpTransport create() {
            NetHttpTransport result = new NetHttpTransport();

            try {
                //TODO(SR): resorting to reflection hack since there is no other way to update connectionFactory
                //see https://github.com/GoogleCloudPlatform/google-cloud-java/issues/3534
                FieldUtils.writeDeclaredField(result, "connectionFactory", new ThrottledConnectionFactory(), true);
            } catch (Exception ex) {
                throw new RuntimeException("Unable to install ThrottledConnectionFactory", ex);
            }
            return result;
        }
    }

class ThrottledConnectionFactory extends DefaultConnectionFactory {
        @Override
        public HttpURLConnection openConnection(URL url) throws IOException {
            return new ThrottledUrlConnection(super.openConnection(url), url);
        }
    }

[ajaaym]

@srosenberg Do you want to intercept inputstream or connection factory? For inputstream, do you mean by getting raw inputstream?

You can create transport factory like below without using reflection.

    NetHttpTransport.Builder builder = new Builder();
    builder.setConnectionFactory(new ThrottledConnectionFactory());
    return builder.build();

[srosenberg]

@ajaaym Thanks, somehow I missed the (availability of) factory; that solves the reflection hack. The other remaining issue is having to extend HttpURLConnection in order to intercept InputStream,

class ThrottledUrlConnection extends HttpURLConnection {
        HttpURLConnection delegate;

        ThrottledUrlConnection(HttpURLConnection delegate, URL url) {
            super(url);
            this.delegate = delegate;
        }

        @Override
        public InputStream getInputStream() throws IOException {
            return new ThrottledInputStream(delegate.getInputStream(), rateLimiter, throughputMeter);
        }
// followed by a long list of overridden methods to redirect via 'delegate'
....
}

[ajaaym]

@srosenberg as of now you can use response interceptor, but you have to use reflection to set the content after calling getContent(). You can create new feature request in that repo to allow overriding content in HttpResponse.

[srosenberg]

@ajaaym I am happy to create a new feature request for response interceptor in google-http-java-client so long that it makes it into this repo (via transitive dependency); we need it in conjunction with google-cloud-storage.

[srosenberg]

Issue created: googleapis/google-http-java-client#586