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

By @ayanda-D: make sure (non-replicated) CQs (classic queues) emit leader and members metrics, just like replicated QQs #12437

Merged
merged 5 commits into from
Oct 3, 2024

Conversation

michaelklishin
Copy link
Member

This is #12422 by @Ayanda-D with a different title and a slightly different explanation from me.

This change may seem controversial at first given the removal of classic queue mirroring starting with 4.0 but this is just a matter of unifying metric keys between non-replicated CQs and replicated QQs.

So with this change, CQs will report a leader and a list of members (of one node, just the leader) as stats keys. For v4.0.x, this seems perfectly fine.

In fact, this will help improve https://github.com/rabbitmq/rabbitmqadmin-ng with its stricter JSON deserializer (compared to the original Python script).

@michaelklishin
Copy link
Member Author

I will have to rebase this to make check_workflow pass. Will do so later today.

@michaelklishin
Copy link
Member Author

michaelklishin commented Oct 3, 2024

Submitted a review for #12422: the new test fails on Actions with

=== Location: [{classic_queue_SUITE,'-leader_locator_client_local/1-lc$^0/1-0-',[87](https://github.com/rabbitmq/rabbitmq-server/pull/classic_queue_suite.src.html#87)},
              {classic_queue_SUITE,'-leader_locator_client_local/1-lc$^0/1-0-',[91](https://github.com/rabbitmq/rabbitmq-server/pull/classic_queue_suite.src.html#91)},
              {test_server,ts_tc,1793},
              {test_server,run_test_case_eval1,1302},
              {test_server,run_test_case_eval,1234}]
=== === Reason: {assertEqual,
                     [{module,classic_queue_SUITE},
                      {line,87},
                      {expression,
                          "rabbit_ct_broker_helpers : rpc ( Config , Server , rabbit_amqqueue , info , [ Leader0 , [ leader , members ] ] )"},
                      {expected,
                          [{leader,'rmq-ct-cluster_size_3-2-21192@localhost'},
                           {members,
                               ['rmq-ct-cluster_size_3-2-21192@localhost']}]},
                      {value,[{leader,''},{members,''}]}]}
  in function  classic_queue_SUITE:'-leader_locator_client_local/1-lc$^0/1-0-'/2 (classic_queue_SUITE.erl, line 87)
  in call from classic_queue_SUITE:'-leader_locator_client_local/1-lc$^0/1-0-'/2 (classic_queue_SUITE.erl, line 91)
  in call from test_server:ts_tc/3 (test_server.erl, line 1793)
  in call from test_server:run_test_case_eval1/6 (test_server.erl, line 1302)
  in call from test_server:run_test_case_eval/9 (test_server.erl, line 1234)

This assertion generally does not belong to this test. It does not test info items returned by CQ processes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants