Skip to content

Commit 0fb28e1

Browse files
Management stats DB: make list_queue_stats/4 more defensive
Closes #16989.
1 parent 195e549 commit 0fb28e1

2 files changed

Lines changed: 70 additions & 18 deletions

File tree

deps/rabbitmq_management/src/rabbit_mgmt_db.erl

Lines changed: 32 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -381,10 +381,17 @@ list_queue_stats(Ranges, Objs, Interval, Fun) ->
381381
[begin
382382
Id = id_lookup(queue_stats, Obj),
383383
Pid = pget(pid, Obj),
384-
QueueData = maps:get(Id, DataLookup),
385-
Props = maps:get(queue_stats, QueueData),
386-
Stats = queue_stats(QueueData, Ranges, Interval),
387-
{Pid, combine(Props, Obj) ++ Stats}
384+
%% A queue deleted before its metrics were collected won't have an entry.
385+
%% Return it without any stats rather than throwing an exception.
386+
%% See rabbitmq/rabbitmq-server#16989.
387+
case maps:find(Id, DataLookup) of
388+
{ok, QueueData} ->
389+
Props = maps:get(queue_stats, QueueData),
390+
Stats = queue_stats(QueueData, Ranges, Interval),
391+
{Pid, combine(Props, Obj) ++ Stats};
392+
error ->
393+
{Pid, Obj}
394+
end
388395
end || Obj <- Objs]).
389396

390397
detail_queue_stats(Ranges, Objs, Interval) ->
@@ -395,20 +402,27 @@ detail_queue_stats(Ranges, Objs, Interval) ->
395402
[begin
396403
Id = id_lookup(queue_stats, Obj),
397404
Pid = pget(pid, Obj),
398-
QueueData = maps:get(Id, DataLookup),
399-
Props = maps:get(queue_stats, QueueData),
400-
Stats = queue_stats(QueueData, Ranges, Interval),
401-
ConsumerStats = rabbit_mgmt_data_compat:fill_consumer_active_fields(
402-
maps:get(consumer_stats, QueueData)),
403-
Consumers = [{consumer_details, ConsumerStats}],
404-
StatsD = [{deliveries,
405-
detail_stats(QueueData, channel_queue_stats_deliver_stats,
406-
deliver_get, second(Id), Ranges, Interval)},
407-
{incoming,
408-
detail_stats(QueueData, queue_exchange_stats_publish,
409-
fine_stats, first(Id), Ranges, Interval)}],
410-
Details = augment_details(Obj, []),
411-
{Pid, combine(Props, Obj) ++ Stats ++ StatsD ++ Consumers ++ Details}
405+
%% See `list_queue_stats/4` and rabbitmq/rabbitmq-server#16989.
406+
case maps:find(Id, DataLookup) of
407+
{ok, QueueData} ->
408+
Props = maps:get(queue_stats, QueueData),
409+
Stats = queue_stats(QueueData, Ranges, Interval),
410+
ConsumerStats = rabbit_mgmt_data_compat:fill_consumer_active_fields(
411+
maps:get(consumer_stats, QueueData)),
412+
Consumers = [{consumer_details, ConsumerStats}],
413+
StatsD = [{deliveries,
414+
detail_stats(QueueData, channel_queue_stats_deliver_stats,
415+
deliver_get, second(Id), Ranges, Interval)},
416+
{incoming,
417+
detail_stats(QueueData, queue_exchange_stats_publish,
418+
fine_stats, first(Id), Ranges, Interval)}],
419+
Details = augment_details(Obj, []),
420+
{Pid, combine(Props, Obj) ++ Stats ++ StatsD ++ Consumers ++ Details};
421+
error ->
422+
%% consumer_details must be present: the channel detail
423+
%% merge below reads it from every entry.
424+
{Pid, [{consumer_details, []} | Obj]}
425+
end
412426
end || Obj <- Objs]),
413427

414428
% patch up missing channel details

deps/rabbitmq_management/test/rabbit_mgmt_test_db_SUITE.erl

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ groups() ->
3333
[
3434
{non_parallel_tests, [], [
3535
queue_coarse_test,
36+
queue_stats_missing_data_test,
3637
connection_coarse_test,
3738
fine_stats_aggregation_time_test,
3839
fine_stats_aggregation_test,
@@ -135,6 +136,43 @@ queue_coarse_test1(_Config) ->
135136
[rabbit_mgmt_metrics_collector:reset_lookups(T) || {T, _} <- ?CORE_TABLES],
136137
ok.
137138

139+
queue_stats_missing_data_test(Config) ->
140+
ok = rabbit_ct_broker_helpers:rpc(Config, 0, ?MODULE,
141+
queue_stats_missing_data_test1, [Config]).
142+
143+
%% A queue deleted before its metrics were collected won't have an entry.
144+
%% Return it without any stats rather than throwing an exception and aborting
145+
%% the entire HTTP client request.
146+
%%
147+
%% See rabbitmq/rabbitmq-server#16989.
148+
queue_stats_missing_data_test1(_Config) ->
149+
ok = meck:new(rabbit_mgmt_data, [passthrough, no_link]),
150+
try
151+
Empty = fun(_, _, _) -> #{} end,
152+
meck:expect(rabbit_mgmt_data, all_list_basic_queue_data, Empty),
153+
meck:expect(rabbit_mgmt_data, all_list_queue_data, Empty),
154+
meck:expect(rabbit_mgmt_data, all_detail_queue_data, Empty),
155+
Obj = [{name, <<"vanished">>},
156+
{vhost, <<"/">>},
157+
{pid, self()},
158+
{state, live}],
159+
Now = exometer_slide:timestamp(),
160+
R = #range{first = Now - 5000, last = Now, incr = 5000},
161+
Ranges = {R, R, R, R},
162+
%% Non-empty ranges bypass the listing cache so each mode runs.
163+
[Basic] = rabbit_mgmt_db:augment_queues([Obj], Ranges, basic),
164+
<<"vanished">> = pget(name, Basic),
165+
[Detailed] = rabbit_mgmt_db:augment_queues([Obj], Ranges, detailed),
166+
<<"vanished">> = pget(name, Detailed),
167+
[Full] = rabbit_mgmt_db:augment_queues([Obj], Ranges, full),
168+
<<"vanished">> = pget(name, Full),
169+
%% The detail mode always carries consumer_details.
170+
[] = pget(consumer_details, Full)
171+
after
172+
meck:unload(rabbit_mgmt_data)
173+
end,
174+
ok.
175+
138176
%% Generate a well-formed interval from Start using Interval steps
139177
last_ts(First, Interval) ->
140178
Now = exometer_slide:timestamp(),

0 commit comments

Comments
 (0)