combine queue deletion and binding deletion in one transaction

[jshen28]

Proposed Changes

previous, when node down, rabbit_binding:process_deletes and
delete_queues_on_node_down are performed in separate transactions.
this patch tries to merge them into one transaction.

compared with running rabbit_binding:process_deletions in one
big transaction, splitting it up and running with queue deletion
should be more ideal.

Signed-off-by: ushen [email protected]

Types of Changes

What types of changes does your code introduce to this project?
Put an x in the boxes that apply

• Bug fix (non-breaking change which fixes issue #NNNN)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause an observable behavior change in existing systems)
• Documentation improvements (corrections, new content, etc)
• Cosmetic change (whitespace, formatting, etc)

[michaelklishin]

Thank you but we won't accept this change.

We have previously explained in detail (on the mailing list) why we moved away from a single giant transaction design (#1513) and why we believe that it is a net positive, even though #1721/#1832 potentially makes this less relevant. We also explained that publishers must be ready to recover from unsuccessful publishes, which would address the originally reported issue.

If you can provide evidence (data from benchmarks) that the original contention problem is no longer present with #1721, we would reconsider this. However, a more likely timeframe for that is 4.0 when we'd have a new schema data store.

Other issues related to bulk binding removal: #1675, #1691, #1715.

[jshen28]

Hi, thank you for answering. Could you explain more about "evidence"? what benchmark should I provide? Is it enough to run perftest on this patch and latest release, and compare the result? Do you have any more specific requirements for test setup?

[lukebakken]

@jshen28 - please read the issues that @michaelklishin linked to. In the discussion, you can see how we analyzed the issue. Basically, it involved deleting very large numbers of queues and bindings at once.

[lukebakken]

@jshen28 here is an example of the kind of test we are looking for.

• I used this code to create 65536 queues that each have a binding to the gh-2190 exchange. Note that the queues are "auto delete", so each queue and its binding will be deleted when RabbitMQ stops.
• Then, run a command like this to time how long shutting RabbitMQ down takes:

  /usr/bin/time -o /home/lbakken/issues/rabbitmq/rabbitmq-server/gh-2190/master-shutdown-timed-4.txt ./sbin/rabbitmqctl shutdown --wait --timeout 120000
  

Here is the result of four shutdowns using the master branch of code, and your PR code. You can see that the timing is similar, which is encouraging:

$ cat ~/issues/rabbitmq/rabbitmq-server/gh-2190/master-shutdown-timed-*
       16.43 real         0.50 user         0.05 sys
       21.65 real         0.45 user         0.07 sys
       17.09 real         0.43 user         0.06 sys
       13.20 real         0.45 user         0.06 sys

$ cat ~/issues/rabbitmq/rabbitmq-server/gh-2190/jshen28-clean-in-one-transaction-shutdown-timed-*
       19.29 real         0.41 user         0.07 sys
       16.18 real         0.45 user         0.08 sys
       16.04 real         0.47 user         0.05 sys
       20.04 real         0.40 user         0.10 sys

I will see if I can figure out the additional performance tests that were done. @michaelklishin do you have any test scripts or details available?

[jshen28]

thank you for the testing. I will try to run it in our environment and give some feedback later on.

…

On Sat, Dec 28, 2019 at 4:21 AM Luke Bakken ***@***.***> wrote: @jshen28 <https://github.com/jshen28> here is an example of the kind of test we are looking for. - I used this code <https://gist.github.com/lukebakken/d9d2960a9e167f88f7384ec06e594896> to create 65536 queues that each have a binding to the gh-2190 exchange. Note that the queues are "auto delete", so they and their binding will be deleted when RabbitMQ stops. - Then, run a command like this to time how long shutting RabbitMQ down takes: /usr/bin/time -o /home/lbakken/issues/rabbitmq/rabbitmq-server/gh-2190/master-shutdown-timed-4.txt ./sbin/rabbitmqctl shutdown --wait --timeout 120000 Here is the result of four shutdowns using the master branch of code, and your PR code. You can see that the timing is similar, which is encouraging: $ cat ~/issues/rabbitmq/rabbitmq-server/gh-2190/master-shutdown-timed-* 16.43 real 0.50 user 0.05 sys 21.65 real 0.45 user 0.07 sys 17.09 real 0.43 user 0.06 sys 13.20 real 0.45 user 0.06 sys $ cat ~/issues/rabbitmq/rabbitmq-server/gh-2190/jshen28-clean-in-one-transaction-shutdown-timed-* 19.29 real 0.41 user 0.07 sys 16.18 real 0.45 user 0.08 sys 16.04 real 0.47 user 0.05 sys 20.04 real 0.40 user 0.10 sys I will see if I can figure out the additional performance tests that were done. @michaelklishin <https://github.com/michaelklishin> do you have any test scripts or details available? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2190?email_source=notifications&email_token=ADH3XA2I3NH6VBMYNC3HFXTQ2ZPVZA5CNFSM4J6PJHBKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHXW54Q#issuecomment-569339634>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ADH3XA6FZPJDXZYVVNWTYATQ2ZPVZANCNFSM4J6PJHBA> .

-- Best Regards, Jiatong Shen