MDEV-32884 Improve S3 options comaptibility

The previous commit for MDEV-32884 fixed the s3_protocol_version option, which was previous only using "Auto", no matter what it was set to. This patch does several things to keep the old behaviour whilst correcting for new behaviour and laying the groundwork for the future. This includes: * `Original` now means v2 protocol, which it would have been due to the option not working, so upgrades will stil work. * A new `Legacy` option has been added to mean v1 protocol. * Options `Path` and `Domain` have been added, these will be the only two options apart from `Auto` in a future release, and are more aligned with what this variable means. * Fixed the s3.debug test so that it works with v2 protocol. * Fixed the s3.amazon test so that it works with region subdomains. * Added additional modes to the s3.amazon test. * Added s3.not_amazon test for the remaining modes. This replaces PR #2902.
MariaDB · Dec 7, 2023 · bc5e904 · bc5e904
1 parent ecbdd72
commit bc5e904
Show file tree

Hide file tree

Showing 7 changed files with 70 additions and 10 deletions.
diff --git a/mysql-test/suite/s3/amazon.result b/mysql-test/suite/s3/amazon.result
@@ -4,4 +4,14 @@ create table t1 (pk int primary key, a int);
 insert into t1 values (1,1),(2,2),(3,3),(4,4);
 alter table t1 engine=S3;
 drop table t1;
+set @@global.s3_protocol_version="Amazon";
+create table t1 (pk int primary key, a int);
+insert into t1 values (1,1),(2,2),(3,3),(4,4);
+alter table t1 engine=S3;
+drop table t1;
+set @@global.s3_protocol_version="Domain";
+create table t1 (pk int primary key, a int);
+insert into t1 values (1,1),(2,2),(3,3),(4,4);
+alter table t1 engine=S3;
+drop table t1;
 set @@global.s3_protocol_version=@save_s3_protocol_version;
diff --git a/mysql-test/suite/s3/amazon.test b/mysql-test/suite/s3/amazon.test
@@ -1,6 +1,6 @@
 --source include/have_s3.inc
 
-if (`SELECT @@s3_host_name <> "s3.amazonaws.com"`)
+if (`SELECT @@s3_host_name NOT LIKE "%.amazonaws.com"`)
 {
   skip Not connected to AWS;
 }
@@ -20,6 +20,22 @@ insert into t1 values (1,1),(2,2),(3,3),(4,4);
 alter table t1 engine=S3;
 drop table t1;
 
+set @@global.s3_protocol_version="Amazon";
+
+create table t1 (pk int primary key, a int);
+insert into t1 values (1,1),(2,2),(3,3),(4,4);
+--replace_result $database database
+alter table t1 engine=S3;
+drop table t1;
+
+set @@global.s3_protocol_version="Domain";
+
+create table t1 (pk int primary key, a int);
+insert into t1 values (1,1),(2,2),(3,3),(4,4);
+--replace_result $database database
+alter table t1 engine=S3;
+drop table t1;
+
 #
 # clean up
 #

diff --git a/mysql-test/suite/s3/debug.result b/mysql-test/suite/s3/debug.result
@@ -18,15 +18,15 @@ select count(*) from t1;
 count(*)
 100
 flush table t1;
-NOT FOUND /storage-engine/s3_test_/ in mysqld.1.err
+NOT FOUND /s3_test_/ in mysqld.1.err
 set @@global.s3_debug=1;
 select count(*) from t1;
 count(*)
 100
 set @@global.s3_debug=0;
-FOUND 6 /storage-engine/s3_test_/ in mysqld.1.err
+FOUND 6 /s3_test_/ in mysqld.1.err
 select count(*) from t1;
 count(*)
 100
 drop table t1;
-FOUND 6 /storage-engine/s3_test_/ in mysqld.1.err
+FOUND 6 /s3_test_/ in mysqld.1.err
diff --git a/mysql-test/suite/s3/debug.test b/mysql-test/suite/s3/debug.test
@@ -21,7 +21,7 @@ select count(*) from t1;
 flush table t1;
 
 --let SEARCH_FILE=$MYSQLTEST_VARDIR/log/mysqld.1.err
---let SEARCH_PATTERN=storage-engine/s3_test_
+--let SEARCH_PATTERN=s3_test_
 --source include/search_pattern_in_file.inc
 set @@global.s3_debug=1;
 select count(*) from t1;

diff --git a/storage/maria/aria_s3_copy.cc b/storage/maria/aria_s3_copy.cc
@@ -87,7 +87,9 @@ static struct my_option my_long_options[] =
    &opt_block_size, &opt_block_size, 0, GET_ULONG, REQUIRED_ARG,
    4*1024*1024, 64*1024, 16*1024*1024, MALLOC_OVERHEAD, 1024, 0 },
   {"s3_protocol_version", 'L',
-   "Protocol used to communication with S3. One of \"Auto\", \"Amazon\" or \"Original\".",
+   "Protocol used to communication with S3. One of \"Auto\", \"Legacy\", "
+   "\"Original\", \"Amazon\", \"Path\" or \"Domain\". "
+   "Note: \"Legacy\", \"Original\" and \"Amazon\" are deprecated.",
    &opt_protocol_version, &opt_protocol_version, &s3_protocol_typelib,
    GET_ENUM, REQUIRED_ARG, 0, 0, 0, 0, 0, 0},
   {"force", 'f', "Force copy even if target exists",
@@ -220,7 +222,20 @@ int main(int argc, char** argv)
 
   if (opt_protocol_version)
   {
-    uint8_t protocol_version= (uint8_t) opt_protocol_version;
+    uint8_t protocol_version;
+    switch (opt_protocol_version)
+    {
+      case 1: /* Legacy means v1 */
+      case 4: /* Path means v1 */
+        protocol_version= 1;
+        break;
+      case 2: /* Original means v2 */
+      case 3: /* Amazon means v2 */
+      case 5: /* Domain means v2 */
+        protocol_version= 2;
+        break;
+    }
+
     ms3_set_option(global_s3_client, MS3_OPT_FORCE_PROTOCOL_VERSION,
                    &protocol_version);
   }

diff --git a/storage/maria/ha_s3.cc b/storage/maria/ha_s3.cc
@@ -170,7 +170,10 @@ static MYSQL_SYSVAR_BOOL(replicate_alter_as_create_select,
 static MYSQL_SYSVAR_ENUM(protocol_version, s3_protocol_version,
                          PLUGIN_VAR_RQCMDARG,
                          "Protocol used to communication with S3. One of "
-                         "\"Auto\", \"Amazon\" or \"Original\".",
+                         "\"Auto\", \"Legacy\", \"Original\", \"Amazon\", "
+                         "\"Path\" or \"Domain\". "
+                         "Note: \"Legacy\", \"Original\" and \"Amazon\" are "
+                         "deprecated.",
                          NULL, NULL, 0, &s3_protocol_typelib);
 
 static MYSQL_SYSVAR_ULONG(pagecache_age_threshold,

diff --git a/storage/maria/s3_func.c b/storage/maria/s3_func.c
@@ -39,7 +39,7 @@ static int s3_read_file_from_disk(const char *filename, uchar **to,
 
 /* Used by ha_s3.cc and tools to define different protocol options */
 
-static const char *protocol_types[]= {"Auto", "Original", "Amazon", NullS};
+static const char *protocol_types[]= {"Auto", "Legacy", "Original", "Amazon", "Path", "Domain", NullS};
 TYPELIB s3_protocol_typelib= {array_elements(protocol_types)-1,"",
                               protocol_types, NULL};
 
@@ -155,8 +155,24 @@ ms3_st *s3_open_connection(S3_INFO *s3)
     my_errno= HA_ERR_NO_SUCH_TABLE;
   }
   if (s3->protocol_version)
+  {
+    uint8_t protocol_version;
+    switch (s3->protocol_version)
+    {
+      case 1: /* Legacy means v1 */
+      case 4: /* Path means v1 */
+        protocol_version= 1;
+        break;
+      case 2: /* Original means v2 */
+      case 3: /* Amazon means v2 */
+      case 5: /* Domain means v2 */
+        protocol_version= 2;
+        break;
+    }
+
     ms3_set_option(s3_client, MS3_OPT_FORCE_PROTOCOL_VERSION,
-                   &s3->protocol_version);
+                   &protocol_version);
+  }
   if (s3->port)
     ms3_set_option(s3_client, MS3_OPT_PORT_NUMBER, &s3->port);