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

close http-connection on internal server error from broker #265

Merged
merged 3 commits into from
Mar 2, 2017

Conversation

rdhabalia
Copy link
Contributor

Motivation

By default AsyncHttpClient has Keep-alive enabled and it get connected to the bad-broker then client repeatedly fails to perform lookup requests. In that case, connection should be closed and client should be able to connect with other broker to get the request served.

eg: sometimes broker is not able to create zk-session and it keeps failing to authenticate client-request so, client should be redirected to other broker by closing client-side connection on broker's InternalServerError

WARN  c.y.p.b.a.AuthorizationManager       - Client  with Role - my-role failed to get permissions for destination - persistent://property/cluster/ns/topic-1
org.apache.zookeeper.KeeperException$SessionExpiredException: KeeperErrorCode = Session expired for /admin/policies/property/cluster/ns
:
at com.yahoo.pulsar.broker.admin.PersistentTopics.getPartitionedMetadata(PersistentTopics.java:359) [pulsar-broker-1.15.7.jar:na]

Modifications

add KeepAliveStrategy in httpClient to disable keep-alive after internalServerError.

Result

AsyncHttpClient should close the connection on InternalServerError

@rdhabalia rdhabalia added this to the 1.17 milestone Mar 1, 2017
@rdhabalia rdhabalia self-assigned this Mar 1, 2017
@merlimat
Copy link
Contributor

merlimat commented Mar 1, 2017

Could we also set the closing of the HTTP session on the server side? When Jetty catches the exception is maybe possible to force it to use Connection: close in the response itself.

@rdhabalia rdhabalia force-pushed the http_strat branch 2 times, most recently from 37957e2 to c548404 Compare March 2, 2017 01:13
@rdhabalia rdhabalia merged commit 522aee8 into apache:master Mar 2, 2017
rdhabalia added a commit that referenced this pull request Mar 10, 2017
* close http-connection on internal server error from broker

* broker kills current session on internal-server-error

* add broker-address into response for client-debugging
@rdhabalia rdhabalia deleted the http_strat branch June 21, 2017 18:54
hangc0276 pushed a commit to hangc0276/pulsar that referenced this pull request May 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants