Skip to content

Conversation

@rdhabalia
Copy link
Contributor

Motivation

Client creates a dedicated binary-proto lookup connection with one broker and there could be a possibility

  • Broker is in bad state: it always gives InternalServerError
  • Broker is overloaded by lookup-request from other clients
    In that case, client should close the existing connection and try to recreate which may end up with new connection with other broker and do successful lookup.

Modifications

check serverError and take appropriate action

  • InternalServerError: close connection immediately
  • TooManyRequest: received error count is more than maxNumberOfRejectedRequestPerConnection in certain time-frame

Result

Client can recreate connection with different broker if it is not able to getting successful response from one particular broker.

NOTE
I have created this PR on top of #181 changes as it requires ServerSide throttling.
So, this PR depends on #181 and we need to merge #181 first.

@rdhabalia rdhabalia added area/client type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages labels Mar 3, 2017
@rdhabalia rdhabalia added this to the 1.17 milestone Mar 3, 2017
@rdhabalia rdhabalia self-assigned this Mar 3, 2017
Copy link
Contributor

@merlimat merlimat left a comment

Choose a reason for hiding this comment

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

Change looks good, let's wait for the dependent PR to be merged and then rebase this one.

Copy link
Contributor

Choose a reason for hiding this comment

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

We should already have a ScheduledExecutor instance around that we can use.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

actually we don't have ScheduledExecutor instance around. should we create one?

Copy link
Contributor

Choose a reason for hiding this comment

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

that's true, then we could use the eventLoop associated with the netty channel to schedule the task.
(If we use a timer per connection, it will create a different thread for each of them)

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, that's right. fixed it.

@rdhabalia rdhabalia force-pushed the close_client branch 3 times, most recently from 4a35d90 to 90ce6e3 Compare March 6, 2017 19:07
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 not used anymore

Copy link
Contributor

@merlimat merlimat left a comment

Choose a reason for hiding this comment

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

👍

@merlimat merlimat merged commit 6d4bd9d into apache:master Mar 11, 2017
@rdhabalia rdhabalia deleted the close_client branch June 21, 2017 18:54
sijie pushed a commit to sijie/pulsar that referenced this pull request Mar 4, 2018
hangc0276 pushed a commit to hangc0276/pulsar that referenced this pull request May 26, 2021
…pache#274)

Fixes apache#273

When KoP handles metadata requests, it only checks the config allowAutoTopicCreation but not the same field in the metadata request. Then it causes that AdminClient#describeTopics returns a success result even if the topics don't exist and topics will be created automatically.

So this PR adds the check for metadata request's allowAutoTopicCreation field and the test for AdminClient#describeTopics.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/client type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants