Re: [openstack-dev] [nova] if by "archived" you mean, "wipes out your tables completely", then sure, it works fine
On 3/16/2015 8:48 AM, Attila Fazekas wrote: Hi Mike, The points was, there is no real need or real use case for archiving the db as the nova-mange does. What is the exact use case ? Auditing ? Accounting ? * Keystone allows permanent delete, if you need to do auditing probably the user accounts would the primary target for saving. * The logs+elasticsearch(or just grep) and ceilometer+mongodb is designed to help in `archiving` and keep the things what you actually need. * After one year you can have ~100M deleted server instance record in the shadow tables (+ the related rows), what to do with them ? Truncate ? If you have proper indexes on the main tables the deleted records mostly just consumes disk space, otherwise they also causes serious performance issues. If anybody would like to keep the deleted things in SQL for whatever reason, he very likely want to do in a different database instance on a different server, it is also likely he would like to do some transformation(OLAP) instead of attacking the production DB with full table scans while also invalidating the `Buffer Pool` content. The feature as it is does not makes sense even after fixing the existing bugs. I do not know what would be it's actual use case, even if there is one, probably it is not the best approach. My suggestion is just nuke it, and came up with `simple` script which archives the old records to /dev/null. $ nova-mange db flush 7d This would deletes the soft-deleted records in small chunks (like token-flush). (or just stop doing soft-delete.) - Original Message - From: "Mike Bayer" To: "Attila Fazekas" Cc: "OpenStack Development Mailing List (not for usage questions)" Sent: Friday, March 13, 2015 5:04:21 PM Subject: Re: [openstack-dev] [nova] if by "archived" you mean, "wipes out your tables completely", then sure, it works fine Attila Fazekas wrote: The archiving has issues since very long time [1], something like this [2] is expected to replace it. yeah I was thinking of just rewriting the archive routine in Nova to be reasonable, but I can build this routine into Oslo.db as well as a generic “move rows with criteria X into tables”. Archiving as it is is mostly useless if it isn’t considering dependencies between tables (https://bugs.launchpad.net/nova/+bug/1183523) so the correct approach would need to consider tables and potentially rows in terms of foreign key dependency. This is what the unit of work was built to handle. Though I’m not sure I can make this a generic ORM play since we want to be able to delete “only N” rows, and it would probably be nice for the system to not spend its time reading in the entire DB if it is only tasked with a few dozen rows, so it might need to implement its own mini-unit-of-work system that works against the same paradigm but specific to this use case. The simplest case is that we address the archival of tables in order of foreign key dependency. However, that has two issues in the “generic” sense. One is that there can be cycles between tables, or a table that refers to itself has a cycle to itself. So in those cases the archival on a “sort the tables” basis needs to be broken into a “sort the rows” basis. This is what SQLAlchemy’s unit of work does and I’d adapt that here. The other possible, but probably unlikely, issue is that to address this “generically”, if a row “Table A row 1” is referred to by a “Table B row 2”, it might not be assumable that it is safe to remove “Table B Row 2” and *not* “Table A row 1”. The application may rely on both of these rows being present, and the SQLAlchemy pattern where this is the case is the so-called “joined table inheritance” case. But the “joined table inheritance” pattern is actually not very easy to adapt to the “shadow” model so I doubt anyone is doing that. IMHO we should forget about solving how to move them safely to a different table, the issue is how to delete them in relative small transactions ~100 instances(+referenced/related records), without causing full table scans or causing reference violation issues. keystone token-flush also has a logic to do the delete in smaller chunks, in order to do not stall regular processing for a long time or hit DB replication limit issues. keystone targets to do 1000 row delete per transaction with mysql, some cases actually the deleted row number differs. PS.: Adding indexes on the deleted_at fields is acceptable. The archiving just move trash to the other side of the desk, usually just permanently deleting everything what is deleted for more than 7 day is better for everyone. For now, maybe just wiping out the shadow tables and the existing nova-mange functionality is better choice. [3] [1] https://bugs.launchpad.net/nova/+bug/1305892 [2] https://blueprints.launchpad.net/nova/+spec/db-purge-engine [3] - Original Message - From: "Mike Bayer"
Re: [openstack-dev] [nova] if by "archived" you mean, "wipes out your tables completely", then sure, it works fine
Hi Mike, The points was, there is no real need or real use case for archiving the db as the nova-mange does. What is the exact use case ? Auditing ? Accounting ? * Keystone allows permanent delete, if you need to do auditing probably the user accounts would the primary target for saving. * The logs+elasticsearch(or just grep) and ceilometer+mongodb is designed to help in `archiving` and keep the things what you actually need. * After one year you can have ~100M deleted server instance record in the shadow tables (+ the related rows), what to do with them ? Truncate ? If you have proper indexes on the main tables the deleted records mostly just consumes disk space, otherwise they also causes serious performance issues. If anybody would like to keep the deleted things in SQL for whatever reason, he very likely want to do in a different database instance on a different server, it is also likely he would like to do some transformation(OLAP) instead of attacking the production DB with full table scans while also invalidating the `Buffer Pool` content. The feature as it is does not makes sense even after fixing the existing bugs. I do not know what would be it's actual use case, even if there is one, probably it is not the best approach. My suggestion is just nuke it, and came up with `simple` script which archives the old records to /dev/null. $ nova-mange db flush 7d This would deletes the soft-deleted records in small chunks (like token-flush). (or just stop doing soft-delete.) - Original Message - > From: "Mike Bayer" > To: "Attila Fazekas" > Cc: "OpenStack Development Mailing List (not for usage questions)" > > Sent: Friday, March 13, 2015 5:04:21 PM > Subject: Re: [openstack-dev] [nova] if by "archived" you mean, "wipes > out your tables completely", then sure, it > works fine > > > > Attila Fazekas wrote: > > > The archiving has issues since very long time [1], > > something like this [2] is expected to replace it. > > > yeah I was thinking of just rewriting the archive routine in Nova to be > reasonable, but I can build this routine into Oslo.db as well as a generic > “move rows with criteria X into tables”. Archiving as it is is mostly > useless if it isn’t considering dependencies between tables > (https://bugs.launchpad.net/nova/+bug/1183523) so the correct approach would > need to consider tables and potentially rows in terms of foreign key > dependency. This is what the unit of work was built to handle. Though I’m > not sure I can make this a generic ORM play since we want to be able to > delete “only N” rows, and it would probably be nice for the system to not > spend its time reading in the entire DB if it is only tasked with a few > dozen rows, so it might need to implement its own mini-unit-of-work system > that works against the same paradigm but specific to this use case. > > The simplest case is that we address the archival of tables in order of > foreign key dependency. However, that has two issues in the “generic” sense. > One is that there can be cycles between tables, or a table that refers to > itself has a cycle to itself. So in those cases the archival on a “sort the > tables” basis needs to be broken into a “sort the rows” basis. This is what > SQLAlchemy’s unit of work does and I’d adapt that here. > > The other possible, but probably unlikely, issue is that to address this > “generically”, if a row “Table A row 1” is referred to by a “Table B row 2”, > it might not be assumable that it is safe to remove “Table B Row 2” and > *not* “Table A row 1”. The application may rely on both of these rows being > present, and the SQLAlchemy pattern where this is the case is the so-called > “joined table inheritance” case. But the “joined table inheritance” pattern > is actually not very easy to adapt to the “shadow” model so I doubt anyone > is doing that. IMHO we should forget about solving how to move them safely to a different table, the issue is how to delete them in relative small transactions ~100 instances(+referenced/related records), without causing full table scans or causing reference violation issues. keystone token-flush also has a logic to do the delete in smaller chunks, in order to do not stall regular processing for a long time or hit DB replication limit issues. keystone targets to do 1000 row delete per transaction with mysql, some cases actually the deleted row number differs. PS.: Adding indexes on the deleted_at fields is acceptable. > > The archiving just move trash to the other side of the desk, > > usually just permanently deleting everything what is deleted > > for more than 7 day is better for everyone. > > > > For now, maybe just wiping out the shadow tables and the exi
Re: [openstack-dev] [nova] if by "archived" you mean, "wipes out your tables completely", then sure, it works fine
Attila Fazekas wrote: > The archiving has issues since very long time [1], > something like this [2] is expected to replace it. yeah I was thinking of just rewriting the archive routine in Nova to be reasonable, but I can build this routine into Oslo.db as well as a generic “move rows with criteria X into tables”. Archiving as it is is mostly useless if it isn’t considering dependencies between tables (https://bugs.launchpad.net/nova/+bug/1183523) so the correct approach would need to consider tables and potentially rows in terms of foreign key dependency. This is what the unit of work was built to handle. Though I’m not sure I can make this a generic ORM play since we want to be able to delete “only N” rows, and it would probably be nice for the system to not spend its time reading in the entire DB if it is only tasked with a few dozen rows, so it might need to implement its own mini-unit-of-work system that works against the same paradigm but specific to this use case. The simplest case is that we address the archival of tables in order of foreign key dependency. However, that has two issues in the “generic” sense. One is that there can be cycles between tables, or a table that refers to itself has a cycle to itself. So in those cases the archival on a “sort the tables” basis needs to be broken into a “sort the rows” basis. This is what SQLAlchemy’s unit of work does and I’d adapt that here. The other possible, but probably unlikely, issue is that to address this “generically”, if a row “Table A row 1” is referred to by a “Table B row 2”, it might not be assumable that it is safe to remove “Table B Row 2” and *not* “Table A row 1”. The application may rely on both of these rows being present, and the SQLAlchemy pattern where this is the case is the so-called “joined table inheritance” case. But the “joined table inheritance” pattern is actually not very easy to adapt to the “shadow” model so I doubt anyone is doing that. > The archiving just move trash to the other side of the desk, > usually just permanently deleting everything what is deleted > for more than 7 day is better for everyone. > > For now, maybe just wiping out the shadow tables and the existing nova-mange > functionality is better choice. [3] > > [1] https://bugs.launchpad.net/nova/+bug/1305892 > [2] https://blueprints.launchpad.net/nova/+spec/db-purge-engine > [3] > > - Original Message - >> From: "Mike Bayer" >> To: "OpenStack Development Mailing List (not for usage questions)" >> >> Sent: Friday, March 13, 2015 12:29:55 AM >> Subject: [openstack-dev] [nova] if by "archived" you mean, "wipes out your >> tables completely", then sure, it works >> fine >> >> Hello Nova - >> >> Not sure if I’m just staring at this for too long, or if >> archive_deleted_rows_for_table() is just not something we ever use. >> Because it looks like it’s really, really broken very disastrously, and I’m >> wondering if I’m just missing something in front of me. >> >> Let’s look at what it does! >> >> First, archive_deleted_rows() calls it with a table name. These names are >> taken by collecting every single table name from nova.db.sqlalchemy.models. >> >> Then, the function uses table reflection (that is, doesn’t look in the model >> at all, just goes right to the database) to load the table definitions: >> >>table = Table(tablename, metadata, autoload=True) >>shadow_tablename = _SHADOW_TABLE_PREFIX + tablename >>rows_archived = 0 >>try: >>shadow_table = Table(shadow_tablename, metadata, autoload=True) >>except NoSuchTableError: >># No corresponding shadow table; skip it. >>return rows_archived >> >> this is pretty heavy handed and wasteful from an efficiency point of view, >> and I’d like to fix this too, but let’s go with it. Now we have the two >> tables. >> >> Then we do this: >> >>deleted_column = table.c.deleted >>query_insert = sql.select([table], >> deleted_column != deleted_column.default).\ >> order_by(column).limit(max_rows) >>query_delete = sql.select([column], >> deleted_column != deleted_column.default).\ >> order_by(column).limit(max_rows) >> >> We make some SELECT statements that we’re going to use to find “soft >> deleted” rows, and these will be embedded into an INSERT >> and a DELETE. It is trying to make a statement like “SELECT .. FROM >> table WHERE deleted != ”, so that it finds rows where >> “deleted” has been changed to something, e.g. the row was >> soft d
Re: [openstack-dev] [nova] if by "archived" you mean, "wipes out your tables completely", then sure, it works fine
On 03/13/2015 07:57 AM, Sean Dague wrote: > On 03/13/2015 07:07 AM, Sean Dague wrote: >> On 03/13/2015 05:54 AM, Thomas Herve wrote: >>> >>> [snip] If we assume that all of our tables are filled up with zeroes for those deleted columns, because that’s the default, this **wipes the whole table clean**. How do the tests pass? Well the tests are in test_db_api->ArchiveTestCase, and actually, they don’t. But they don’t fail every time, because the test suite here runs with a database that is almost completely empty anyway, so the broken archival routine doesn’t find too many rows to blow away except for the rows in “instance_types”, which it only finds sometimes because the tests are only running it with a small number of things to delete and the order of the tables is non-deterministic. I’ve posted the bug report at https://bugs.launchpad.net/nova/+bug/1431571 where I started out not knowing much about how this worked except that my tests were failing, and slowly stumbled my way to come to this conclusion. A patch is at https://review.openstack.org/#/c/164009/, where we look at the actual Python-side default. However I’d recommend that we just hardcode the zero here, since that’s how our soft-delete columns work. >>> >>> Hi Mike, >>> >>> Thanks for the investigation. I was wondering when that behavior was >>> introduced and it seems that >>> http://git.openstack.org/cgit/openstack/nova/commit/?id=ecf74d4c0a5a8a4290ecac048fc437dafe3d40fc >>> is the likely culprit, which would mean that only Kilo is affected. Can >>> you confirm? >>> >>> Thanks, >> >> Yes, that looks like the problematic patch. I'd rather actually revert >> that patch instead. >> >> Also, real tests would be nice to actually prevent future regression. >> >> -Sean >> > > Ok, we've done a straight revert here - > https://review.openstack.org/#/c/164140/ > > I'm also working on a test enhancement that ensures that all the shadow > tables except the one we believe should contain entries are empty. That > seems to specifically expose and nail this bug. Needs some cleanup, but > should be posted by midday. The test additions are now posted here - https://review.openstack.org/#/c/164178/ -Sean -- Sean Dague http://dague.net __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [nova] if by "archived" you mean, "wipes out your tables completely", then sure, it works fine
On 03/13/2015 07:07 AM, Sean Dague wrote: > On 03/13/2015 05:54 AM, Thomas Herve wrote: >> >> [snip] >>> If we assume that all of our tables are filled up with zeroes for those >>> deleted columns, because that’s the default, this **wipes the whole table >>> clean**. >>> >>> How do the tests pass? Well the tests are in test_db_api->ArchiveTestCase, >>> and actually, they don’t. But they don’t fail every time, because the test >>> suite here runs with a database that is almost completely empty anyway, so >>> the broken archival routine doesn’t find too many rows to blow away except >>> for the rows in “instance_types”, which it only finds sometimes because the >>> tests are only running it with a small number of things to delete and the >>> order of the tables is non-deterministic. >>> >>> I’ve posted the bug report at https://bugs.launchpad.net/nova/+bug/1431571 >>> where I started out not knowing much about how this worked except that my >>> tests were failing, and slowly stumbled my way to come to this conclusion. A >>> patch is at https://review.openstack.org/#/c/164009/, where we look at the >>> actual Python-side default. However I’d recommend that we just hardcode the >>> zero here, since that’s how our soft-delete columns work. >> >> Hi Mike, >> >> Thanks for the investigation. I was wondering when that behavior was >> introduced and it seems that >> http://git.openstack.org/cgit/openstack/nova/commit/?id=ecf74d4c0a5a8a4290ecac048fc437dafe3d40fc >> is the likely culprit, which would mean that only Kilo is affected. Can you >> confirm? >> >> Thanks, > > Yes, that looks like the problematic patch. I'd rather actually revert > that patch instead. > > Also, real tests would be nice to actually prevent future regression. > > -Sean > Ok, we've done a straight revert here - https://review.openstack.org/#/c/164140/ I'm also working on a test enhancement that ensures that all the shadow tables except the one we believe should contain entries are empty. That seems to specifically expose and nail this bug. Needs some cleanup, but should be posted by midday. -Sean -- Sean Dague http://dague.net __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [nova] if by "archived" you mean, "wipes out your tables completely", then sure, it works fine
On 03/13/2015 05:54 AM, Thomas Herve wrote: > > [snip] >> If we assume that all of our tables are filled up with zeroes for those >> deleted columns, because that’s the default, this **wipes the whole table >> clean**. >> >> How do the tests pass? Well the tests are in test_db_api->ArchiveTestCase, >> and actually, they don’t. But they don’t fail every time, because the test >> suite here runs with a database that is almost completely empty anyway, so >> the broken archival routine doesn’t find too many rows to blow away except >> for the rows in “instance_types”, which it only finds sometimes because the >> tests are only running it with a small number of things to delete and the >> order of the tables is non-deterministic. >> >> I’ve posted the bug report at https://bugs.launchpad.net/nova/+bug/1431571 >> where I started out not knowing much about how this worked except that my >> tests were failing, and slowly stumbled my way to come to this conclusion. A >> patch is at https://review.openstack.org/#/c/164009/, where we look at the >> actual Python-side default. However I’d recommend that we just hardcode the >> zero here, since that’s how our soft-delete columns work. > > Hi Mike, > > Thanks for the investigation. I was wondering when that behavior was > introduced and it seems that > http://git.openstack.org/cgit/openstack/nova/commit/?id=ecf74d4c0a5a8a4290ecac048fc437dafe3d40fc > is the likely culprit, which would mean that only Kilo is affected. Can you > confirm? > > Thanks, Yes, that looks like the problematic patch. I'd rather actually revert that patch instead. Also, real tests would be nice to actually prevent future regression. -Sean -- Sean Dague http://dague.net __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [nova] if by "archived" you mean, "wipes out your tables completely", then sure, it works fine
[snip] > If we assume that all of our tables are filled up with zeroes for those > deleted columns, because that’s the default, this **wipes the whole table > clean**. > > How do the tests pass? Well the tests are in test_db_api->ArchiveTestCase, > and actually, they don’t. But they don’t fail every time, because the test > suite here runs with a database that is almost completely empty anyway, so > the broken archival routine doesn’t find too many rows to blow away except > for the rows in “instance_types”, which it only finds sometimes because the > tests are only running it with a small number of things to delete and the > order of the tables is non-deterministic. > > I’ve posted the bug report at https://bugs.launchpad.net/nova/+bug/1431571 > where I started out not knowing much about how this worked except that my > tests were failing, and slowly stumbled my way to come to this conclusion. A > patch is at https://review.openstack.org/#/c/164009/, where we look at the > actual Python-side default. However I’d recommend that we just hardcode the > zero here, since that’s how our soft-delete columns work. Hi Mike, Thanks for the investigation. I was wondering when that behavior was introduced and it seems that http://git.openstack.org/cgit/openstack/nova/commit/?id=ecf74d4c0a5a8a4290ecac048fc437dafe3d40fc is the likely culprit, which would mean that only Kilo is affected. Can you confirm? Thanks, -- Thomas __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [nova] if by "archived" you mean, "wipes out your tables completely", then sure, it works fine
The archiving has issues since very long time [1], something like this [2] is expected to replace it. The archiving just move trash to the other side of the desk, usually just permanently deleting everything what is deleted for more than 7 day is better for everyone. For now, maybe just wiping out the shadow tables and the existing nova-mange functionality is better choice. [3] [1] https://bugs.launchpad.net/nova/+bug/1305892 [2] https://blueprints.launchpad.net/nova/+spec/db-purge-engine [3] https://bugs.launchpad.net/nova/+bug/1426873 - Original Message - > From: "Mike Bayer" > To: "OpenStack Development Mailing List (not for usage questions)" > > Sent: Friday, March 13, 2015 12:29:55 AM > Subject: [openstack-dev] [nova] if by "archived" you mean, "wipes out your > tables completely", then sure, it works > fine > > Hello Nova - > > Not sure if I’m just staring at this for too long, or if > archive_deleted_rows_for_table() is just not something we ever use. > Because it looks like it’s really, really broken very disastrously, and I’m > wondering if I’m just missing something in front of me. > > Let’s look at what it does! > > First, archive_deleted_rows() calls it with a table name. These names are > taken by collecting every single table name from nova.db.sqlalchemy.models. > > Then, the function uses table reflection (that is, doesn’t look in the model > at all, just goes right to the database) to load the table definitions: > > table = Table(tablename, metadata, autoload=True) > shadow_tablename = _SHADOW_TABLE_PREFIX + tablename > rows_archived = 0 > try: > shadow_table = Table(shadow_tablename, metadata, autoload=True) > except NoSuchTableError: > # No corresponding shadow table; skip it. > return rows_archived > > this is pretty heavy handed and wasteful from an efficiency point of view, > and I’d like to fix this too, but let’s go with it. Now we have the two > tables. > > Then we do this: > > deleted_column = table.c.deleted > query_insert = sql.select([table], > deleted_column != deleted_column.default).\ > order_by(column).limit(max_rows) > query_delete = sql.select([column], > deleted_column != deleted_column.default).\ > order_by(column).limit(max_rows) > > We make some SELECT statements that we’re going to use to find “soft > deleted” rows, and these will be embedded into an INSERT > and a DELETE. It is trying to make a statement like “SELECT .. FROM > table WHERE deleted != ”, so that it finds rows where > “deleted” has been changed to something, e.g. the row was > soft deleted. > > But what’s the value of “deleted_default” ? Remember, all this > table knows is what the database just told us about it, because it only > uses reflection. Let’s see what the “deleted” column in a table like > instance_types looks like: > > MariaDB [nova]> show create table instance_types; > | instance_types | CREATE TABLE `instance_types` ( > `created_at` datetime DEFAULT NULL, > > … [omitted] ... > > `deleted` int(11) DEFAULT NULL, > ) > > The default that we get for this column is NULL. That is very interesting! > Because, if we look at the *Python-side value of deleted*, we see something > that is quite the opposite of NULL, e.g. a thing that is most certainly not > null: > > class SoftDeleteMixin(object): > deleted_at = Column(DateTime) > deleted = Column(Integer, default=0) > > See that zero there? That’s a ***Python-side default***. It is **not the > server default**!! You will **not** get it from reflection, the database has > no clue about it (oddly enough, this entire subject matter is fully > documented in SQLAlchemy’s documentation, and guess what, the docs are free! > Read them all you like, I won’t ask for a dime, no questions asked!). > > So, all of our INSERTS **will** put a zero, not NULL, into that column. > Let’s look in instance_types and see: > > MariaDB [nova]> select id, name, deleted from instance_types; > ++---+-+ > | id | name | deleted | > ++---+-+ > | 3 | m1.large | 0 | > | 1 | m1.medium | 0 | > | 7 | m1.micro | 0 | > | 6 | m1.nano | 0 | > | 5 | m1.small | 0 | > | 2 | m1.tiny | 0 | > | 4 | m1.xlarge | 0 | > ++---+-+ > 7 rows in set (0.00 sec) > > No NULLs. The value of non-deleted rows is zero. > > What does this all mean? > > It means, when this archival routine runs, it runs queries like this:
Re: [openstack-dev] [nova] if by "archived" you mean, "wipes out your tables completely", then sure, it works fine
On 03/12/2015 05:29 PM, Mike Bayer wrote: If we assume that all of our tables are filled up with zeroes for those deleted columns, because that’s the default, this **wipes the whole table clean**. How do the tests pass? Well the tests are in test_db_api->ArchiveTestCase, and actually, they don’t. But they don’t fail every time, because the test suite here runs with a database that is almost completely empty anyway, so the broken archival routine doesn’t find too many rows to blow away except for the rows in “instance_types”, which it only finds sometimes because the tests are only running it with a small number of things to delete and the order of the tables is non-deterministic. I’ve posted the bug report at https://bugs.launchpad.net/nova/+bug/1431571 where I started out not knowing much about how this worked except that my tests were failing, and slowly stumbled my way to come to this conclusion. A patch is at https://review.openstack.org/#/c/164009/, where we look at the actual Python-side default. However I’d recommend that we just hardcode the zero here, since that’s how our soft-delete columns work. Nice detective work. I imagine there was some hair pulling on that one... Chris __ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
[openstack-dev] [nova] if by "archived" you mean, "wipes out your tables completely", then sure, it works fine
Hello Nova - Not sure if I’m just staring at this for too long, or if archive_deleted_rows_for_table() is just not something we ever use. Because it looks like it’s really, really broken very disastrously, and I’m wondering if I’m just missing something in front of me. Let’s look at what it does! First, archive_deleted_rows() calls it with a table name. These names are taken by collecting every single table name from nova.db.sqlalchemy.models. Then, the function uses table reflection (that is, doesn’t look in the model at all, just goes right to the database) to load the table definitions: table = Table(tablename, metadata, autoload=True) shadow_tablename = _SHADOW_TABLE_PREFIX + tablename rows_archived = 0 try: shadow_table = Table(shadow_tablename, metadata, autoload=True) except NoSuchTableError: # No corresponding shadow table; skip it. return rows_archived this is pretty heavy handed and wasteful from an efficiency point of view, and I’d like to fix this too, but let’s go with it. Now we have the two tables. Then we do this: deleted_column = table.c.deleted query_insert = sql.select([table], deleted_column != deleted_column.default).\ order_by(column).limit(max_rows) query_delete = sql.select([column], deleted_column != deleted_column.default).\ order_by(column).limit(max_rows) We make some SELECT statements that we’re going to use to find “soft deleted” rows, and these will be embedded into an INSERT and a DELETE. It is trying to make a statement like “SELECT .. FROM table WHERE deleted != ”, so that it finds rows where “deleted” has been changed to something, e.g. the row was soft deleted. But what’s the value of “deleted_default” ? Remember, all this table knows is what the database just told us about it, because it only uses reflection. Let’s see what the “deleted” column in a table like instance_types looks like: MariaDB [nova]> show create table instance_types; | instance_types | CREATE TABLE `instance_types` ( `created_at` datetime DEFAULT NULL, … [omitted] ... `deleted` int(11) DEFAULT NULL, ) The default that we get for this column is NULL. That is very interesting! Because, if we look at the *Python-side value of deleted*, we see something that is quite the opposite of NULL, e.g. a thing that is most certainly not null: class SoftDeleteMixin(object): deleted_at = Column(DateTime) deleted = Column(Integer, default=0) See that zero there? That’s a ***Python-side default***. It is **not the server default**!! You will **not** get it from reflection, the database has no clue about it (oddly enough, this entire subject matter is fully documented in SQLAlchemy’s documentation, and guess what, the docs are free! Read them all you like, I won’t ask for a dime, no questions asked!). So, all of our INSERTS **will** put a zero, not NULL, into that column. Let’s look in instance_types and see: MariaDB [nova]> select id, name, deleted from instance_types; ++---+-+ | id | name | deleted | ++---+-+ | 3 | m1.large | 0 | | 1 | m1.medium | 0 | | 7 | m1.micro | 0 | | 6 | m1.nano | 0 | | 5 | m1.small | 0 | | 2 | m1.tiny | 0 | | 4 | m1.xlarge | 0 | ++---+-+ 7 rows in set (0.00 sec) No NULLs. The value of non-deleted rows is zero. What does this all mean? It means, when this archival routine runs, it runs queries like this: INSERT INTO shadow_quota_usages SELECT quota_usages.created_at, quota_usages.updated_at, quota_usages.deleted_at, quota_usages.id, quota_usages.project_id, quota_usages.resource, quota_usages.in_use, quota_usages.reserved, quota_usages.until_refresh, quota_usages.deleted, quota_usages.user_id FROM quota_usages WHERE quota_usages.deleted IS NOT NULL ORDER BY quota_usages.id LIMIT ? OFFSET ? 2015-03-12 17:01:01,218 INFO [sqlalchemy.engine.base.Engine] (7, 0) 2015-03-12 17:01:01,219 INFO [sqlalchemy.engine.base.Engine] DELETE FROM quota_usages WHERE quota_usages.id in (SELECT T1.id FROM (SELECT quota_usages.id FROM quota_usages WHERE quota_usages.deleted IS NOT NULL ORDER BY quota_usages.id LIMIT ? OFFSET ?) as T1) The second query is to DELETE rows from a table like quota_usages based on looking at rows where the “deleted” column is “NOT NULL”. Which means, all of them! They are all zeros, not NULL! If we assume that all of our tables are filled up with zeroes for those deleted columns, because that’s the default, this **wipes the whole table clean**. How do the tests pass? Well the tests are in test_db_api->ArchiveTestCase, and actually, they don’t. But they don’t fail every time, because the test suite here runs with a database that is almost completely empty anyway, so the broken archival routine doesn’t find too many rows to blow away except for the rows in “i
