-
Notifications
You must be signed in to change notification settings - Fork 94
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Working on funcs to move default partition data to new partitions #2547
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2547 +/- ##
=======================================
Coverage 94.7% 94.8%
=======================================
Files 281 281
Lines 21337 21514 +177
Branches 2433 2443 +10
=======================================
+ Hits 20215 20389 +174
- Misses 673 674 +1
- Partials 449 451 +2 |
IF (copy_data OR | ||
(object_rec.table_name ~ 'partitioned_tables') OR | ||
(object_rec.table_name ~ 'django_migrations')) AND | ||
(object_rec.table_kind = 'r') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is to fix a bug copying partitioned table data.
' DETACH PARTITION ' || | ||
quote_ident(OLD.schema_name) || '.' || quote_ident(OLD.table_name) || | ||
' ;'; | ||
IF ( OLD.active ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The multiple active
checks here prevent unnecessary (and unwanted) action when updating the partitiion_parameters when the record is inactive.
quote_ident(OLD.schema_name) || '.' || quote_ident(OLD.table_name) || ' '; | ||
IF ( (NEW.partition_parameters->>'default') = 'true' ) | ||
THEN | ||
action_stmt = action_stmt || 'DEFAULT ;'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixes malformed statement build
cur = conn_execute(part_track_insert_sql, vals, _conn=self.conn) | ||
self.tracking_rec = fetchone(cur) | ||
|
||
def _create_partition(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
_create_partition
, _attach_partition
, and _detach_partition
were written to take advantage of the trigger code.
|
||
return res | ||
|
||
def repartition_default_data(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the main method to operate on a partitioned table's default partition.
@Red-HAP can you include the bugfix in the OCP processor here so we do start creating partitions See: https://github.com/project-koku/koku/blob/master/koku/masu/processor/ocp/ocp_report_processor.py#L143 Here is the bug: https://github.com/project-koku/koku/blob/master/koku/masu/processor/ocp/ocp_report_processor.py#L241 on that line we are not doing the replace, and the try actually always fails on the parser, but we fail silently and no partition is created. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Checked out master
- Ran
make load-test-customer-data start=2020-11-01 end=2021-01-25
- Checked and only saw default partition
- Checked out move_default_partition_data
- Ran run-migratons
- Checked partitions and confirmed all data is where it should be.
Ticket
COST-812
Description
Functions that will move partitioned data from the default partition by month to newly created month partitions.
It's broken up into a few components:
PartitionDefaultData
: This is the meat of the processing. The flow works like this: Given a schema, partitioned table, and default table partitionrepartition_default_data
: This is a driver function that will get default partition information from all tenant schemata and usePartitionDefaultData
to fix 'emDesigning the code this way was necessary for two reasons:
Utilization of the code can be done as a task to crawl the entire database or to simply execute once processing has been completed, whichever is more relevant.
This code is not meant to bypass the most efficient means of managing partitions which is to create them as needed during data processing.
Currently, this branch contains a migration that will apply the functionality on a one-time basis.
Review Request
Please have a close look at the tests to see if there is something I missed in testing. This must be bulletproof as it is moving customer data (delete + insert).
Testing
This is easiest to check with the
reporting_ocpusagelineitem_daily_summary
table (OCPUsageLineItemDailySummary
model)Hard
usage_start
value 10 years in the future to get data into the default partition.Easy
usage_start
value 10 years in the future to get data into the default partition.move_default_partition_data
branchmake run-migrations