Run OPTIMIZE TABLE when required #101
Conversation
Codecov ReportBase: 76.59% // Head: 76.81% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #101 +/- ##
==========================================
+ Coverage 76.59% 76.81% +0.22%
==========================================
Files 16 16
Lines 3905 3930 +25
Branches 932 940 +8
==========================================
+ Hits 2991 3019 +28
+ Misses 702 688 -14
- Partials 212 223 +11
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
nicois
force-pushed
the
nicois/INC-348-optimise-tables-on-error
branch
3 times, most recently
from
195f83e
to
e4057b6
Compare
Due to changes in the underlying mysql version, it may be required to run OPTIMIZE TABLE on tables which have been ALTERed, to prevent cuprruption. xtrabackup detects this situation and reports which tables are impacted, so myhoard simply needs to carry out this operation.
nicois
force-pushed
the
nicois/INC-348-optimise-tables-on-error
branch
from
e4057b6
to
78d7b26
Compare
|
This change originated from INC-348 |
There was a problem hiding this comment.
- Github is reporting conflicts that need to be addressed
- Some tests are failing
- The implemented behaviour seems to be very xtrabackup-version dependent, but we currently only test with one xtrabackup version, while in prod for Aiven we employ the same version for mysql and xtrabackup.
This change won't appear for mysql < 8.0.30 and xtrabackup 8.0.30 btw; but I think we should pin the xtrabackup version in ci files, otherwise this will break as soon as xtrabackup 8.0.30 is out.
| for line in xtrabackup_lines[2:]: | ||
| if line.startswith("Please run OPTIMIZE TABLE"): | ||
| return | ||
| yield line.replace("/", ".") |
There was a problem hiding this comment.
the output of this function is fed directly to the db via string interpolation. I think we'd need a bit of validation here - hcheck for valid chars only, or perform some escaping so that the wrong data cannot enter the db).
There was a problem hiding this comment.
Also: I'd return a better object, e.g. a 2-tuple with (db, table), then I'd compose it when creating the query, so that we can validate the names separately and we don't have to deal with string content if we want to perform some other manipulation in the future.
| @@ -25,6 +25,29 @@ | |||
| ERR_TIMEOUT = 2013 | |||
|
|
|||
|
|
|||
| def get_tables_to_optimise(error_lines: list[str], *, log: Logger) -> Iterator[str]: | |||
There was a problem hiding this comment.
I'd really prefer a non-lazy return type. Why?
I doubt there's a performance issue here and that generating an iterator helps us in that regard, and it's quite a common bug to try accidentally reusing an iterator.
|
Also, I think that #102 should work as well? |
Due to changes in the underlying mysql version, it may be required to run OPTIMIZE TABLE on tables which have been ALTERed, to prevent cuprruption.
xtrabackup detects this situation and reports which tables are impacted, so myhoard simply needs to carry out this operation.