Skip to content

Conversation

@binarin
Copy link

@binarin binarin commented Apr 4, 2017

E.g. for 10000 queues performing the sort using the default sorting
order costs ~0.3 second on my machine.

Now that rabbitmq/rabbitmq-management#369 is
merged we can avoid this cost. This change is backward compatible, for older versions of RabbitMQ adding empty sort parameter will result in using the default sort order(vhost, name)

@kbudde
Copy link
Owner

kbudde commented Apr 5, 2017

Hi,

very nice addition and I like it. Unfortunately it is not backward compatible. The empty sort paramater fails in rabbitmq 3.5 and earlier. See the error message below.

Adding a space to the sort will work with 3.4+.
config.RabbitURL+"/api/"+endpoint+"?sort=%20"

Can you say/verify that this will disabling sort in the latest rabbitmq version? Perhaps we are lucky and the parameter is trimmed.
I do not want to break with 3.4 and 3.5 for the moment.

{ error: "Internal Server Error", reason: "{error,{error,function_clause, [{rabbit_mgmt_util,get_dotted_value0, [[], [{name,<<"amq.direct">>}, {vhost,<<"/">>}, {type,direct}, {durable,true}, {auto_delete,false}, {internal,false}, {arguments,{struct,[]}}, {policy,<<"ha-2">>}]], []}, {rabbit_mgmt_util,sort_key,2,[]}, {rabbit_mgmt_util,'-sort_list/4-lc$^1/1-1-',2,[]}, {rabbit_mgmt_util,sort_list,4,[]}, {rabbit_mgmt_util,reply_list,4,[]}, {webmachine_resource,resource_call,3,[]}, {webmachine_resource,do,3,[]}, {webmachine_decision_core,resource_call,1,[]}]}} " }

@binarin
Copy link
Author

binarin commented Apr 5, 2017

Yeah, I've forgot about cowboy switch in 3.6, so currently this is only compatible with 3.6 and 3.7.

I'll add a config option for explicitly enabling new behavior

@binarin binarin force-pushed the no-sort branch 2 times, most recently from 266828a to 6a36641 Compare April 7, 2017 10:48
@binarin
Copy link
Author

binarin commented Apr 7, 2017

I've updated my code, now this no-sort functionality should be explicitly enabled.

The reason for using map to track the supported capabilities (instead of simple bool) is that I'm going to add at least one other optional feature - rabbitmq/rabbitmq-management#367

E.g. for 10000 queues performing the sort using the default sorting
order costs ~0.3 second on my machine.

Now that rabbitmq/rabbitmq-management#369 is
merged we can avoid this cost.
@kbudde
Copy link
Owner

kbudde commented Apr 17, 2017

Hi,

I've merged it into the develop branch. Docker image was created (tag: develop).
I will do some more tests and merge it aferwards.

LGTM

@kbudde kbudde assigned kbudde and unassigned binarin Apr 19, 2017
@kbudde kbudde merged commit 95cb2d5 into kbudde:master Apr 19, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants