Skip to content

Commit adbe13d

Browse files
jcjonesjcjones
authored
Set WAIT 2 for all ALTER TABLE REORGANIZE PARTITION calls (#57)
* Set WAIT 2 for all ALTER TABLE REORGANIZE PARTITION calls * Ensure alter_time_seconds gets updated even on nonzero SQL exits. Thanks @mcpherrinm * Add a metric for alter_errors and reorder logs to be clearer on large tables
1 parent 840c62d commit adbe13d

File tree

7 files changed

+65
-36
lines changed

7 files changed

+65
-36
lines changed

‎partitionmanager/cli.py‎

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -296,9 +296,15 @@ def do_partition(conf):
296296
help_text="Time in seconds to complete the ALTER command",
297297
type_name="gauge",
298298
)
299+
metrics.describe(
300+
"alter_errors",
301+
help_text="Number of errors observed during ALTER commands",
302+
type_name="counter",
303+
)
299304

300305
all_results = dict()
301306
for table in conf.tables:
307+
time_start = None
302308
try:
303309
table_problems = pm_tap.get_table_compatibility_problems(conf.dbcmd, table)
304310
if table_problems:
@@ -311,11 +317,13 @@ def do_partition(conf):
311317
if table.partition_period:
312318
duration = table.partition_period
313319

320+
log.info(f"Evaluating {table} (duration={duration})")
321+
314322
positions = pm_tap.get_current_positions(
315323
conf.dbcmd, table, map_data["range_cols"]
316324
)
317325

318-
log.info(f"Evaluating {table} (duration={duration}) (pos={positions})")
326+
log.info(f"{table} (pos={positions})")
319327

320328
cur_pos = partitionmanager.types.Position()
321329
cur_pos.set_position([positions[col] for col in map_data["range_cols"]])
@@ -344,22 +352,29 @@ def do_partition(conf):
344352
log.info(f"{table} running SQL: {composite_sql_command}")
345353
time_start = datetime.utcnow()
346354
output = conf.dbcmd.run(composite_sql_command)
347-
time_end = datetime.utcnow()
348355

349356
all_results[table.name] = {"sql": composite_sql_command, "output": output}
350357
log.info(f"{table} results: {output}")
351-
metrics.add(
352-
"alter_time_seconds",
353-
table.name,
354-
(time_end - time_start).total_seconds(),
355-
)
358+
356359
except partitionmanager.types.NoEmptyPartitionsAvailableException:
357360
log.warning(
358361
f"Unable to automatically handle {table}: No empty "
359362
"partition is available."
360363
)
364+
except partitionmanager.types.DatabaseCommandException as e:
365+
log.warning("Failed to automatically handle %s: %s", table, e)
366+
metrics.add("alter_errors", table.name, 1)
361367
except (ValueError, Exception) as e:
362368
log.warning("Failed to handle %s: %s", table, e)
369+
metrics.add("alter_errors", table.name, 1)
370+
371+
time_end = datetime.utcnow()
372+
if time_start:
373+
metrics.add(
374+
"alter_time_seconds",
375+
table.name,
376+
(time_end - time_start).total_seconds(),
377+
)
363378

364379
if conf.prometheus_stats_path:
365380
do_stats(conf, metrics=metrics)

‎partitionmanager/cli_test.py‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ def test_partition_cmd_noop(self):
8080
{
8181
"testtable_noop": {
8282
"sql": (
83-
"ALTER TABLE `testtable_noop` REORGANIZE PARTITION "
83+
"ALTER TABLE `testtable_noop` WAIT 2 REORGANIZE PARTITION "
8484
"`p_20201204` INTO "
8585
"(PARTITION `p_20201112` VALUES LESS THAN (548), "
8686
"PARTITION `p_20201212` VALUES LESS THAN MAXVALUE);"
@@ -102,7 +102,7 @@ def test_partition_cmd_final(self):
102102
"testtable_commit": {
103103
"output": [],
104104
"sql": (
105-
"ALTER TABLE `testtable_commit` REORGANIZE PARTITION "
105+
"ALTER TABLE `testtable_commit` WAIT 2 REORGANIZE PARTITION "
106106
"`p_20201204` INTO "
107107
"(PARTITION `p_20201112` VALUES LESS THAN (548), "
108108
"PARTITION `p_20201212` VALUES LESS THAN MAXVALUE);"
@@ -223,10 +223,12 @@ def test_partition_period_seven_days(self):
223223
set(
224224
[
225225
"INFO:partition:Evaluating Table partitioned_last_week "
226-
"(duration=7 days, 0:00:00) (pos={'id': 150})",
226+
"(duration=7 days, 0:00:00)",
227+
"INFO:partition:Table partitioned_last_week (pos={'id': 150})",
227228
"DEBUG:partition:Table partitioned_last_week has no pending SQL updates.",
228229
"INFO:partition:Evaluating Table partitioned_yesterday "
229-
"(duration=7 days, 0:00:00) (pos={'id': 150})",
230+
"(duration=7 days, 0:00:00)",
231+
"INFO:partition:Table partitioned_yesterday (pos={'id': 150})",
230232
"DEBUG:partition:Table partitioned_yesterday has no pending SQL updates.",
231233
]
232234
),
@@ -500,7 +502,7 @@ def test_migrate_cmd_in(self):
500502
+ "PARTITION BY RANGE (id) (",
501503
"\tPARTITION p_assumed VALUES LESS THAN MAXVALUE",
502504
");",
503-
"ALTER TABLE `partitioned_yesterday_new_20210421` "
505+
"ALTER TABLE `partitioned_yesterday_new_20210421` WAIT 2 "
504506
+ "REORGANIZE PARTITION `p_assumed` INTO (PARTITION "
505507
+ "`p_20210421` VALUES LESS THAN (150), PARTITION "
506508
+ "`p_20210521` VALUES LESS THAN (300), PARTITION "
@@ -525,7 +527,7 @@ def test_migrate_cmd_in(self):
525527
"ALTER TABLE two_new_20210421 PARTITION BY RANGE (id) (",
526528
"\tPARTITION p_assumed VALUES LESS THAN MAXVALUE",
527529
");",
528-
"ALTER TABLE `two_new_20210421` REORGANIZE PARTITION "
530+
"ALTER TABLE `two_new_20210421` WAIT 2 REORGANIZE PARTITION "
529531
+ "`p_assumed` INTO (PARTITION `p_20210421` VALUES "
530532
+ "LESS THAN (150), PARTITION `p_20210521` VALUES LESS "
531533
+ "THAN (375), PARTITION `p_20210620` VALUES LESS THAN "
@@ -585,7 +587,7 @@ def test_migrate_cmd_in_unpartitioned_with_override(self):
585587
"ALTER TABLE unpartitioned_new_20210421 PARTITION BY RANGE (id) (",
586588
"\tPARTITION p_assumed VALUES LESS THAN MAXVALUE",
587589
");",
588-
"ALTER TABLE `unpartitioned_new_20210421` REORGANIZE "
590+
"ALTER TABLE `unpartitioned_new_20210421` WAIT 2 REORGANIZE "
589591
+ "PARTITION `p_assumed` INTO (PARTITION `p_20210421` "
590592
+ "VALUES LESS THAN (150), PARTITION `p_20210521` VALUES "
591593
+ "LESS THAN (300), PARTITION `p_20210620` VALUES LESS "

‎partitionmanager/migrate_test.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ def test_read_state_info(self):
131131
"ALTER TABLE test_new_20210303 PARTITION BY RANGE (id) (",
132132
"\tPARTITION p_start VALUES LESS THAN MAXVALUE",
133133
");",
134-
"ALTER TABLE `test_new_20210303` REORGANIZE PARTITION `p_start` "
134+
"ALTER TABLE `test_new_20210303` WAIT 2 REORGANIZE PARTITION `p_start` "
135135
+ "INTO (PARTITION `p_20210303` VALUES LESS THAN (156), "
136136
+ "PARTITION `p_20210402` VALUES LESS THAN (2406), PARTITION "
137137
+ "`p_20210502` VALUES LESS THAN MAXVALUE);",
@@ -187,7 +187,7 @@ def test_read_state_info_map_table(self):
187187
+ "COLUMNS (orderID, authzID) (",
188188
"\tPARTITION p_assumed VALUES LESS THAN (MAXVALUE, MAXVALUE)",
189189
");",
190-
"ALTER TABLE `map_table_new_20210303` REORGANIZE PARTITION "
190+
"ALTER TABLE `map_table_new_20210303` WAIT 2 REORGANIZE PARTITION "
191191
+ "`p_assumed` INTO (PARTITION `p_20210303` VALUES LESS THAN "
192192
+ "(11, 22), PARTITION `p_20210402` VALUES LESS THAN "
193193
+ "(41, 82), PARTITION `p_20210502` VALUES LESS THAN "

‎partitionmanager/sql.py‎

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -123,15 +123,23 @@ def __init__(self, exe):
123123

124124
def run(self, sql_cmd):
125125
logging.debug(f"SubprocessDatabaseCommand executing {sql_cmd}")
126-
result = subprocess.run(
127-
[self.exe, "-X"],
128-
input=sql_cmd,
129-
stdout=subprocess.PIPE,
130-
stderr=subprocess.DEVNULL,
131-
encoding="UTF-8",
132-
check=True,
133-
)
134-
return XmlResult().parse(result.stdout)
126+
try:
127+
result = subprocess.run(
128+
[self.exe, "-X"],
129+
input=sql_cmd,
130+
stdout=subprocess.PIPE,
131+
stderr=subprocess.DEVNULL,
132+
encoding="UTF-8",
133+
check=True,
134+
)
135+
return XmlResult().parse(result.stdout)
136+
except subprocess.CalledProcessError as cpe:
137+
logging.error(
138+
"SubprocessDatabaseCommand failed, error code %d", cpe.returncode
139+
)
140+
logging.error("stdout: %s", cpe.stdout)
141+
logging.error("stderr: %s", cpe.stderr)
142+
raise partitionmanager.types.DatabaseCommandException(cpe.stderr)
135143

136144
def db_name(self):
137145
rows = self.run("SELECT DATABASE();")

‎partitionmanager/table_append_partition.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -729,7 +729,7 @@ def generate_sql_reorganize_partition_commands(table, changes):
729729
partition_update = ", ".join(partition_strings)
730730

731731
alter_cmd = (
732-
f"ALTER TABLE `{table.name}` "
732+
f"ALTER TABLE `{table.name}` WAIT 2 "
733733
f"REORGANIZE PARTITION `{modified_partition.old.name}` INTO ({partition_update});"
734734
)
735735

‎partitionmanager/table_append_partition_test.py‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -669,7 +669,7 @@ def test_plan_partition_changes_short_names(self):
669669
self.assertEqual(
670670
output,
671671
[
672-
"ALTER TABLE `table` REORGANIZE PARTITION `p_future` INTO "
672+
"ALTER TABLE `table` WAIT 2 REORGANIZE PARTITION `p_future` INTO "
673673
"(PARTITION `p_20210201` VALUES LESS THAN (12960433003), "
674674
"PARTITION `p_20210208` VALUES LESS THAN MAXVALUE);"
675675
],
@@ -705,7 +705,7 @@ def test_plan_partition_changes_bespoke_names(self):
705705
self.assertEqual(
706706
output,
707707
[
708-
"ALTER TABLE `table` REORGANIZE PARTITION `p_future` INTO "
708+
"ALTER TABLE `table` WAIT 2 REORGANIZE PARTITION `p_future` INTO "
709709
"(PARTITION `p_20210108` VALUES LESS THAN (170), "
710710
"PARTITION `p_20210115` VALUES LESS THAN MAXVALUE);"
711711
],
@@ -1007,7 +1007,7 @@ def testgenerate_sql_reorganize_partition_commands_single_change(self):
10071007
)
10081008
),
10091009
[
1010-
"ALTER TABLE `table` REORGANIZE PARTITION `p_20210102` INTO "
1010+
"ALTER TABLE `table` WAIT 2 REORGANIZE PARTITION `p_20210102` INTO "
10111011
"(PARTITION `p_20210116` VALUES LESS THAN (542, 190));"
10121012
],
10131013
)
@@ -1028,9 +1028,9 @@ def testgenerate_sql_reorganize_partition_commands_two_changes(self):
10281028
)
10291029
),
10301030
[
1031-
"ALTER TABLE `table` REORGANIZE PARTITION `p_20210120` INTO "
1031+
"ALTER TABLE `table` WAIT 2 REORGANIZE PARTITION `p_20210120` INTO "
10321032
"(PARTITION `p_20210214` VALUES LESS THAN (2000));",
1033-
"ALTER TABLE `table` REORGANIZE PARTITION `p_20210102` INTO "
1033+
"ALTER TABLE `table` WAIT 2 REORGANIZE PARTITION `p_20210102` INTO "
10341034
"(PARTITION `p_20210116` VALUES LESS THAN (500));",
10351035
],
10361036
)
@@ -1052,7 +1052,7 @@ def testgenerate_sql_reorganize_partition_commands_new_partitions(self):
10521052
)
10531053
),
10541054
[
1055-
"ALTER TABLE `table` REORGANIZE PARTITION `p_20210102` INTO "
1055+
"ALTER TABLE `table` WAIT 2 REORGANIZE PARTITION `p_20210102` INTO "
10561056
"(PARTITION `p_20210102` VALUES LESS THAN (200), "
10571057
"PARTITION `p_20210116` VALUES LESS THAN (542), "
10581058
"PARTITION `p_20210123` VALUES LESS THAN (662));"
@@ -1081,7 +1081,7 @@ def testgenerate_sql_reorganize_partition_commands_maintain_new_partition(self):
10811081
)
10821082
),
10831083
[
1084-
"ALTER TABLE `table` REORGANIZE PARTITION `future` INTO "
1084+
"ALTER TABLE `table` WAIT 2 REORGANIZE PARTITION `future` INTO "
10851085
"(PARTITION `p_20210114` VALUES LESS THAN (800), "
10861086
"PARTITION `p_20210116` VALUES LESS THAN (1000), "
10871087
"PARTITION `p_20210123` VALUES LESS THAN (1200), "
@@ -1147,9 +1147,9 @@ def test_plan_andgenerate_sql_reorganize_partition_commands_with_future_partitio
11471147
self.assertEqual(
11481148
list(generate_sql_reorganize_partition_commands(Table("water"), planned)),
11491149
[
1150-
"ALTER TABLE `water` REORGANIZE PARTITION `future` INTO "
1150+
"ALTER TABLE `water` WAIT 2 REORGANIZE PARTITION `future` INTO "
11511151
"(PARTITION `p_20210105` VALUES LESS THAN MAXVALUE);",
1152-
"ALTER TABLE `water` REORGANIZE PARTITION `p_20210104` INTO "
1152+
"ALTER TABLE `water` WAIT 2 REORGANIZE PARTITION `p_20210104` INTO "
11531153
"(PARTITION `p_20210102` VALUES LESS THAN (200));",
11541154
],
11551155
)
@@ -1211,7 +1211,7 @@ def test_get_pending_sql_reorganize_partition_commands_with_changes(self):
12111211
self.assertEqual(
12121212
list(cmds),
12131213
[
1214-
"ALTER TABLE `plushies` REORGANIZE PARTITION `future` INTO "
1214+
"ALTER TABLE `plushies` WAIT 2 REORGANIZE PARTITION `future` INTO "
12151215
"(PARTITION `p_20210104` VALUES LESS THAN (550), "
12161216
"PARTITION `p_20210111` VALUES LESS THAN (900), "
12171217
"PARTITION `p_20210118` VALUES LESS THAN MAXVALUE);"

‎partitionmanager/types.py‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -603,3 +603,7 @@ class TableInformationException(Exception):
603603

604604
class NoEmptyPartitionsAvailableException(Exception):
605605
"""Raised if no empty partitions are available to safely modify."""
606+
607+
608+
class DatabaseCommandException(Exception):
609+
"""Raised if the database command failed."""

0 commit comments

Comments
 (0)