osc: version refactor

Summary:
This change refactors the MySQL version parsing code to support FB-infra MySQL version strings better. In particular, it queries `@version_comment` now instead of `@version`, which has richer information such as the release version.

This allows us to make finer grained decisions about whether OSC can use a certain capability or not, such as checksum extensions or `DUMP TABLE`.

There is already a precedent for this, with things like `is_high_pri_ddl_supported` and `is_trigger_rbr_safe`.

Reviewed By: preritj24

Differential Revision: D60781614

fbshipit-source-id: 8dc062d18630494db4b59da27df56f5f8b69d9d5

core/lib/mysql_version.py

@@ -9,78 +9,74 @@
 LICENSE file in the root directory of this source tree.
 """
 
-FB_FORK_NAME = "fb"
+from typing_extensions import Self
 
 
 class MySQLVersion:
     def __init__(self, version_str):
-        self._version_str = version_str
-        self._version = ""
-        self._fork = ""
-        self._build = ""
-        self.parse_str()
-
-    def parse_str(self):
-        segments = self._version_str.split("-")
-        self._version = segments[0]
-        # It's possible for a version string to have no fork and build segment
-        if len(segments) > 1:
-            self._fork = segments[1]
-        if len(segments) > 2:
-            self._build = segments[2]
-
-    @property
-    def major(self):
         """
-        Major version is the first segment of a version string
+        @param version_str: Value of @@version_comment (not @@version).
+        Examples:
+        8.0.32-202407011440.prod
+        8.0.XX-YYYYMMDDHHmm.dev.alexbud
         """
-        return int(self._version.split(".")[0])
+
+        segments = version_str.split("-")
+        version_segments = segments[0].split(".")
+        self._major: int = int(version_segments[0])
+        self._minor: int = int(version_segments[1])
+        self._release: int = int(version_segments[2])
+
+        build_segments = segments[1].split(".")
+        self._build: str = build_segments[0]
+        self._prod: bool = build_segments[1] == "prod"
 
     @property
-    def minor(self):
+    def major(self) -> int:
         """
-        Minor version is the seconds segment of a version string
+        Major version is the first segment of a version string.
+        E.g. 8 in 8.0.32
         """
-        return int(self._version.split(".")[1])
+        return self._major
 
     @property
-    def release(self):
+    def minor(self) -> int:
         """
-        Release is the third segment of a version string
+        Minor version is the seconds segment of a version string.
+        E.g. 0 in 8.0.32
         """
-        return int(self._version.split(".")[2])
+        return self._minor
 
     @property
-    def build(self):
+    def release(self) -> int:
         """
-        If there's a build information it will be the last part the version
-        string
+        Release is the third segment of a version string
+        E.g. 32 in 8.0.32
         """
-        return self._build
+        return self._release
 
     @property
-    def fork(self):
+    def build(self) -> str:
         """
-        If the running MySQL is not a community version, it will have something
-        after the general version string separated by dash
+        The build number as a str. E.g. 202407011440 in 8.0.32-202407011440.prod
         """
-        return self._fork
+        return self._build
 
     @property
-    def is_fb(self):
+    def is_mysql8(self) -> bool:
         """
-        Return if current running MySQL is a Facebook fork
+        Return True if major version is 8.
         """
-        return self.fork == FB_FORK_NAME
+        return self.major == 8
 
     @property
-    def is_mysql8(self):
+    def is_prod(self) -> bool:
         """
-        Return if current running MySQL is mysql8
+        Return True if build is a prod build (not a dev build)
         """
-        return self.major == 8
+        return self._prod
 
-    def __gt__(self, other):
+    def __gt__(self, other: Self):
         if self.major > other.major:
             return True
         elif self.major == other.major:
@@ -89,57 +85,31 @@ def __gt__(self, other):
             elif self.minor == other.minor:
                 if self.release > other.release:
                     return True
+                elif self.release == other.release:
+                    if self.build > other.build:
+                        return True
+                    else:
+                        return False
                 else:
                     return False
             else:
                 return False
         else:
             return False
 
-    def __lt__(self, other):
-        if self.major < other.major:
-            return True
-        elif self.major == other.major:
-            if self.minor < other.minor:
-                return True
-            elif self.minor == other.minor:
-                if self.release < other.release:
-                    return True
-                else:
-                    return False
-            else:
-                return False
-        else:
-            return False
+    def __eq__(self, other: Self):
+        return (
+            self.major == other.major
+            and self.minor == other.minor
+            and self.release == other.release
+            and self.build == other.build
+        )
 
-    def __ge__(self, other):
-        if self.major > other.major:
-            return True
-        elif self.major == other.major:
-            if self.minor > other.minor:
-                return True
-            elif self.minor == other.minor:
-                if self.release >= other.release:
-                    return True
-                else:
-                    return False
-            else:
-                return False
-        else:
-            return False
+    def __lt__(self, other: Self):
+        return not self > other and not self == other
 
-    def __le__(self, other):
-        if self.major < other.major:
-            return True
-        elif self.major == other.major:
-            if self.minor < other.minor:
-                return True
-            elif self.minor == other.minor:
-                if self.release <= other.release:
-                    return True
-                else:
-                    return False
-            else:
-                return False
-        else:
-            return False
+    def __ge__(self, other: Self):
+        return not self < other
+
+    def __le__(self, other: Self):
+        return not self > other

core/lib/payload/base.py

@@ -60,7 +60,7 @@ def __init__(self, **kwargs):
         self.mysql_engine = kwargs.get("mysql_engine", None)
         self.sudo = kwargs.get("sudo", False)
         self.skip_named_lock = kwargs.get("skip_named_lock", False)
-        self.mysql_vars = {}
+        self.mysql_vars: dict[str, str] = {}
         self.is_slave_stopped_by_me = False
         self.num_finished_dbs = 0
         self._current_db = None
@@ -150,7 +150,7 @@ def init_mysql_version(self):
         """
         Parse the mysql_version string into a version object
         """
-        self.mysql_version = MySQLVersion(self.mysql_vars["version"])
+        self.mysql_version = MySQLVersion(self.mysql_vars["version_comment"])
 
     def check_replication_type(self):
         """
@@ -326,13 +326,9 @@ def is_high_pri_ddl_supported(self):
         Only fb-mysql supports having DDL killing blocking queries by
         setting high_priority_ddl=1
         """
-        if self.mysql_version.is_fb:
-            if self.mysql_version >= MySQLVersion("5.6.35"):
-                return True
-            else:
-                return False
-        else:
-            return False
+
+        # 8.0 migration is completely done. No need to check for 5.x
+        return True
 
     @property
     def get_block_no_pk_creation_variable(self):
@@ -346,8 +342,7 @@ def get_block_no_pk_creation_variable(self):
         if self.mysql_version.is_mysql8:
             return "sql_require_primary_key", "session", "session"
         else:
-            if self.mysql_version.is_fb:
-                return "block_create_no_primary_key", "session", "global"
+            return "block_create_no_primary_key", "session", "global"
 
         return None, None, None
 

core/lib/payload/copy.py

@@ -586,13 +586,7 @@ def is_trigger_rbr_safe(self):
         """
         # We only need to check this if RBR is enabled
         if self.mysql_vars["binlog_format"] == "ROW":
-            if self.mysql_version.is_fb:
-                if not self.is_var_enabled("sql_log_bin_triggers"):
-                    return True
-                else:
-                    return False
-            else:
-                return False
+            return not self.is_var_enabled("sql_log_bin_triggers")
         else:
             return True
 

core/tests/copy_payload.py

@@ -876,37 +876,26 @@ def test_skip_affected_rows_check(self):
         payload.replay_insert_row(row, 1)
 
     def test_is_rbr_safe_stmt(self):
-        # is_trigger_rbr_safe should always be True if STATEMENT binlog_format
+        # is_trigger_rbr_safe should be True if STATEMENT binlog_format
         # is being used
         payload = self.payload_setup()
         payload.mysql_vars["binlog_format"] = "STATEMENT"
-        payload.mysql_version = MySQLVersion("5.1.1")
         self.assertTrue(payload.is_trigger_rbr_safe)
 
-    def test_is_rbr_safe_row_fb(self):
-        # is_trigger_rbr_safe should always be True if Facebook version
-        # is being used and sql_log_bin_triggers is OFF
+    def test_is_rbr_safe_row(self):
+        # is_trigger_rbr_safe should be True if RBR is being used and
+        # sql_log_bin_triggers is OFF
         payload = self.payload_setup()
         payload.mysql_vars["binlog_format"] = "ROW"
         payload.mysql_vars["sql_log_bin_triggers"] = "OFF"
-        payload.mysql_version = MySQLVersion("5.1.1-fb")
         self.assertTrue(payload.is_trigger_rbr_safe)
 
-    def test_is_rbr_safe_row_fb_but_logs_on(self):
-        # is_trigger_rbr_safe should False if we are using a Facebook version
-        # but sql_log_bin_triggers is still enabled
+    def test_is_rbr_safe_row_but_logs_on(self):
+        # is_trigger_rbr_safe should return False if RBR is used and
+        # sql_log_bin_triggers is enabled
         payload = self.payload_setup()
         payload.mysql_vars["binlog_format"] = "ROW"
         payload.mysql_vars["sql_log_bin_triggers"] = "ON"
-        payload.mysql_version = MySQLVersion("5.1.1-fb")
-        self.assertFalse(payload.is_trigger_rbr_safe)
-
-    def test_is_rbr_safe_row_other_forks(self):
-        # is_trigger_rbr_safe should False if we are using a Facebook version
-        # but sql_log_bin_triggers is still enabled
-        payload = self.payload_setup()
-        payload.mysql_vars["binlog_format"] = "ROW"
-        payload.mysql_version = MySQLVersion("5.5.30-percona")
         self.assertFalse(payload.is_trigger_rbr_safe)
 
     def test_divide_changes_all_the_same_type(self):
@@ -1087,26 +1076,6 @@ def test_wait_for_slow_query_never_finish(self):
             payload.wait_until_slow_query_finish()
         self.assertEqual(err_context.exception.err_key, "LONG_RUNNING_TRX")
 
-    def test_high_pri_ddl_does_not_wait_for_slow_query(self):
-        payload = self.payload_setup()
-        payload.stop_slave_sql = Mock()
-        payload.ddl_guard = Mock()
-        payload.mysql_version = MySQLVersion("8.0.1-fb-1")
-        payload.get_conn = Mock()
-        payload.execute_sql = Mock()
-        payload.wait_until_slow_query_finish = Mock()
-        payload.create_triggers()
-        self.assertTrue(payload.is_high_pri_ddl_supported)
-        payload.wait_until_slow_query_finish.assert_not_called()
-
-        # If high pri ddl is not supported, we should call wait_until_slow_query_finish
-        payload.get_long_trx = Mock(return_value=False)
-        payload.mysql_version = MySQLVersion("8.0.1-test-1")
-        payload.wait_until_slow_query_finish = Mock(return_value=True)
-        self.assertFalse(payload.is_high_pri_ddl_supported)
-        payload.create_triggers()
-        payload.wait_until_slow_query_finish.assert_called_once()
-
     def test_auto_table_collation_population(self):
         payload = self.payload_setup()
         sql = """