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

Allow client(producer/consumer) to check topic stats #482

Merged
merged 1 commit into from
Jun 15, 2017

Conversation

rdhabalia
Copy link
Contributor

Motivation

In many cases subscribers of the topic requires to check the stats and internal-stats of the topics to check backlog, unack-messages, markDeletePosition, etc. Right now, broker only allows stats check to property owner and subscribers may not have property ownership.

Modifications

Allow producer/consumer to check the topic stats.

Result

producer/consumer can check the topic stats now.

@rdhabalia rdhabalia added the type/feature The PR added a new feature or issue requested a new feature label Jun 15, 2017
@rdhabalia rdhabalia added this to the 1.18 milestone Jun 15, 2017
@rdhabalia rdhabalia self-assigned this Jun 15, 2017
protected void validateAdminAndClientPermission(DestinationName destination) {
try {
validateAdminAccessOnProperty(destination.getProperty());
} catch (Exception ve) {
Copy link
Contributor

@merlimat merlimat Jun 15, 2017

Choose a reason for hiding this comment

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

Minor thing: do you think it would be easy to not rely on exceptions on the normal cases? Exception can be expensive to throw (basically it needs to generate the whole stack trace).

If that requires lot of refactoring, let's go ahead with this change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

do you think it would be easy to not rely on exceptions

We can but then we need to ignore code-modularity where we need to modify property-check-function which will validate permission on property and topic based on given booleans which will decide what to validate. If it will be ugly and if we don't touch this method then we need to add duplicate code to introduce such method. so, do you think it's worth to introduce duplicate-code or ignoring modularity and stats call may not be used that often in normal case.

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 ok, let's not worry about it for now

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.

👍

@rdhabalia rdhabalia merged commit 29b2dd1 into apache:master Jun 15, 2017
@rdhabalia rdhabalia deleted the statsPer branch June 21, 2017 18:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/feature The PR added a new feature or issue requested a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants