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

Support "Connection : Keep-Alive" header on Http operations #536

Open
roelofkemp opened this issue May 25, 2020 · 5 comments
Open

Support "Connection : Keep-Alive" header on Http operations #536

roelofkemp opened this issue May 25, 2020 · 5 comments
Labels
2 - Working Working on this issue. bug Something isn't working

Comments

@roelofkemp
Copy link
Contributor

When performing subsequent PUT operations without the "Connection : Close" header, the second PUT operation does not complete and times out after 60 seconds. Setting the Connection header to close avoids this problem, and introduces overhead of setting up a connection for each new request (especially SSL costs might be expensive).

Desired solution: subsequent PUTs work with the Keep-Alive header (note that this header doesn't need to be set explicitly, the server will take care of it)

See: com.dexels.navajo.resource.http/src/com/dexels/navajo/resource/http/impl/ResourceComponent.java line 99

	@Override
	public Single<ReactiveReply> put(String tenant, String bucket, String id, String type, Publisher<byte[]> data) {
		logger.info("Putting: {} type: {}",assembleURL(tenant,bucket, id),type);
		return client.callWithBody(assembleURL(tenant,bucket, id), 
					r->r.header("Authorization", this.authorization)
						.header("Connection", "Close")
						.method(HttpMethod.PUT)
						.idleTimeout(idle_timeout, TimeUnit.SECONDS)
						.timeout(timeout, TimeUnit.SECONDS)
				,Flowable.fromPublisher(data)
				,type)
				.timeout(timeout+1, TimeUnit.SECONDS)
				.firstOrError();
	}
@ghost ghost self-assigned this Jul 7, 2020
@ghost ghost added bug Something isn't working 2 - Working Working on this issue. labels Jul 7, 2020
@ghost
Copy link

ghost commented Jul 7, 2020

Started working on branch _fix_E536_async_http_keepalive, with test scripts at navajo-basic-environment/_fix_E536_async_http_keepalive.

@ghost
Copy link

ghost commented Jul 9, 2020

Wireshark appears to show that the data is properly traveling over the wire back and forth: two data packets to the server and one back to the client per request (both with and without the workaround).

@ghost
Copy link

ghost commented Jul 9, 2020

To problem appears to be that only the status code of the HTTP response is used in the code, the content (data) of the HTTP response (presumably empty) is not consumed. A candidate solution is to attach a subscriber to the content publisher.

@ghost ghost changed the title Support "Connection : Keep-Alive" header on Http PUT Support "Connection : Keep-Alive" header on Http operations Jul 13, 2020
@ghost
Copy link

ghost commented Jul 13, 2020

To problem appears to be that only the status code of the HTTP response is used in the code, the content (data) of the HTTP response (presumably empty) is not consumed. A candidate solution is to attach a subscriber to the content publisher.

@ghost
Copy link

ghost commented Jul 13, 2020

Generalized the title since the problem also occurs with HTTP operations other than put (i.e., head and delete).

@ghost ghost added 0 - Backlog New issue, unvetted. and removed 0 - Backlog New issue, unvetted. labels Sep 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 - Working Working on this issue. bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant