add detect database variant; vary backup locking on that basis

[deitch]

Fixes #439

@ZetaMap can you validate this is what you were looking at?

• Percona: just do LOCK TABLES FOR BACKUP and done.
• Others: LOCK TABLES FOR BACKUP TABLE1 READ, TABLE2 READ, ...

In all cases, end with UNLOCK TABLES.

If I could find a sane way to test this, I would be happier. The existing tests will catch any regressions, but I would be happier with an explicit test of the new functionality.

[deitch]

FYI, I used multiple heuristics to check for the database type. We always can iterate on it.

[ZetaMap]

Mmmm I'm not sure of that. I will test this query because I think in my case, this will return nothing

[deitch]

I am not sure either. There is no one reliable heuristic for this, hence the "let's try comment", then "let's try engine for Maria", then "let's try plugins for percona".

In the overwhelming majority of cases, the comment will be good enough. As you wrote in the related issue, only those who actually compile and override comment will fail to detect with the first heuristic. The isolation of this all into variant() let's us iterate on this.

[ZetaMap]

I confirm, in my case, this return nothing.
Available plugins:

> SELECT plugin_name FROM information_schema.plugins;
+---------------------------------+
| plugin_name                     |
+---------------------------------+
| binlog                          |
| mysql_native_password           |
| sha256_password                 |
| caching_sha2_password           |
| sha2_cache_cleaner              |
| PERFORMANCE_SCHEMA              |
| CSV                             |
| MEMORY                          |
| InnoDB                          |
| INNODB_TRX                      |
| INNODB_CMP                      |
| INNODB_CMP_RESET                |
| INNODB_CMPMEM                   |
| INNODB_CMPMEM_RESET             |
| INNODB_CMP_PER_INDEX            |
| INNODB_CMP_PER_INDEX_RESET      |
| INNODB_BUFFER_PAGE              |
| INNODB_BUFFER_PAGE_LRU          |
| INNODB_BUFFER_POOL_STATS        |
| INNODB_TEMP_TABLE_INFO          |
| INNODB_METRICS                  |
| INNODB_FT_DEFAULT_STOPWORD      |
| INNODB_FT_DELETED               |
| INNODB_FT_BEING_DELETED         |
| INNODB_FT_CONFIG                |
| INNODB_FT_INDEX_CACHE           |
| INNODB_FT_INDEX_TABLE           |
| INNODB_TABLES                   |
| INNODB_TABLESTATS               |
| INNODB_INDEXES                  |
| INNODB_TABLESPACES              |
| INNODB_COLUMNS                  |
| INNODB_VIRTUAL                  |
| INNODB_CACHED_INDEXES           |
| INNODB_SESSION_TEMP_TABLESPACES |
| INNODB_TABLESPACES_ENCRYPTION   |
| INNODB_TABLESPACES_SCRUBBING    |
| INNODB_CHANGED_PAGES            |
| MyISAM                          |
| MRG_MYISAM                      |
| TempTable                       |
| ARCHIVE                         |
| BLACKHOLE                       |
| FEDERATED                       |
| ngram                           |
| mysqlx_cache_cleaner            |
| mysqlx                          |
+---------------------------------+```

[deitch]

I'm not surprised. There just isn't a single guaranteed clear way to do this.

With it in its own utility function, if you see improved heuristics, just open a PR for it.

[deitch]

If I understood this correctly, current state is:

• Will correctly use LOCK TABLES FOR BACKUP with Percona, no change for anything else.
• Relies on a single function, variants() to determine what variant we are running, allowing isolation of all of the heuristics in one place.
• variants() will work for the vast majority of cases using comments, failing only if someone compiled independently and overrode the comment, and does not have percona* plugins

That sounds like an improvement with no downside.

I will let this sit for a little bit today, but unless that is in error, I will merge it in. We then can try and improve the heuristics independently.

I also like the idea of controlling the backup table locking mechanism via config. We can add that.

[deitch]

OK, now I am happy with it. Variant detection moved into its package (pkg/util/database), and an explicit test was created for it. Once this is green, we will merge it in, then we can iterate on the detection.