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

MDEV-35208 mtr correct behaviour of --port-base #3586

Open
wants to merge 1 commit into
base: 10.5
Choose a base branch
from

Conversation

grooverdan
Copy link
Member

  • The Jira issue number for this PR is: MDEV-35208

Description

Commit c2d9762 added additional ports for galera, and moved the starting port into the 19000 range.

Unfortunately --port-base settings have some odd relationships and now mtr --port-base=21000 will end up looking for ports 0-30.

This patch removes some existing code related to 4.0 and 5.1 MySQL testing concurrently which I assume no-one has done for 15+ years.

Replace the concept of a build_thread to refer straight to the port number of the worker being used. Once this is done the number of calculations is reduced.

Also put the default --port-base back to 16000 where it was for very many years.

Release Notes

Correct mariadb-test-suite --port-base behaviour and puts default port-base back to 16000

How can this PR be tested?

mtr main.select
mtr --mtr-port-base=19000 main.select
mtr --mtr-port-base=21000 main.select
mtr --mtr-port-base=21000 --parallel 30 main.select{,,,,,,,,,,,,,,,,,,}

If the changes are not amenable to automated testing, please explain why not and carefully describe how to test manually.

Basing the PR against the correct MariaDB version

  • This is a new feature or a refactoring, and the PR is based against the main branch.
  • This is a bug fix, and the PR is based against the earliest maintained branch in which the bug can be reproduced.

PR quality check

  • I checked the CODING_STANDARDS.md file and my PR conforms to this where appropriate.
  • For any trivial modifications to the PR, I am ok with the reviewer making the changes themselves.

Commit c2d9762 added
additional ports for galera, and moved the starting port
into the 19000 range.

Unfortunately --port-base settings have some odd relationships
and now mtr --port-base=21000 will end up looking for ports 0-30.

This patch removes some existing code related to 4.0 and 5.1
MySQL testing concurrently which I assume no-one has done for
15+ years.

Replace the concept of a build_thread to refer straight
to the port number of the worker being used. Once this is
done the number of calculations is reduced.

Also put the default --port-base back to 16000 where it
was for very many years.
Copy link
Member

@elenst elenst left a comment

Choose a reason for hiding this comment

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

This is incorrect in general. The build thread is not the same as the port base. They have different ranges and different meanings.

MTR_BUILD_THREAD is a "magic number" based on which (and the group size) the base port is calculated. E.g. with mtr-build-thread=400, the base port (in the absence of rivals) would be 14000 in MySQL, in MariaDB it would be 18000 with group size 20, and 22000 with group size 30. The default value of MTR_BUILD_THREAD is 300, which gives port 13000 in MySQL, 16000 in MariaDB before increasing the group size, and 19000 after.

Strange invention as it is, MTR_BUILD_THREAD has existed for many years now, both in MySQL and MariaDB. It can be configured on a command line or in the environment. This patch breaks it for every value other than "auto".

MTR_PORT_BASE, on the other hand, is a later invention (probably because MTR_BUILD_THREAD is too obscure). It is meant to be a non-magic beginning of the port range. It never worked in MariaDB, as it was adopted from MySQL "as is", with the assumption of the hardcoded group size 10, while in MariaDB the group size was 20 and now is 30. If we were to fix mtr-port-base logic, it would probably be something like

diff --git a/mysql-test/mysql-test-run.pl b/mysql-test/mysql-test-run.pl
index 7127bedffdc..08a05668161 100755
--- a/mysql-test/mysql-test-run.pl
+++ b/mysql-test/mysql-test-run.pl
@@ -1474,7 +1474,7 @@ sub command_line_setup {
       mtr_warning ("Port base $opt_port_base rounded down to multiple of 10");
       $opt_port_base-= $rem;
     }
-    $opt_build_thread= $opt_port_base / 10 - 1000;
+    $opt_build_thread= ($opt_port_base - 10000) / $opt_port_group_size
   }

but not quite, because it would also rounding problems, so it needs to be done more carefully.

It won't, however, change the default 19000 back to 16000, because 19000 is a result of a valid calculation according to the written algorithm. To change it back to 16000, we would need to change the default MTR_BUILD_THREAD to 200. I have no strong opinion on whether we want to do it.

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

Successfully merging this pull request may close these issues.

2 participants