Don't choke on lhmn_* in cleanup with range #119
Conversation
When running an Lhm.cleanup with a range it tries to strptime every
table name, but if there are unswitched new tables then it fails:
> Lhm.cleanup(false)
Existing LHM backup tables: lhmn_statement_lines.
Existing LHM triggers: .
Run Lhm.cleanup(true) to drop them all.
> Lhm.cleanup(true, until: 1.week.ago)
...:in `eval': invalid strptime format - `lhma_%Y_%m_%d_%H_%M_%S' (ArgumentError)
from .../lhm-2.2.0/lib/lhm.rb:64:in `block in cleanup'
from .../lhm-2.2.0/lib/lhm.rb:63:in `select!'
from .../lhm-2.2.0/lib/lhm.rb:63:in `cleanup'
This patch includes the table in cleanup over a time range. We can't really
date this table, so can't apply the range logic, but it should just be
duplicated data in the table so should be safe to delete.
|
+1 on the fix. But I'm wondering if we are introducing risk by removing |
|
Yeah I was wondering that too, @taoza, but this is generated duplicate data, not canonical data:
|
|
@sj26 Any chance the Lhm.cleanup is triggered while an online migration is in process and |
|
@tguo sure, but if you're running your migrations in multiple places then I think you have bigger problems. |
| @@ -59,9 +59,11 @@ def change_table(table_name, options = {}, &block) | |||
| def cleanup(run = false, options = {}) | |||
| lhm_tables = connection.select_values('show tables').select { |name| name =~ /^lhm(a|n)_/ } | |||
| if options[:until] | |||
| lhm_tables.select! do |table| | |||
There was a problem hiding this comment.
Instead of changing this code block, what about just changing the regex on line 60 to ^lhma_?
There was a problem hiding this comment.
Because we want to clean up all lhm tables, but only filter the archive tables.
|
I'll close this one if there's no movement. :-) |
When running an Lhm.cleanup with a range it tries to strptime every table name, but if there are unswitched new tables then it fails:
This patch includes the table in cleanup over a time range. We can't really date this table, so can't apply the range logic, but it should just be duplicated data in the table so should be safe to delete.