Bug 214807 - New port: databases/mysql-wsrep56-server: MySQL version enabled for Galera Replication
Summary: New port: databases/mysql-wsrep56-server: MySQL version enabled for Galera Re...
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: amd64 Any
: --- Affects Some People
Assignee: Nikolai Lifanov
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-11-24 13:04 UTC by Philip Stoev
Modified: 2017-02-22 17:40 UTC (History)
4 users (show)

See Also:


Attachments
.shar file for mysql-wsrep56-server port (36.04 KB, text/plain)
2016-11-24 13:04 UTC, Philip Stoev
no flags Details
Updated .shar file (36.42 KB, text/plain)
2017-02-02 08:44 UTC, Philip Stoev
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Philip Stoev 2016-11-24 13:04:43 UTC
Created attachment 177353 [details]
.shar file for mysql-wsrep56-server port

This is my first port submission, so please bear with me.

The port is a version of MySQL that is enabled for Galera replication and is part of Galera Cluster, a multi-master clustering solution for MySQL that is used in OpenStack and elsewhere. The port is a fork of the mysql56-server port, but takes its source tarball from www.galeracluster.com and includes a modified rc.d script.

For more information: http://www.galeracluster.com

Thank you.
Comment 1 Nikolai Lifanov freebsd_committer 2017-02-01 15:31:57 UTC
take
Comment 2 Nikolai Lifanov freebsd_committer 2017-02-01 16:29:48 UTC
Hi Philip!

I poked at this problem before and in my opinion the best way forward is to create a series of slaves for existing MySQL ports with Codership patches applied (https://launchpad.net/codership-mysql). I'm sure Mahdi will help with conditional-izing the variables we need, like plist, version, etc.

The benefit of this approach is that we already have a USES=mysql framework that supports setting a default MySQL version (currently combination of 5.5, 5.6, 5.7, 8.0, 10.0, and 10.1 for some of MySQL, Percona Server, and MariaDB) and the WSREP-enabled submission will need to fit into this.

By slaving the ports, this avoids the matrix effects of exploding all of the options, enables binary package users to make use of the new WSREP-enabled versions, and makes it easier to add support for new server versions (for example, I can already see that patches for MySQL 5.7 are available).

This also enables us to make use of the heavy lifting and adaptation to FreeBSD that existing maintainers already perform.

I will have an initial proof of concept patch to you some time today.

Mahdi, I CC'd you on this, so feel free to chime in.

Thanks!
Comment 3 Mahdi Mokhtari freebsd_committer freebsd_triage 2017-02-01 17:23:03 UTC
(In reply to Nikolai Lifanov from comment #2)
Hi, Thanks for CC'ing me ;) I hope I can help in it.
I agree with conditionising approach, It's more compatible with `avoiding-bundled-libs` approach of FreeBSD IMO. (plus the advantages @lifanov pointed ;D)

MySQL in ports-tree is newer than the latest patched one on the codership's launchpad page.
I'll look at it, If the patch applies cleanly on the newer one, we easily can have slave ports.
Otherwise, I'll fix the patches for the new, keep it locally on our tree until they merge it on the upstream.

I'll notify you on each case ;)

Regards!
Comment 4 Philip Stoev 2017-02-01 17:24:20 UTC
Understood. By diffing between mysql-wsrep56-server and mysql56-server you can see  the customisations that I needed to make for Galera Cluster. I recall at least the following:
 - downloading source code from www.galeracluster.com rather than mysql.com
 - running cmake with a different set of options
 - init script is different
Comment 5 Philip Stoev 2017-02-01 17:25:06 UTC
(In reply to Mahdi Mokhtari from comment #3)

Note that the authoritative location of mysql-wsrep source code is www.galeracluster.com  . Any files found on www.launchpad.net are obsolete.
Comment 6 Nikolai Lifanov freebsd_committer 2017-02-01 17:32:13 UTC
To Mahdi,

Yes, the versions will need to diverge now and in the future. That's why PORTVERSION will need a ?= on it. Check sysutils/ansible and sysutils/ansible1 port history to see how I can apply master-specific and slave-specific patches to these. tl;dr: the approach is to put port-specific patches in EXTRA_PATCHES?= variable and just keep it an empty string on a master whenever these are not required.

Hi Philip,

The Launchpad site provides patches *to* the MySQL source code tree. Is there a way to obtain the same thing from the new www.galeracluster.com site? If not, that's not a big deal and we can add a different MASTER_SITES and distinfo files for it.
Comment 7 Mahdi Mokhtari freebsd_committer freebsd_triage 2017-02-01 18:03:24 UTC
(In reply to Nikolai Lifanov from comment #6)
> Yes, the versions will need to diverge now and in the future. That's why PORTVERSION will need a ?= on it.
Yeah I see, the sample of ansible was exactly what I thought when I was writing the comment :D
But in little different context,
I meant that, I think we need to use the patches of launchpad to avoid fetching mysql source again from different place (to avoid using bundled mysqls and also being able to correctly use USES=mysql$VER)
But your point about EXTRA_PATCHES are totally valid :)
Comment 8 Mahdi Mokhtari freebsd_committer freebsd_triage 2017-02-01 18:05:11 UTC
(In reply to Philip Stoev from comment #5)
So, can we have newer (in-obsolete) patches from www.galeracluster.com instead of launchpad ?
This will help a lot IMO.
Comment 9 Nikolai Lifanov freebsd_committer 2017-02-01 18:12:06 UTC
I would also prefer that www.galeracluster.com publishes a separate patchset, but it's not a blocker for me if this doesn't happen. There are support concerns for Codership customers if they end up using a non-validated version of MySQL + patches. For example, Percona XtraDB Cluster ships as a single tarball with the WSREP patchset applied and won't entertain splitting for this reason. In my opinion, this is mitigated by allowing for PORTVERSION to be different.

Philip, is it possible to publish the official patchset on www.galeracluster.com?
Comment 10 Mahdi Mokhtari freebsd_committer freebsd_triage 2017-02-01 18:22:48 UTC
(In reply to Nikolai Lifanov from comment #9)
Aha, I see your points.
Makes sense for me too.

So you mean by changing the PORTVERSION (and setting it via `?=`), It'd be Okay to fetch the source package from another MASTER than MySQL's (like www.galeracluster.com)? 

Also what would be the point of using USES=mysql$VER in this case? we can ignore using it too? :)
Comment 11 Philip Stoev 2017-02-01 19:04:13 UTC
Unfortunately we are not set up to provide a patchset right now. We provide a complete patched source code tarball:

http://releases.galeracluster.com/source/mysql-wsrep-5.6.35-25.19.tar.gz
Comment 12 Nikolai Lifanov freebsd_committer 2017-02-01 19:06:56 UTC
OK, in this case it may not make sense to slave these, so we can proceed with reviewing this submission. I would like to defer to Mahdi's judgement for what makes sense here.
Comment 13 Mahdi Mokhtari freebsd_committer freebsd_triage 2017-02-01 19:10:48 UTC
(In reply to Nikolai Lifanov from comment #12)
Thanks :)
I guess It's nice to have the port sooner (cause it provides tempting features for people)
Do you think we can change it to slave port when they provided patches after the port went on the tree?
Comment 14 Nikolai Lifanov freebsd_committer 2017-02-01 19:16:30 UTC
Yes, this plan sounds reasonable. I'll make a first pass over the submission and we can move on to the review.

Meanwhile, some bikeshed topics:

1) How would we like to hook it up to USES=mysql?
    - I propose 56g or 56w

2) How do we name it consistently?
    - I propose databases/mysql56-server-wsrep or something
Comment 15 Philip Stoev 2017-02-01 19:44:41 UTC
mysql56-server-wsrep is fine as well, however, just for the record, our packages on Linux have all historically had "mysql-wsrep" together as part of their name , such as:

mysql-wsrep-server-5.6_5.6.35-25.19-1precise_amd64.deb
Comment 16 Mahdi Mokhtari freebsd_committer freebsd_triage 2017-02-01 20:00:38 UTC
(In reply to Nikolai Lifanov from comment #14)
> 1) How would we like to hook it up to USES=mysql?
>     - I propose 56g or 56w
I'd suggest 56w :) cause consists of the name of the `package`, but if you like the other I'll be fine with it, each one has its rational behind. 

> 2) How do we name it consistently?
>     - I propose databases/mysql56-server-wsrep or something
Because of the consistency of mysql-related ports (percona for example), I'd suggest to keep the $NAME$VERSION-server format.
It may be galeracluster56-server or mysql56-wsrep-server.
The second would also be compatible with what Philip suggests.
Comment 17 Mahdi Mokhtari freebsd_committer freebsd_triage 2017-02-01 20:03:51 UTC
(In reply to Mahdi Mokhtari from comment #16)
Oh sorry I had a typo on my second suggest.
It was mysqlwsrep56-server
:)
Comment 18 Nikolai Lifanov freebsd_committer 2017-02-01 23:29:13 UTC
I started testing it and I can't actually fetch it:
=> Attempting to fetch http://releases.galeracluster.com/source/mysql-wsrep-5.6.34-25.18.tar.gz
fetch: http://releases.galeracluster.com/source/mysql-wsrep-5.6.34-25.18.tar.gz: Not Found
Comment 19 Nikolai Lifanov freebsd_committer 2017-02-01 23:34:38 UTC
I rerolled this, and there are a bunch of warnings:

$ portlint -A
WARN: /usr/home/lifanov/src/svn/freebsd/ports/head/databases/mysql-wsrep56-server/pkg-plist: [35]: installing shared libraries, please define USE_LDCONFIG as appropriate
WARN: /usr/home/lifanov/src/svn/freebsd/ports/head/databases/mysql-wsrep56-server/pkg-plist: [46]: installing shared libraries, please define USE_LDCONFIG as appropriate
FATAL: Makefile: [9]: use a tab (not space) after a variable name
FATAL: Makefile: [1]: incorrect header; use Created by: with a single space, then $FreeBSD$.
WARN: Makefile: COMMENT is not supposed to begin with 'A ', 'An ', or 'The '.
WARN: Makefile: possible use of absolute pathname "/var/db/mysql".
FATAL: Makefile: non-comment line in comment section.
FATAL: Makefile: no $FreeBSD$ line in comment section.
FATAL: Makefile: extra item "MAINTAINER" placed in the PORTNAME section.
FATAL: Makefile: extra item "COMMENT" placed in the PORTNAME section.
FATAL: Makefile: PORTNAME has to be there.
WARN: Makefile: PORTVERSION/DISTVERSION is set externally to this port's Makefile, but this port is not configured as a slave port.
WARN: Makefile: CATEGORIES is set externally to this port's Makefile, but this port is not configured as a slave port.
FATAL: Makefile: extra item "BROKEN_powerpc64" placed in the MAINTAINER section.
WARN: Makefile: COMMENT is set externally to this port's Makefile, but this port is not configured as a slave port.
WARN: Makefile: Consider defining LICENSE.
WARN: Makefile: "RUN_DEPENDS" has to appear earlier.

It's a good idea to run "portlint -A" and fix all or most of things for new ports.
Comment 20 Nikolai Lifanov freebsd_committer 2017-02-01 23:36:22 UTC
Can you update this to a new version (mysql-wsrep-5.6.35-25.19), reroll patches with "make makepatch" (except for armv6 one), and satisfy "portlint -A" please?

After this, please attach the updated version and we can proceed.
Comment 21 Philip Stoev 2017-02-02 08:44:55 UTC
Created attachment 179521 [details]
Updated .shar file

Please find attached the updated .shar file:
- the package has been renamed to mysqlwsrep56-server
- portlint now reports the following:

WARN: Makefile: possible use of absolute pathname "/var/db/mysql".
WARN: Makefile: unless this is a master port, PORTNAME has to be set by "=", not by "?=".
WARN: Makefile: unless this is a master port, COMMENT has to be set by "=", not by "?=".
0 fatal errors and 3 warnings found.

Those warnings also occur with the mysql56-server package.

- make makepatch was run.
Comment 22 Philip Stoev 2017-02-02 08:47:17 UTC
I am not sure about the USE_MYSQL= directive, so I did not change it. No matter what value I try, I get:

===>  mysqlwsrep56-server-5.6.35 cannot install: unknown MySQL version:
mysql:56w.
*** Error code 1
Comment 23 Nikolai Lifanov freebsd_committer 2017-02-02 14:16:25 UTC
Thanks Philip,

The 56w thing will require a framework update.
I'll take care of it when I create the review.
Comment 24 Nikolai Lifanov freebsd_committer 2017-02-03 15:59:46 UTC
Hi Philip,

I made a couple of minor changes to the port and submitted it for review.
Let's move there: https://reviews.freebsd.org/D9427
Comment 25 Nikolai Lifanov freebsd_committer 2017-02-03 16:36:13 UTC
It's a good idea that you grab the review and answer questions, submit changes, etc.

If you never used Phabricator in the context of FreeBSD project, this should be helpful: https://wiki.freebsd.org/Phabricator
Comment 26 commit-hook freebsd_committer 2017-02-22 17:32:21 UTC
A commit references this bug:

Author: lifanov
Date: Wed Feb 22 17:31:42 UTC 2017
New revision: 434624
URL: https://svnweb.freebsd.org/changeset/ports/434624

Log:
  add databases/mysqlwsrep56-server

  - MySQL version enabled for Galera Replication

  PR:		214807
  Submitted by:	Philip Stoev <philip.stoev@galeracluster.com>
  Reviewed by:	matthew, mat, mmokhi
  Approved by:	portmgr (mat)
  Differential Revision:	https://reviews.freebsd.org/D9427

Changes:
  head/Mk/Uses/mysql.mk
  head/Mk/bsd.default-versions.mk
  head/databases/Makefile
  head/databases/mysqlwsrep56-server/
  head/databases/mysqlwsrep56-server/Makefile
  head/databases/mysqlwsrep56-server/distinfo
  head/databases/mysqlwsrep56-server/files/mysql-server.in
  head/databases/mysqlwsrep56-server/files/patch-CMakeLists.txt
  head/databases/mysqlwsrep56-server/files/patch-client_CMakeLists.txt
  head/databases/mysqlwsrep56-server/files/patch-cmake_plugin.cmake
  head/databases/mysqlwsrep56-server/files/patch-cmake_ssl.cmake
  head/databases/mysqlwsrep56-server/files/patch-extra_CMakeLists.txt
  head/databases/mysqlwsrep56-server/files/patch-include_CMakeLists.txt
  head/databases/mysqlwsrep56-server/files/patch-include_my__compare.h
  head/databases/mysqlwsrep56-server/files/patch-include_my_compare.h
  head/databases/mysqlwsrep56-server/files/patch-include_myisam.h
  head/databases/mysqlwsrep56-server/files/patch-libmysql_CMakeLists.txt
  head/databases/mysqlwsrep56-server/files/patch-libservices_CMakeLists.txt
  head/databases/mysqlwsrep56-server/files/patch-man_CMakeLists.txt
  head/databases/mysqlwsrep56-server/files/patch-mysys__ssl_my__default.cc
  head/databases/mysqlwsrep56-server/files/patch-mysys_ssl_my_default.cc
  head/databases/mysqlwsrep56-server/files/patch-plugin_password__validation_validate__password.cc
  head/databases/mysqlwsrep56-server/files/patch-plugin_password_validation_validate_password.cc
  head/databases/mysqlwsrep56-server/files/patch-scripts_CMakeLists.txt
  head/databases/mysqlwsrep56-server/files/patch-scripts_mysqld__safe.sh
  head/databases/mysqlwsrep56-server/files/patch-scripts_mysqld_safe.sh
  head/databases/mysqlwsrep56-server/files/patch-sql_CMakeLists.txt
  head/databases/mysqlwsrep56-server/files/patch-sql_sql__trigger.cc
  head/databases/mysqlwsrep56-server/files/patch-sql_sql__view.cc
  head/databases/mysqlwsrep56-server/files/patch-sql_sql_trigger.cc
  head/databases/mysqlwsrep56-server/files/patch-sql_sql_view.cc
  head/databases/mysqlwsrep56-server/files/patch-sql_sys__vars.cc
  head/databases/mysqlwsrep56-server/files/patch-sql_sys_vars.cc
  head/databases/mysqlwsrep56-server/files/patch-support-files_CMakeLists.txt
  head/databases/mysqlwsrep56-server/pkg-descr
  head/databases/mysqlwsrep56-server/pkg-message
  head/databases/mysqlwsrep56-server/pkg-plist
Comment 27 Nikolai Lifanov freebsd_committer 2017-02-22 17:40:15 UTC
Hi Philip, this landed with some changes. Great job!