Only preserve stats for local queues #1278
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Simplifies code formatting by using intermediate variables in test suite setup.
Previously, GC would not clear out metrics for non-local queues, so stale data remained in the metrics tables.
lukebakken
changed the title
|
Why triggering a full GC when it is know which queue is being synchronised? If we want to show the queue changes instantly, it would be more effective to GC only the named queue. |
dcorbacho
requested changes
Jul 3, 2017
src/rabbit_core_metrics_gc.erl
Outdated
| handle_info({run_gc_soon, Interval}, State) -> | ||
| {noreply, start_timer(Interval, State)}; | ||
| handle_info(run_gc_soon, State) -> | ||
| {noreply, start_timer(5000, State)}; |
There was a problem hiding this comment.
Use an interval from config, so the default could be easily changed.
src/rabbit_mirror_queue_sync.erl
Outdated
|
|
||
| run_gc_soon() -> | ||
| rabbit_core_metrics_gc:run_gc_soon(), | ||
| rabbit_mgmt_gc:run_gc_soon(). |
There was a problem hiding this comment.
rabbit application can never call anything on rabbitmq_management or rabbitmq_management_agent as those plugins might or might not be active. The dependency is the other way around.
=ERROR REPORT==== 3-Jul-2017::14:07:12 ===
Error in process <0.631.0> on node 'rmq-ct-cluster_tests-1-21000@localhost' with exit value:
{undef,[{rabbit_mgmt_gc,run_gc_soon,[],[]},
{rabbit_mirror_queue_sync,syncer_loop,3,
[{file,"src/rabbit_mirror_queue_sync.erl"},
{line,266}]}]}
| % when a node starts. | ||
| rabbit_ct_broker_helpers:rpc(Config, Node0, rabbit_core_metrics_gc, start_link, []), | ||
| rabbit_ct_broker_helpers:rpc(Config, Node1, rabbit_core_metrics_gc, start_link, []), | ||
| rabbit_ct_broker_helpers:rpc(Config, Node0, rabbit_mgmt_gc, start_link, []), |
There was a problem hiding this comment.
This part of the test (call rabbit_mgmt_gc) could never work on the rabbit app, as the management application is not started.
=== Ended at 2017-07-03 14:07:12
=== Location: [{rabbit_ct_broker_helpers,rpc,967},
{rabbit_core_metrics_gc_SUITE,cluster_queue_metrics,383},
{test_server,ts_tc,1533},
{test_server,run_test_case_eval1,1053},
{test_server,run_test_case_eval,985}]
=== Reason: undefined function rabbit_mgmt_gc:run_gc_soon/1
in function rpc:'-handle_call_call/6-fun-0-'/5 (rpc.erl, line 187)
=ERROR REPORT==== 3-Jul-2017::14:07:12 ===
Error in process <0.631.0> on node 'rmq-ct-cluster_tests-1-21000@localhost' with exit value:
{undef,[{rabbit_mgmt_gc,run_gc_soon,[],[]},
{rabbit_mirror_queue_sync,syncer_loop,3,
[{file,"src/rabbit_mirror_queue_sync.erl"},
{line,266}]}]}
| Node0 = rabbit_ct_broker_helpers:get_node_config(Config, 0, nodename), | ||
| Node1 = rabbit_ct_broker_helpers:get_node_config(Config, 1, nodename), | ||
|
|
||
| % Note: must start these processes as there is a delay in starting them up |
There was a problem hiding this comment.
Wait until everything is started
| % Note: the default is to schedule a GC 5 seconds after a sync, but we want it a bit | ||
| % sooner here. Note that running it too soon can cause badarg errors for missing | ||
| % stats tables | ||
| rabbit_ct_broker_helpers:rpc(Config, Node0, rabbit_core_metrics_gc, run_gc_soon, [1000]), |
There was a problem hiding this comment.
Why trigger it manually? Then the test will never verify that the synchronisation is doing the clean up. Moreover, setting a configurable default will allow to reduce the wait time
kjnilsson
force-pushed
the
rabbitmq-management-427
branch
2 times, most recently
from
c77ec2a to
872cdd2
Compare
This change handles all non-crash termination cases. The assumption here is that once an amqqueue_process terminates the master is no longer on the current node. [#147753285]
kjnilsson
force-pushed
the
rabbitmq-management-427
branch
from
872cdd2 to
0e9b333
Compare
dcorbacho
approved these changes
Jul 10, 2017
michaelklishin
approved these changes
Jul 10, 2017
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes rabbitmq/rabbitmq-management#427
By using
list_local_names, metrics entries that are stale are removed.