Update per reviews as of 10am PT 21 April, except DocString revisions.

jcjones · jcjones · commit 5e09ae259be8 · 2021-04-22T10:37:36.000-07:00
diff --git a/README.md b/README.md
@@ -14,7 +14,43 @@ Similar tools:
 # Usage
 
 ```sh
- → pip install --editable .
+ → git clone https://github.com/letsencrypt/mariadb-sequential-partition-manager-py.git
+ → cd mariadb-sequential-partition-manager-py
+ → python3 -m venv .venv
+ → . .venv/bin/activate
+ → python3 -m pip install .
+ → tee /tmp/partman.conf.yml <<EOF
+ partitionmanager:
+  num_empty: 2
+  partition_period:
+      days: 90
+  dburl: "sql://user:password@localhost3306:/test_db"
+  tables:
+    cats: {}
+    dogs:
+      partition_period:
+        days: 30
+  prometheus_stats: "/tmp/prometheus-textcollect-partition-manager.prom"
+EOF
+ → partition-manager --config /tmp/partman.conf.yml add --noop
+INFO:root:No-op mode
+INFO:partition:Evaluating Table dogs (duration=30 days, 0:00:00) (pos={'id': 150})
+INFO:partition:Table dogs planned SQL: ALTER TABLE `dogs` REORGANIZE PARTITION `p_20201204` INTO (PARTITION `p_20210422` VALUES LESS THAN (221), PARTITION `p_20210522` VALUES LESS THAN MAXVALUE);
+
+dogs:
+ sql: ALTER TABLE `dogs` REORGANIZE PARTITION `p_20201204` INTO (PARTITION `p_20210422` VALUES LESS THAN (221), PARTITION `p_20210522` VALUES LESS THAN MAXVALUE);
+ noop: True
+```
+
+
+## Hacking on this Project
+
+```sh
+ → git clone https://github.com/letsencrypt/mariadb-sequential-partition-manager-py.git
+ → cd mariadb-sequential-partition-manager-py
+ → python3 -m venv .venv
+ → . .venv/bin/activate
+ → python3 -m pip install --editable .
  → partition-manager --log-level=debug  \
     --mariadb test_tools/fake_mariadb.sh \
     add --noop --table tablename
@@ -28,7 +64,8 @@ ALTER TABLE `dbname`.`tablename` REORGANIZE PARTITION `p_20201204` INTO (PARTITI
 
 ```
 
-You can also use a yaml configuration file with the `--config` parameter of the form:
+# Configuration
+You can use a yaml configuration file with the `--config` parameter of the form:
 ```yaml
 partitionmanager:
   dburl: sql://user:password@localhost/db-name
@@ -48,6 +85,7 @@ partitionmanager:
     table3:
       retention:
         days: 14
+    table4: {}
 ```
 
 For tables which are either partitioned but not yet using this tool's schema, or which have no empty partitions, the `bootstrap` command can be useful for proposing alterations to run manually. Note that `bootstrap` proposes commands that are likely to require partial copies of each table, so likely they will require a maintenance period.
@@ -66,7 +104,16 @@ orders:
 
 ```
 
-# Algorithm
+# Procedure
+
+## Configuration Processing
+
+- At start, if any configuration file specified as a CLI argument, read that configuration file to set all other values.
+- Then, process all remaining command line arguments, overriding values loaded from the configuration file in case of conflicts.
+- From those command-line arguments, determine whether to collect statistics `stats`, determine an initial partition layout `bootstrap`, or operate in the normal `add` mode.
+- Use the configuration information as inputs to the required algorithm.
+
+## "Add" Algorithm
 
 The core algorithm is implemented in a method `plan_partition_changes` in `table_append_partition.py`. That algorithm is:
 
@@ -105,6 +152,14 @@ Procedure:
   - Append the new partition to the intended empty partition list.
 - Return the lists of non-empty partitions, the current empty partitions, and the post-algorithm intended empty partitions.
 
+### "Add" Finalization
+
+The results of the algorithm are converted into `ALTER` statements; if the user configured `--noop` they're emitted to console and the logs for each table. If not set to `--noop`, the application will execute the ALTERs at the database server and emit the results, including execution time as prometheus statistics if so configured.
+
+## "Bootstrap" Algorithm
+
+The bootstrap mode is a limited form of the "Add" Algorithm, using a temporary state file to determine rates-of-change. The bootstrap mode also does not limit itself to only affecting empty partitions, it can and will request changes that will prmopt row copies, in order to prepare a table for future use of the "Add" algorithm.
+
 # TODOs
 
 Lots:
diff --git a/partitionmanager/cli.py b/partitionmanager/cli.py
@@ -14,11 +14,11 @@
     write_state_info,
 )
 from partitionmanager.table_append_partition import (
-    evaluate_partition_changes,
     generate_sql_reorganize_partition_commands,
     get_current_positions,
     get_partition_map,
     plan_partition_changes,
+    should_run_changes,
     table_is_compatible,
 )
 from partitionmanager.types import (
@@ -289,7 +289,7 @@ def do_partition(conf):
                 conf.num_empty,
             )
 
-            if not evaluate_partition_changes(partition_changes):
+            if not should_run_changes(partition_changes):
                 log.info(f"{table} does not need to be modified currently.")
                 continue
             log.debug(f"{table} has changes waiting.")
diff --git a/partitionmanager/table_append_partition.py b/partitionmanager/table_append_partition.py
@@ -338,7 +338,7 @@ def plan_partition_changes(
     # increasing until we get to "now" and the active partition. "Now" actually
     # takes place _after_ active partition's start date (naturally), but
     # contains a position that is before the top of active, by definition. For
-    # the rate processing to work, we need to cross the "now" and the active
+    # the rate processing to work, we need to swap the "now" and the active
     # partition's dates and positions.
     rate_relevant_partitions = filled_partitions + [
         InstantPartition(active_partition.timestamp(), current_positions),
@@ -422,14 +422,13 @@ def plan_partition_changes(
     return results
 
 
-def evaluate_partition_changes(altered_partitions):
+def should_run_changes(altered_partitions):
     """
     Evaluate the list from plan_partition_changes and determine if the set of
     changes should be performed - if all the changes are minor, they shouldn't
-    be run. Returns True if the changeset should run, otherwise logs the reason
-    for skipping and returns False
+    be run. Returns True if the changeset should run, otherwise returns False.
     """
-    log = logging.getLogger("evaluate_partition_changes")
+    log = logging.getLogger("should_run_changes")
 
     for p in altered_partitions:
         if isinstance(p, NewPlannedPartition):
@@ -460,6 +459,7 @@ def generate_sql_reorganize_partition_commands(table, changes):
         if isinstance(p, NewPlannedPartition):
             new_partitions.append(p)
         else:
+            assert not new_partitions, "Modified partitions must preceed new partitions"
             modified_partitions.append(p)
 
     # If there's not at least one modification, bail out
diff --git a/partitionmanager/table_append_partition_test.py b/partitionmanager/table_append_partition_test.py
@@ -19,7 +19,6 @@
     UnexpectedPartitionException,
 )
 from partitionmanager.table_append_partition import (
-    evaluate_partition_changes,
     generate_sql_reorganize_partition_commands,
     generate_weights,
     get_current_positions,
@@ -30,6 +29,7 @@
     plan_partition_changes,
     predict_forward_position,
     predict_forward_time,
+    should_run_changes,
     split_partitions_around_positions,
     table_information_schema_is_compatible,
     table_is_compatible,
@@ -709,15 +709,15 @@ def test_plan_partition_changes(self):
             ],
         )
 
-    def test_evaluate_partition_changes(self):
+    def test_should_run_changes(self):
         self.assertFalse(
-            evaluate_partition_changes(
+            should_run_changes(
                 [ChangePlannedPartition(mkPPart("p_20210102", 200)).set_position([300])]
             )
         )
 
         self.assertFalse(
-            evaluate_partition_changes(
+            should_run_changes(
                 [
                     ChangePlannedPartition(mkPPart("p_20210102", 200)).set_position(
                         [300]
@@ -728,9 +728,9 @@ def test_evaluate_partition_changes(self):
                 ]
             )
         )
-        with self.assertLogs("evaluate_partition_changes", level="DEBUG") as logctx:
+        with self.assertLogs("should_run_changes", level="DEBUG") as logctx:
             self.assertTrue(
-                evaluate_partition_changes(
+                should_run_changes(
                     [
                         ChangePlannedPartition(mkPPart("p_20210102", 200)).set_position(
                             [302]
@@ -749,15 +749,12 @@ def test_evaluate_partition_changes(self):
             )
         self.assertEqual(
             logctx.output,
-            [
-                "DEBUG:evaluate_partition_changes:Add: [542] 2021-01-16 "
-                "00:00:00+00:00 is new"
-            ],
+            ["DEBUG:should_run_changes:Add: [542] 2021-01-16 " "00:00:00+00:00 is new"],
         )
 
-        with self.assertLogs("evaluate_partition_changes", level="DEBUG") as logctx:
+        with self.assertLogs("should_run_changes", level="DEBUG") as logctx:
             self.assertTrue(
-                evaluate_partition_changes(
+                should_run_changes(
                     [
                         ChangePlannedPartition(mkPPart("p_20210102", 200)),
                         NewPlannedPartition()
@@ -771,10 +768,7 @@ def test_evaluate_partition_changes(self):
             )
         self.assertEqual(
             logctx.output,
-            [
-                "DEBUG:evaluate_partition_changes:Add: [542] 2021-01-16 "
-                "00:00:00+00:00 is new"
-            ],
+            ["DEBUG:should_run_changes:Add: [542] 2021-01-16 " "00:00:00+00:00 is new"],
         )
 
     def test_generate_sql_reorganize_partition_commands_no_change(self):
@@ -901,6 +895,25 @@ def test_generate_sql_reorganize_partition_commands_with_duplicate(self):
                 )
             )
 
+    def test_generate_sql_reorganize_partition_commands_out_of_order(self):
+        with self.assertRaises(AssertionError):
+            list(
+                generate_sql_reorganize_partition_commands(
+                    Table("table_with_out_of_order_changeset"),
+                    [
+                        ChangePlannedPartition(mkTailPart("past"))
+                        .set_position([800])
+                        .set_timestamp(datetime(2021, 1, 14, tzinfo=timezone.utc)),
+                        NewPlannedPartition()
+                        .set_position([1000])
+                        .set_timestamp(datetime(2021, 1, 15, tzinfo=timezone.utc)),
+                        ChangePlannedPartition(mkTailPart("future"))
+                        .set_position([1200])
+                        .set_timestamp(datetime(2021, 1, 16, tzinfo=timezone.utc)),
+                    ],
+                )
+            )
+
     def test_plan_and_generate_sql_reorganize_partition_commands_with_future_partition(
         self
     ):
diff --git a/partitionmanager/tools.py b/partitionmanager/tools.py
@@ -19,8 +19,8 @@ def iter_show_end(iterable):
     iterable -> (s0, false), (s1, false), ... (s_n, true).
     """
     it = iter(iterable)
-    last = next(it)
+    prev = next(it)
     for val in it:
-        yield last, False
-        last = val
-    yield last, True
+        yield prev, False
+        prev = val
+    yield prev, True
