Skip to content
/ server Public

Comments

Misc Debian/Salsa-CI fixes#2299

Merged
LinuxJedi merged 5 commits intoMariaDB:10.6from
ottok:10.6-salsa-ci-oct2022
Nov 11, 2022
Merged

Misc Debian/Salsa-CI fixes#2299
LinuxJedi merged 5 commits intoMariaDB:10.6from
ottok:10.6-salsa-ci-oct2022

Conversation

@ottok
Copy link
Contributor

@ottok ottok commented Oct 23, 2022

Description

See commit messages for details of changes.

How can this PR be tested?

Anybody can sign up for a free account at salsa.debian.org, the Gitlab instance for Debian development.

The Salsa-CI status for current 10.6 head (ab01901) is visible at https://salsa.debian.org/otto/mariadb-server/-/pipelines/443617

image

CI status after these commits is visible at https://salsa.debian.org/otto/mariadb-server/-/pipelines/444495
image

Once #2294 is merged up from branch 10.5 to 10.6 and once @illuusio completes the Lintian fixes in #2275 the CI status will be even more green as visible in the mockup at https://salsa.debian.org/otto/mariadb-server/-/pipelines/444496
image

The last failure will be fixed if/when I finalize #2300. It is a hard one, so I wasn't able to crack it this weekend.

Basing the PR against the correct MariaDB version

  • This is a bug fix and the PR is based against the earliest branch in which the bug can be reproduced

Backward compatibility

These changes only affect building and testing. Thus there are no changes in the installed MariaDB binaries and thus this is safe to do on a stable branch. Actually this improves the safety of the stable branch as it gets back the Salsa-CI test coverage it used to have.

@ottok ottok changed the title WIP: Misc Debian/Salsa-CI fixes Misc Debian/Salsa-CI fixes Oct 23, 2022
@ottok ottok force-pushed the 10.6-salsa-ci-oct2022 branch from 97bb6b4 to 9b59c47 Compare October 25, 2022 07:07
@ottok ottok requested review from grooverdan and illuusio October 27, 2022 06:05
@ottok ottok force-pushed the 10.6-salsa-ci-oct2022 branch from 9b59c47 to e92812c Compare October 29, 2022 21:44
@ottok
Copy link
Contributor Author

ottok commented Oct 29, 2022

Rebased now on latest 10.6, and previous version already had all review comments addressed. Please re-review.

@illuusio
Copy link
Contributor

Seems solid to me. @grooverdan had some hacks on Sid building so if he is fine with this then I'm also. @LinuxJedi do you have any comments?

@LinuxJedi
Copy link
Contributor

Seems solid to me. @grooverdan had some hacks on Sid building so if he is fine with this then I'm also. @LinuxJedi do you have any comments?

Three of our DEB autobake builders are now failing in weird ways with this PR. I don't fully understand why yet, but I'm cautious about merging it until the cause is understood.

@ottok
Copy link
Contributor Author

ottok commented Nov 1, 2022 via email

@ottok ottok force-pushed the 10.6-salsa-ci-oct2022 branch 2 times, most recently from b8afce1 to 78e245e Compare November 3, 2022 06:54
@ottok
Copy link
Contributor Author

ottok commented Nov 3, 2022

Three of our DEB autobake builders are now failing in weird ways with this PR. I don't fully understand why yet, but I'm cautious about merging it until the cause is understood.

The remaining issue is that this line seems to evaluate to true and it always disabled Columnstore:

# ColumnStore can build only on amd64 and arm64
ifneq (,$(filter $(DEB_HOST_ARCH),amd64 arm64))
    CMAKEFLAGS += -DPLUGIN_COLUMNSTORE=NO
endif

Buildbot amd64-debian-10-deb-autobake fails because it expects Columnstore to be built.

@ottok ottok force-pushed the 10.6-salsa-ci-oct2022 branch from 78e245e to 2785f56 Compare November 3, 2022 15:00
@ottok
Copy link
Contributor Author

ottok commented Nov 3, 2022

Builder amd64-ubuntu-2004-debug seems to fail randomly on tests:

Completed: Failed 3/5772 tests, 99.95% were successful.
Failing test(s): encryption.innodb_encryption_discard_import

encryption.innodb_encryption_discard_import 'innodb' w24 [ fail ]
        Test ended at 2022-11-03 04:13:10
CURRENT_TEST: encryption.innodb_encryption_discard_import
--- /home/buildbot/amd64-ubuntu-2004-debug/build/mysql-test/suite/encryption/r/innodb_encryption_discard_import.result	2022-11-03 04:04:02.000000000 +0000
+++ /home/buildbot/amd64-ubuntu-2004-debug/build/mysql-test/suite/encryption/r/innodb_encryption_discard_import.reject	2022-11-03 04:13:09.655334209 +0000
@@ -22,7 +22,7 @@
 # t1 yes on expecting NOT FOUND
 NOT FOUND /foobar/ in t1.ibd
 # t2 ... on expecting NOT FOUND
-NOT FOUND /temp/ in t2.ibd
+FOUND 1 /temp/ in t2.ibd
 # t3 ... on expecting NOT FOUND
 NOT FOUND /barfoo/ in t3.ibd
 # restart

Completed: Failed 3/5772 tests, 99.95% were successful.
Failing test(s): encryption.innodb-redo-nokeys

encryption.innodb-redo-nokeys 'cbc,innodb' w26 [ fail ]  Found warnings/errors in server log file!
        Test ended at 2022-11-03 15:13:09
line
2022-11-03 15:13:08 0 [ERROR] InnoDB: Unable to apply log to corrupted page [page id: space=1, page number=3]; set innodb_force_recovery to ignore
^ Found warnings in /home/buildbot/amd64-ubuntu-2004-debug/build/mysql-test/var/26/log/mysqld.1.err
ok

I don't think the above is related to this PR but I can't prove it.

The builder buildbot/amd64-centos-7-rpm-autobake fails on tarball fetch, not related to this PR.

@ottok ottok force-pushed the 10.6-salsa-ci-oct2022 branch from 2785f56 to a1ab2e6 Compare November 3, 2022 15:43
@CLAassistant
Copy link

CLAassistant commented Nov 4, 2022

CLA assistant check
All committers have signed the CLA.

@ottok ottok requested review from LinuxJedi and removed request for illuusio November 4, 2022 05:10
@ottok
Copy link
Contributor Author

ottok commented Nov 4, 2022

This CI run passed, no failures - random or otherwise. Please re-review.

@ottok ottok dismissed LinuxJedi’s stale review November 4, 2022 05:39

Issue addressed

@LinuxJedi
Copy link
Contributor

@bigon can you please either click through the CLA to approve or choose the 3-Clause BSD license or declare here that your contributions for this pull request are under the 3-Clause BSD license.

@bigon
Copy link

bigon commented Nov 4, 2022

@bigon can you please either click through the CLA to approve or choose the 3-Clause BSD license or declare here that your contributions for this pull request are under the 3-Clause BSD license.

That change has been pulled directly from the debian/rules from the debian package: https://salsa.debian.org/mariadb-team/mariadb-server/-/commit/77202c19642bfd60b0f3d48cc304cede5a576538 so according to the debian/copyright file, the content of the debian/* directory is licensed as GPL-2+.

But the change is trivial enough and I'm not intending to sign a CLA, so I hereby declare here that my contribution for this pull request are under the 3-Clause BSD license in addition to GPL-2+ (as stated in the debian/copyright file in the debian package).

The github search functionality (didn't clone the full repository) doesn't show me any reference to a CLA or 3-Clause BSD license in the code or the COPYING/CONTRIBUTING.md files, BTW.

@LinuxJedi
Copy link
Contributor

Thank you, and noted on the .md files. We are working on improving the process in-general.

@LinuxJedi
Copy link
Contributor

@ottok can you please take a look at this output, around line 1017? I think this is breaking aarch64 autobake: https://buildbot.mariadb.org/#/builders/44/builds/14243/steps/5/logs/stdio

@ottok ottok force-pushed the 10.6-salsa-ci-oct2022 branch from c552fcf to 3e70922 Compare November 6, 2022 23:13
@ottok
Copy link
Contributor Author

ottok commented Nov 7, 2022

Comparing https://buildbot.mariadb.org/#/builders/44/builds/14243/steps/5/logs/stdio (fail) and to others the difference was that the failing run included -DWITH_PMEM=yes. Thus I now dropped the extra commit about it and now https://buildbot.mariadb.org/#/builders/44/builds/14265/steps/5/logs/stdio passed.

I will continue with the other commit in another (later) PR - for now I just want this "green" part to get in so we have progress on mainline 10.6.

@ottok ottok force-pushed the 10.6-salsa-ci-oct2022 branch from 3e70922 to f9bf6fc Compare November 7, 2022 03:48
@illuusio
Copy link
Contributor

illuusio commented Nov 7, 2022

This one is not particularly doing something with this bug merge this and #2275 to see how much problems it solved on Salsa-CI now there is branch ottok-10.6-salsa-ci-oct2022-MDEV-28834 just for testing on Salsa-CI also

@LinuxJedi
Copy link
Contributor

Three of our DEB autobake builders are now failing in weird ways with this PR. I don't fully understand why yet, but I'm cautious about merging it until the cause is understood.

The remaining issue is that this line seems to evaluate to true and it always disabled Columnstore:

# ColumnStore can build only on amd64 and arm64
ifneq (,$(filter $(DEB_HOST_ARCH),amd64 arm64))
    CMAKEFLAGS += -DPLUGIN_COLUMNSTORE=NO
endif

Buildbot amd64-debian-10-deb-autobake fails because it expects Columnstore to be built.

Missed this comment before. Isn't this because PLUGIN_COLUMNSTORE=NO is in two different places? It looked like only one was removed when I last looked at it.

Copy link
Contributor

@LinuxJedi LinuxJedi left a comment

Choose a reason for hiding this comment

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

I think we are looking good now, related failures appear to be fixed.

@LinuxJedi
Copy link
Contributor

Approved, but needs a manual rebasing it seems.

@ottok ottok requested a review from LinuxJedi November 9, 2022 08:09
@ottok ottok force-pushed the 10.6-salsa-ci-oct2022 branch from f9bf6fc to 51fb1fe Compare November 9, 2022 08:09
@ottok
Copy link
Contributor Author

ottok commented Nov 9, 2022

Rebased on latest 10.6.

…cleanup

This fixes autobake-deb.sh builds on Sid which was visible as 4 failing
build steps on Salsa-CI.

- In Sid the LSBNAME might evaluate to 'n/a', so accept it as 'Sid' to
  fix builds that failed with error:

    Error - unknown release codename n/a

- Refactor list to have Ubuntu versions first, then Debian, and as last
  the special case of Debian Sid

- Fix minor syntax issues detected by Shellcheck

Also remove useless DEB_HOST_ARCH_CPU check from debian/rules:
* It was never in effect as the 'sed' in autobake-deb.sh cleared it anyway
* The variable name was wrong and always empty
* If variable would have been correct, logic was still reversed
This commit fixes 3 certain CI job failures, several potential failures
due to timeouts, and exposes MDEV-28640 on a job that no longer should
be ignored.

- Define 3h timeout as the default 1h timeout on Gitlab.com (and others)
  is usually not enough for initial (uncached) MariaDB builds.

- Replace Buster to Bookworm/Sid upgrade testing with upgrade inside Buster
  testing as direct upgrades from Stretch to Bullseye and Buster to Bookworm
  are no longer possible due to:

    Bug#993755: libcrypt.so.1: cannot open shared object file when
    upgrading from Stretch to Sid (https://bugs.debian.org/993755)

- Stop ignoring MariaDB.org 10.6 to this version upgrade testing failures
  to reveal bug MDEV-28640. Originally this step was failing as the uring
  dependencies in upstream builders lagged behind and there was nothing
  that needed work, only time time to resolve. Now there is an actual bug
  in packaging that should be visible as a CI failure.

- Stop testing for 'service mysql status' on systems that upgraded from
  MySQL 8.0 to MariaDB.org vended 10.6. Due to some unidentified debian/control
  changes in 10.6 on upstream the upgrade is no longer compatible in
  a way that would maintain the init.d script with name 'mysql'.

- Fix typos where mergers had changed occurrences of 10.5 to 10.6 while
  they intentionally need to be exactly 10.5, otherwise the meaning
  changes.
- Align autopkgtest code with downstream official Debian packaging one.
  This is change is safe on a stable branch because is only affects builds
  and testing, not any actual usage of MariaDB 10.6.

- Standardize on using capitalized 'YES' in CMake build options
  (instead of 'yes' or mixed case)

- Add some comments to better document debian/rules

- Fix typo in Lintian overrides
Ubuntu bug: https://bugs.launchpad.net/ubuntu/+source/mariadb-10.6/+bug/1970634
MariaDB ticket: https://jira.mariadb.org/browse/MDEV-25633

When built with LTO on Ubuntu, MariaDB does not catch an exception when
the uring initialization fails due to a low RLIMIT_MEMLOCK value.

This commit amends the commit 0609b34
to be identical to the one done downstream in Debian:
https://salsa.debian.org/mariadb-team/mariadb-server/-/commit/8d20ca979cf422d3a507283b86c2547d78559179

This way both the inline comments and 'git blame' for this section will
show properly why this is needed, and the fix is one that is fully tested
on Debian and Ubuntu.

Also having this section fully identical in upstream MariaDB and downstream
Debian will make the packaging maintenance easier as 'diff` runs on this
file will not flag this as a difference anymore.
In MDEV-28640 the init script failed to stop/start the MariaDB server
due to missing mysqladmin on the system. This was however very hard to
spot from the console output.

Add an explicit check for the binary the script depends on, and fail
verbosely if the dependency is missing.
@ottok ottok force-pushed the 10.6-salsa-ci-oct2022 branch from 51fb1fe to a7fb9f8 Compare November 11, 2022 05:12
@ottok
Copy link
Contributor Author

ottok commented Nov 11, 2022

Rebased on latest 10.6. Can you @illuusio merge if you approve?

@LinuxJedi LinuxJedi merged commit f4adf35 into MariaDB:10.6 Nov 11, 2022
@ottok
Copy link
Contributor Author

ottok commented Nov 11, 2022

@LinuxJedi Why did you squash and merge this commit?

The commit message is now hard to read and diffs are not 'atomic':

From f4adf3547420cced027d26ee8b8190be8a2bdea2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Otto=20Kek=C3=A4l=C3=A4inen?= <otto@kekalainen.net>
Date: Thu, 10 Nov 2022 23:51:34 -0800
Subject: [PATCH] Misc Debian/Salsa-CI fixes (#2299)

* Deb: Handle codename 'n/a' from Debian Sid properly and autobake-deb cleanup

This fixes autobake-deb.sh builds on Sid which was visible as 4 failing
build steps on Salsa-CI.

- In Sid the LSBNAME might evaluate to 'n/a', so accept it as 'Sid' to
  fix builds that failed with error:

    Error - unknown release codename n/a

- Refactor list to have Ubuntu versions first, then Debian, and as last
  the special case of Debian Sid

- Fix minor syntax issues detected by Shellcheck

Also remove useless DEB_HOST_ARCH_CPU check from debian/rules:
* It was never in effect as the 'sed' in autobake-deb.sh cleared it anyway
* The variable name was wrong and always empty
* If variable would have been correct, logic was still reversed

- Define 3h timeout as the default 1h timeout on Gitlab.com (and others)
  is usually not enough for initial (uncached) MariaDB builds.

- Replace Buster to Bookworm/Sid upgrade testing with upgrade inside Buster
  testing as direct upgrades from Stretch to Bullseye and Buster to Bookworm
  are no longer possible due to:

    Bug#993755: libcrypt.so.1: cannot open shared object file when
    upgrading from Stretch to Sid (https://bugs.debian.org/993755)

- Stop ignoring MariaDB.org 10.6 to this version upgrade testing failures
  to reveal bug MDEV-28640. Originally this step was failing as the uring
  dependencies in upstream builders lagged behind and there was nothing
  that needed work, only time time to resolve. Now there is an actual bug
  in packaging that should be visible as a CI failure.

- Stop testing for 'service mysql status' on systems that upgraded from
  MySQL 8.0 to MariaDB.org vended 10.6. Due to some unidentified debian/control
  changes in 10.6 on upstream the upgrade is no longer compatible in
  a way that would maintain the init.d script with name 'mysql'.

- Fix typos where mergers had changed occurrences of 10.5 to 10.6 while
  they intentionally need to be exactly 10.5, otherwise the meaning
  changes.

- Align autopkgtest code with downstream official Debian packaging one.
  This is change is safe on a stable branch because is only affects builds
  and testing, not any actual usage of MariaDB 10.6.

- Standardize on using capitalized 'YES' in CMake build options
  (instead of 'yes' or mixed case)

- Add some comments to better document debian/rules

- Fix typo in Lintian overrides

Ubuntu bug: https://bugs.launchpad.net/ubuntu/+source/mariadb-10.6/+bug/1970634
MariaDB ticket: https://jira.mariadb.org/browse/MDEV-25633

When built with LTO on Ubuntu, MariaDB does not catch an exception when
the uring initialization fails due to a low RLIMIT_MEMLOCK value.

This commit amends the commit 0609b345554f9a148e165c497aadbe368e0900aa
to be identical to the one done downstream in Debian:
https://salsa.debian.org/mariadb-team/mariadb-server/-/commit/8d20ca979cf422d3a507283b86c2547d78559179

This way both the inline comments and 'git blame' for this section will
show properly why this is needed, and the fix is one that is fully tested
on Debian and Ubuntu.

Also having this section fully identical in upstream MariaDB and downstream
Debian will make the packaging maintenance easier as 'diff` runs on this
file will not flag this as a difference anymore.

In MDEV-28640 the init script failed to stop/start the MariaDB server
due to missing mysqladmin on the system. This was however very hard to
spot from the console output.

Add an explicit check for the binary the script depends on, and fail
verbosely if the dependency is missing.

See https://github.com/MariaDB/server/commit/f4adf3547420cced027d26ee8b8190be8a2bdea2.patch

@LinuxJedi
Copy link
Contributor

@ottok I don't see any indication in this PR that the intention was to deviate from standard for PRs (squash merges) and apply the commits without a merge outside of GH. Please indicate this is your intention in future and we can facilitate this.

@ottok
Copy link
Contributor Author

ottok commented Nov 15, 2022 via email

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.

6 participants