Bug #57023 | innobackupC(mysqlbackup) C API doesn't read cnf file during mysqld connection | ||
---|---|---|---|
Submitted: | 25 Sep 2010 20:35 | Modified: | 28 Jan 2011 19:14 |
Reporter: | Ritheesh Vedire | Email Updates: | |
Status: | Closed | Impact on me: | |
Category: | MySQL Enterprise Backup | Severity: | S3 (Non-critical) |
Version: | OS: | Any | |
Assigned to: | Ritheesh Vedire | CPU Architecture: | Any |
[25 Sep 2010 20:35]
Ritheesh Vedire
[27 Sep 2010 16:51]
Ritheesh Vedire
Submitted an 'experimental' patch for now.
[28 Sep 2010 10:50]
Ingo Strüwing
Please find comments by email.
[18 Oct 2010 10:50]
Ritheesh Vedire
Yes, the program should read cnf files. It is a very important FR.
[19 Oct 2010 13:00]
Ritheesh Vedire
mysqlbackup has only four client related options, which are user, password, port and socket. (host is local always). but a user might want to extra options how a connection to the server should be made. She will specify the options if any under [client] group. The mysqlbackup program should read essentially the options under [mysqlbackup] and [client] group.
[20 Oct 2010 9:26]
Ingo Strüwing
I do not attempt to revert a decision. But I want to document my concerns. I strongly would prefer to have [1] (a) implemented instead of [2] (b). For the reasoning, let's assume, we implement [1] (a). In that case, I can state the following: If the customer has a "clean" MySQL setup, he uses CNF files at the standard locations as documented in the MySQL reference manual, chapter "MySQL Programs :: Using MySQL Programs :: Specifying Program Options :: Using Option Files". In that case, the Command Line CNF file (CLCNF) is one of the standard CNF files and it is examined for [client] and [mysqlbackup] options by mysqlbackup implicitly. In so far the "intuitivity expectation" is fulfilled. Or the customer is used to the --defaults-file and/or --defaults-extra-file options. He specifies CLCNF twice on the command line, first as a value to --defaults-file or --defaults-extra-file and then as an argument. Though it looks ugly, it shouldn't confuse one, who always uses the explicit options. One day we might find a feature request to drop the command line argument as we can read the required backup options from standard option files or from an explicit defaults file or from explicit command line options. If the user has an uncommon setup, where CLCNF is not in a standard location, and not given to --defaults-file or --defaults-extra-file, then the file is not really the "server CNF file". It is not the file that was used to start the MySQL server. The server would recognize it only at standard locations or with --defaults-file or --defaults-extra-file. This means that the customer takes the dangerous route to duplicate the backup-relevant options in a separate CNF file for backup purposes. In that case we should be willing to accept that such customer does need to use --defaults-file or --defaults-extra-file if he also wants to grab the connection options from that file. Now, let's assume, we implement [2] (b). Mysqlbackup will behave differently from innobackup(Perl). - It will not read options from standards options files, while innobackup(Perl) calls 'mysql', which does read from standards options files. - If the customer has a "clean" MySQL setup, CLCNF will be the real server CNF file, which might not necessarily contain [client] options as these could be in another CNF in another standard place. Hence, we would force a "clean" customer to "uglify" his setup to please mysqlbackup requirements. (Uglify by copying [client] options the the server CNF, or by always having explicit mysqlbackup command line options.) Implementation considerations: As Ritheesh did already rightfully point out, it would be a hack to try to combine defaults file options with CLCNF. Things are even worse than in his analysis. To safely identify the CLCNF argument, one would need to do a full parse of the command line. Then one needs to construct a new command line, except if a --no-defaults-file, --defaults-file or --defaults-extra-file option is already present. But since load_defaults() removes them from the command line, they are invisible to handle_options(). To detect the presence of these options, we would need to examine the command line "manually" (Ugh. What a hack. :-/). Now compare that with the probability that CLCNF is a standard option file anyway, as explained above. For (a), I suggest to copy all initializers for my_long_options[] from mysql.cc, which are related to connection establishment. The most important ones are mentioned in the MySQL reference manual, chapters "MySQL Programs :: Using MySQL Programs :: Connecting to the MySQL Server" and "MySQL Server Administration :: MySQL User Account Management :: Using SSL for Secure Connections :: SSL Command Options". However, at least the first (non-ssl) list is not complete. More connection relevant options exist. --compress is one example. Since seemingly all MySQL client programs know that option, it can be found in a [client] group, and hence, mysqlbackup must recognize it (though not necessarily act on it). The exact set of options that can appear in a [client] group seem to be difficult to evaluate exactly. I guess it would be better to have one option too much than to forget one as the program would fail on such CNF with "Bad command line arguments".
[20 Oct 2010 12:18]
Ritheesh Vedire
@Thava: I think introducing new options such as --defaults-extra-file, --defaults-file, etc will confirm to other client programs such as mysqladmin etc. In the long run we have to introduce certain client specific options for mysqlbackup such as --connect-timeout etc.
[20 Oct 2010 13:47]
Ingo Strüwing
Option (i) would not fulfil the decision of 21 Sep. Option (ii) looks neat. I hope that load_defaults() does not use global variables, which let the second run fail. But Thava's idea, how to locate the CLCNF argument may not work in all cases. Options like --socket, which do not have any default value, can be written with '=' *or* space between the option name and its value. If we have a command line ending in --socket /home/my/my.sock /home/my/my.cnf then Thava's approach would fail. I stick with my claim that we need to do a full parse run of the original command line to safely identify the CLCNF argument.
[21 Oct 2010 7:21]
Ritheesh Vedire
multiple calls to load_defaults() works as expected, i.e we get the new command line constructed from the options. As for the parsing algorithm goes, What Thava mentioned was this: <CLCNF> will not start with "--" characters. Also it is not possible to create files with first two characters being --. So, we check the second last argument and if it is not starting with "--" we take it as the CLCNF file. Even if the above procedure fails, also, I realized that it may not be a big deal to parse the whole cmd line to get the CLCNF file. A "virtual" call to handle_options() should do that.
[21 Oct 2010 7:43]
Ritheesh Vedire
oops! I'm sorry. I retract my words for using "--" as an identifier for CLCNF file. mysqlbackup --socket ~/hello/my.sock ~/my.cnf will end up with "~/hello/my.sock" configuration file not found. Initially I thought handle_options() would parse only --option=<value> where '=' is the *only* separator. but then I'm wrong. we can argue that mysqlbackup help text has only '=' as the separator and the printed error message above is perfectly fine. :)
[21 Oct 2010 9:03]
Ingo Strüwing
Ritheesh wrote: "... it is not possible to create files with first two characters being --." Hehe. I didn't even think of the nasty idea to start config files with --. But then I have to say that the above claim is not true. Programs that understand --options, by convention do also understand a standalone -- as an "end of options" mark. So the following is possible: bash $ echo mytest1-contents > --mytest1-file $ ls -- --* --mytest1-file $ cat -- --mytest1-file mytest1-contents $ rm -- --mytest1-file $ ls -- --* ls: cannot access --*: No such file or directory
[26 Oct 2010 10:03]
Ritheesh Vedire
By mistake, I set the status as "patch queued". It should be "patch pending". Setting the correct state now.
[29 Oct 2010 7:58]
Ingo Strüwing
Changes required. Please see email.
[11 Nov 2010 14:26]
Ingo Strüwing
Approved pending test failure fixes. Find minor comments by email.
[19 Nov 2010 13:38]
Thava Alagu
Changes look OK. Approved.