Skip to content
This repository was archived by the owner on Nov 16, 2020. It is now read-only.

Only preserve stats for local queues #49

Merged
merged 7 commits into from
Jul 10, 2017
Merged

Conversation

lukebakken
Copy link
Contributor

@lukebakken lukebakken commented Jun 28, 2017

@lukebakken lukebakken changed the base branch from master to stable June 29, 2017 16:30
@lukebakken lukebakken changed the title Only preserve stats for local queues WIP: Only preserve stats for local queues Jun 29, 2017
@lukebakken lukebakken changed the title WIP: Only preserve stats for local queues Only preserve stats for local queues Jun 30, 2017
@lukebakken lukebakken requested a review from dcorbacho June 30, 2017 13:54
handle_info({run_gc_soon, Interval}, State) ->
{noreply, start_timer(Interval, State)};
handle_info(run_gc_soon, State) ->
{noreply, start_timer(5000, State)};
Copy link
Contributor

Choose a reason for hiding this comment

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

Use an interval from config, so the default could be easily changed.

Else we'd have to wait for a full stats background gc which can take a
very long time during which metrics may be duplicated in the UI.

[#147753285]
@kjnilsson kjnilsson force-pushed the rabbitmq-management-427 branch from de8cc0a to 983a228 Compare July 4, 2017 10:59
kjnilsson added 2 commits July 5, 2017 09:43
When zeroing out stats after deletion.

[#147753285]
@lukebakken
Copy link
Contributor Author

@kjnilsson - there used to be a change to this line to use rabbit_amqqueue:list_local_names(). Is this no longer needed?

@kjnilsson
Copy link
Contributor

@lukebakken that is correct. Management GC needs to use the cluster-wide list of queues in order to retain historical queue metrics for queues that used to reside on the current node.

[insert_entry(queue_msg_stats, Queue, TS,
QNegStats, Size, Interval, false)
|| {Size, Interval} <- BPolicies],
% these need to be delayed by an interval
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kjnilsson - is this comment still relevant? i.e, should a delay be introduced?

Copy link
Contributor

Choose a reason for hiding this comment

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

Good point that can be removed.

@michaelklishin michaelklishin merged commit 1553a70 into stable Jul 10, 2017
@lukebakken lukebakken deleted the rabbitmq-management-427 branch July 10, 2017 18:39
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants