fix: Restore pre-Tutor 16 behavior, disable the MySQL binlog by default

[fghaas]

Up to Tutor 15, when Tutor was configured with RUN_MYSQL: true, it ran MySQL 5.7, which disabled binary logging ("the binlog") by default unless enabled with the --log-bin option.

In Tutor 16, we switched to MySQL 8 which flipped this default: now, the binlog is enabled by default, unless disabled with --skip-log-bin or --disable-log-bin.

The binlog is relevant for two purposes:

1. MySQL replication.
2. Point-in-time restore (that is, incrementally rolling forward from the most recently restored full backup, using binlog data).

However, in case neither of those purposes are relevant to the Tutor deployment at hand (and it stands to reason that in most deployments they are not), enabling the binlog comes with disadvantages:

• In busy Open edX databases, the binlog files can grow to considerable size, particularly considering the default expiry period for the binlog is a whopping 30 days.
• This can cause a MySQL volume to fill up and then disable the database altogether, because it runs into ENOSPC ("no space left on device") errors.
• This is especially true in tutor k8s configurations, where the MySQL volume size is just 5 GiB by default.

Thus, restore the pre-Tutor 16 behavior of running without the binlog by default, and give users the opportunity to enable it by setting MYSQL_ENABLE_BINLOG: true.

Reference:
https://dev.mysql.com/doc/refman/8.0/en/replication-options-binary-log.html#option_mysqld_log-bin

[fghaas]

@regisb Please don't consider this for merging yet as I still need to do a decent amount of testing, but can you let me know if you agree with the general rationale here?

I think it would also be okay for the config option to be true by default; it's basically a toss-up between not surprising existing Tutor users coming from MySQL 5.7 who would reasonably expect the binlog to be off by default, and not surprising new Tutor users who have used MySQL 8 elsewhere who would reasonably expect it to be on by default.

But I do think it should be configurable.

[regisb]

Thanks for the detailed explanations Florian. Yes, I agree with the rationale. It makes sense not to create an infinitely growing log file by default.

I'd rather avoid creating a setting just for this though -- but you knew I would say this, right? ;) Then again, I agree that users should be able to configure this parameter. Is it sufficient if plugins can override the command of the "mysql" container?

I'm grateful for your testing. In particular, I'd like to know what's going to happen to the binlog file for current tutor v16 users who upgrade to the next version, where the binglog file will be disabled.

[fghaas]

Is it sufficient if plugins can override the command of the "mysql" container?

Well you're already saying you're going to make something configurable. :) If you object to a separate option for the binlog, then I'd suggest you also reconsider the hard-coded character set fix from #890/#896, and introduce an option named MYSQLD_ARGS or similar. That way, people could override the character set, disable binary logging, etc. as they please — difficulty: people then also need to fix tutor/commands/jobs.py for the mysql init job so that the client's character set doesn't go out of whack with the server's.

But I don't see how it makes sense to hard-code some mysqld options, and then also have a way to override any other. There be dragons.

So, if Tutor is meant to decide which options should be set (i.e. which mysqld defaults should be overridden) and which shouldn't, then I think the approach of having a separate toggle for the binlog makes the most sense.

In particular, I'd like to know what's going to happen to the binlog file for current tutor v16 users who upgrade to the next version, where the binglog file will be disabled.

When mysqld restarts, it will no longer be capable of acting as a replication master, nor will it be able to do point-in-time restore. To the best of my knowledge, pre-existing binlog files will remain in place, being somewhat useless, and will need to be removed manually.

[fghaas]

Summarizing test results.

Starting mysqld with --disable-log-bin means that

• no more binlog.NNNNNN files appear in /var/lib/mysql from the moment the container/pod restarts;
• the files remain in that directory, and must subsequently be deleted manually from there;
• the PURGE BINARY LOGS SQL statement, issued in a mysql -u root client session, has no effect (though it also does not raise an error).

@regisb, have you made up your mind on which way the default should go, and/or what to do about mysqld options in general?

[regisb]

@regisb, have you made up your mind on which way the default should go, and/or what to do about mysqld options in general?

I suggest we go with a file-based my.cnf configuration, similar to uwsgi.ini. This my.cnf file should include a {{ patch("mysql-settings") }} statement such that plugin developers can easily add their custom configuration there.

Would such a solution work for you?

[fghaas]

Would such a solution work for you?

It would work but I'd strongly dislike it, to be frank.

[fghaas]

Meanwhile I've taken the liberty to rebase this on 16.1.4.

Also, @mrtmm kindly gave this another pair of eyes, and a test run in a dev environment.

[regisb]

It would work but I'd strongly dislike it, to be frank.

Why is that? Is it because it would require creating a plugin?

[fghaas]

Why is that? Is it because it would require creating a plugin?

Just for the same reason where we already know that we respectfully disagree with each other. :) It's because rather than curbing the growth of complexity, it only shifts complexity from the developer to the user. I've recently counted, and we carry 25 plugins on our production platform, out of which more than half do not add any functionality, but only modify configuration settings.

I think it's just as fair for you as the upstream developer to say "I don't want more config options, for they add complexity and increase my maintenance burden" as it is for me as a downstream user to say "I don't want more plugins, for they add complexity and increase my maintenance burden". :) But like I said, there's no real way to resolve that conundrum, so it's always a compromise.

However, I just really don't understand the logic of adding a facility that drops a my.cnf file in view of #890/#896. Suppose the user now wants to run with a different default character encoding and they assume that they can drop those settings into the proposed mysql-settings patch. Then they're in for a rough landing, because the currently hard-coded CLI options override what's in my.cnf.

So I think you can pick either one or the other, that is, either trying to be clever about mysqld options (which is what Tutor currently does, and what this PR currently expands on) or throwing your hands up and saying "I don't care, do what you want" (which is what a proposed my.cnf injection would be, if it coincides with simultaneously dropping the hard-coded mysqld command-line options). But doing a mix of the two doesn't make sense to me at all, and leads to all manner of unpleasant surprises. Am I making sense?

[fghaas]

I've converted this back to a draft since running mysqld with the --disable-log-bin option apparently makes the tutor-contrib-backup plugin break with the following error:

mysqldump: Got error: 2061: "RSA Encryption not supported - caching_sha2_password plugin was built with GnuTLS support" when trying to connect

Why it would break with precisely that error I have no idea, but the problem started popping up when adding --disable-log-bin, and then went away after dropping that option.

[fghaas]

PR #926 (also a Draft right now, pending further testing) may be an alternate approach here that does without a new config option.

[regisb]

I think it's just as fair for you as the upstream developer to say "I don't want more config options, for they add complexity and increase my maintenance burden" as it is for me as a downstream user to say "I don't want more plugins, for they add complexity and increase my maintenance burden". :) But like I said, there's no real way to resolve that conundrum, so it's always a compromise.

@fghaas this part of your comment really struck me. Yes, 25 plugins are too much. You are without a doubt a power user of Tutor! And it's crucial that Tutor works well for power users, because all users hope to become power users.

I'm wondering what fundamental distinction you make between plugins and configuration settings. Are configuration settings easier to modify because you can just run tutor config save and not have to create/store/version-control a separate plugin? If that's the case, I'm sure we can find a middle ground where we make it easier to implement simple plugins.

But doing a mix of the two doesn't make sense to me at all, and leads to all manner of unpleasant surprises. Am I making sense?

Yes, sorry about my confusing comment. I do want to move all current mysqld options to my.cnf, just like for uwsgi. It would be very confusing to have two sources of configuration (CLI options and my.cnf).

[fghaas]

Why it would break with precisely that error I have no idea, but the problem started popping up when adding --disable-log-bin, and then went away after dropping that option.

Just to update this part: this was probably unrelated. Rather than being due to --disable-bin-log, it was likely a client incompatibility between the backup plugin's mysqldump client and the 8.0 server version. We believe this is fixed with hastexo/tutor-contrib-backup#69 and the 3.0.0 release of that plugin.

[fghaas]

Having come to the conclusion that #926 is the better option, and having tested the 3-day binlog to promising results, I am closing this PR and suggest we go ahead with #926 instead.