Support for a global database teardown

[evansde77]

Both DBS and Tier 0 will need an Oracle supporting version of the WMQuality/TestInit.py module to set up and tear down database instances for unitests.

CC'ing Lassi, since he may already have something along these lines for SiteDB.

[sfoulkes]

sfoulkes: TestInit is database agnostic. You just need to create DAOs to create and tear down your schema.

[PerilousApricot]

meloam: Except that it's not. With the FK relationships in oracle, it's super super easy to get stuck in a weird state where the SetUp/TearDown leaves the database in an inconsistent state where subsequent SetUp() fails because tables already exist, but then the TearDown DAOs also don't work. You need a way to get the database back to a clean state or it'll take a lot of manual intervention to get things running again.

For Mysql (WMQuality.TestInit), we have:

            formatter.sql = "SET foreign_key_checks = 0"
            query = "DROP TABLE IF EXISTS `%s`" % ("`,`".join(allTables))
            formatter.sql = "SET foreign_key_checks = 1"

With Oracle, we have: elif (dialect == 'Oracle'): print "Rookie, fix blowing away oracle in TestInit. thanks" pass

The MySQL version is much better :)

[sfoulkes]

sfoulkes: Your case doesn't work on MySQL either as a test that doesn't clean up properly will still cause subsequent tests to fail.

[hufnagel]

hufnagel: For Oracle you just need a generic kill all destroy DAO.

set serveroutput on size 100000 BEGIN -- Tables FOR o IN (SELECT table_name name FROM user_tables) LOOP dbms_output.put_line ('Dropping table ' || o.name || ' with dependencies'); execute immediate 'drop table ' || o.name || ' cascade constraints'; END LOOP;

-- Sequences FOR o IN (SELECT sequence_name name FROM user_sequences) LOOP dbms_output.put_line ('Dropping sequence ' || o.name); execute immediate 'drop sequence ' || o.name; END LOOP;

-- Triggers FOR o IN (SELECT trigger_name name FROM user_triggers) LOOP dbms_output.put_line ('Dropping trigger ' || o.name); execute immediate 'drop trigger ' || o.name; END LOOP;

-- Synonyms FOR o IN (SELECT synonym_name name FROM user_synonyms) LOOP dbms_output.put_line ('Dropping synonym ' || o.name); execute immediate 'drop synonym ' || o.name; END LOOP; END;

[PerilousApricot]

meloam: Replying to [comment:3 sfoulkes]:

Your case doesn't work on MySQL either as a test that doesn't clean up properly will still cause subsequent tests to fail.

Empirically, when I was spending a lot of time on getting the testing harness to play nice, it worked as advertised. When the test harness tries to call setDatabaseConnection, it immediately blows away every table in the database before setSchema has a chance to get called. If you use the tempDir stuff in TestInit, you're also writing to a directory that guaranteed to be empty with each test case. If you'd like to clarify where I screwed up my logic, I can toss a patch your way

Replying to [comment:4 hufnagel]: If that raw string got passed to the SQL layer, would all the magic get done with removing key constraints, etc? (This is what I tried to write once, but failed because I don't know a damned thing about oracle)

[ghost]

lat: I can't see any of this getting done what I would call "properly" with code like quoted above. Creating "fully designed" Oracle schema is a great deal more than what you have above.

I don't have anything for SiteDB (V2) at the moment, but when I do, it will look a great deal like what existed for PhEDEx seven years ago, except probably done in python in some way or another. I still think what exists in PhEDEx is vastly superior compared to anything else I've seen in our type of apps since then. Or put another way, I don't see above type of code solving any of the large number of issues that need to (and were) solved for PhEDEx, and what I would call "serious database work".

Note that PhEDEx has tools to "reset" the schema to its original state, and tools for cloning the schema, and tools for moving a schema aside and recreating a new in place, and migrating contents between the two clones. The reset part is guaranteed to yield a result similar to reloading the schema, to the extent it's possible to provide that guarantee with Oracle (= there will be few lingering table/partition storage effects; most tests do not need to worry about them). For certain types of highly demanding performance tests those guarantees are still too weak, and what you need to do is actually drop the database structure entirely and recreate it. Obviously, even the latter is fully scripted.

For SiteDB, all I have is oracle schema file which doesn't even load. I've hacked at it locally so it actually loads, and I've created a dev schema using that and the app appears to work, but I have no idea if it corresponds to the actual schema in the database or not -- it obviously wasn't used to load the production schema that exists. Among the things still on my to-do list is to take the PhEDEx schema tools and port them over to SiteDB, and use them to dump the schema from production and my dev databases, and check for any differences.

Sorry to sound negative, but I am not interested in pfaffing around with toys, when I know exactly what is needed, don't see it around, and did it all at least twice (for PhEDEx and DBS) myself, several years ago. So when I get to it for SiteDB, I'll go back to what I know works, and works well, and scales for what I call do things "properly" -- even if SiteDB doesn't need it all. I fully acknowledge it's entirely possible some of the apps you need to build have no such needs, and what exists is adequate. Perhaps you can beat on the code above to make it do what you need, and probably can be happy with that.

[ghost]

lat: Replying to [comment:5 meloam]:

If that raw string got passed to the SQL layer, would all the magic get done with removing key constraints, etc?

That's what "drop table foo cascade constraints" does. The bit Dirk quoted is basically a clone of some of the things in PhEDEx. Which schema objects you need to drop depends on what the schema contains.

However there are differences in whether you actually want to drop the tables and recreate them, or just nuke all the contents. It varies by the tests you do; I'd say normally you should just empty the schema, not recreate it.

Yes you can grab the entire bit of code and execute it as a single massive operation -- I think. It's PL/SQL (a procedure or program), not a single SQL statement. Though normally those things get executed with sqlplus. I think the "set serveroutput xx size N" are sqlplus options for sure.

[PerilousApricot]

meloam: Replying to [comment:7 lat]:

However there are differences in whether you actually want to drop the tables and recreate them, or just nuke all the contents. It varies by the tests you do; I'd say normally you should just empty the schema, not recreate it.

The use-case is for unit testing. Each test case is supposed to set up the schema, do the work then tear it back down again (leaving a blank database), but if the tear down fails (which happens occasionally), the subsequent SetUp commands start failing and things are in a gross state. Even worse, sometimes the create commands wouldn't bomb when a table already existed, so there were subsequent test cases using previous data which caused a heisenbug or two (running the test by itself would be okay, but running the whole suite, assertions would fail). To fix that, I added logic to the test harness that would completely destroy a database immediately after getting a connection. So far, we don't have a way to do it for oracle.

Basically, we need a fragment that will get as close as possible to a "clean" database.

Yes you can grab the entire bit of code and execute it as a single massive operation -- I think. It's PL/SQL (a procedure or program), not a single SQL statement. Though normally those things get executed with sqlplus. I think the "set serveroutput xx size N" are sqlplus options for sure.

I wonder if sqlalchemy will let us pass PL/SQL through.

[ghost]

lat: Adding to my previous comment: a) you can find all the things to delete from the schema tables (user_tables, user_constraints, user_synonyms, etc.), and b) things get complicated when you have _admin, _writer, *_reader accounts, and synonyms between them, and grants. For the latter, even in normal circumstances you need to use at least three different accounts (and passwords) to remove / install the schema. You can't just login to one account, say the writer, and run the drop all synonyms, then reload the schema - that will not do what you want. Add to that if the app uses 'alter session set current_schema' (or sets current_schema property on cx-oracle database connection handle), and/or schema prefixes in sql, and it gets even more complicated.

[ghost]

lat: Replying to [comment:8 meloam]:

The use-case is for unit testing.

Yeah, I realised, but see the comment I added above. It's still not obvious if you should do the tests in single-account schema, or multi-account schema, and what app does with selecting schema it uses at run time. I suspect you'll initially end up choosing to use a single account devdb schema for unit tests, but I also suspect you'll end up reverting on that decision later because you'll find it misses out on too many bugs.

[Test should set up, run, tear down schema]

Yeah, except setting up real schema in Oracle isn't quite that simple. I am not sure you want to do that for every unit test. Most likely you will want to create the schema just once, and reset just the contents for every test (to empty, or some preload data set). Testing schema migrations and company is a different realm.

All this is basically saying that schema in properly done oracle database apps tends to be a great deal more complicated beast than you may have been used to when using mysql databases, so you may have to rethink the testing strategy - and invent new types of tests.

[When schema tear down fails]

I'd say that's just bad quality, like any bad quality. Require devs to provide robust schema initialisation/removal/empty tools, and don't put up with any odd crap. Just flag them as schema failures, fail the patches, and ask the devs to accompany the changes with required modifications to schema side.

Then again, I will acknowledge I have slightly different take on producing end-to-end quality, with deployment and schema management at play, than your average developer... That said, I wouldn't put up with substandard schema management any more than I wouldn't up with other bugs in the code.

To fix that, I added logic to the test harness that would completely destroy a database immediately after getting a connection. So far, we don't have a way to do it for oracle.

And you will shortly discover you can't :-) At least not if the schema resembles anything we actually will do in production. At the very least you'll need to deal with three accounts, not just one.

Basically, we need a fragment that will get as close as possible to a "clean" database.

For this, as I said before, you'll likely want to just empty the database. Unsurprisingly, PhEDEx has a script to do that :-)

I wonder if sqlalchemy will let us pass PL/SQL through.

Possibly, I have no idea since I've never used sqlalchemy. I doubt it cares, for app it normally looks like any odd SQL statement execution. Only the server knows. (But that said, I've not tried to execute multi-statement PL/SQL from an app, only from sqlplus, and I don't know what magic the latter does to break it all apart.)

[ghost]

lat: Here's what you do to empty the schema entirely. Note that it requires schema designer to be militant in naming their schema objects systematically. I'd make that a requirement anyway, but hey, that's just me.

{{{

!sql

---

-- Drop all schema objects.

set serveroutput on size 100000 BEGIN -- Disable constraints: first foreign keys, then others FOR o IN (SELECT table_name, constraint_name FROM user_constraints WHERE constraint_name LIKE 'FK%') LOOP dbms_output.put_line ('Disabling constraint ' || o.constraint_name || ' on ' || o.table_name); execute immediate 'alter table ' || o.table_name || ' disable constraint ' || o.constraint_name; END LOOP;

-- Tables FOR o IN (SELECT table_name name FROM user_tables) LOOP dbms_output.put_line ('Emptying table ' || o.name); execute immediate 'truncate table ' || o.name || ' drop storage'; END LOOP;

-- Re-enable constraints FOR o IN (SELECT table_name, constraint_name FROM user_constraints WHERE constraint_name LIKE 'FK%') LOOP dbms_output.put_line ('Enabling constraint ' || o.constraint_name || ' on ' || o.table_name); execute immediate 'alter table ' || o.table_name || ' enable constraint ' || o.constraint_name; END LOOP; END; / }}}

[ghost]

lat: And here's what PhEDEx does to delete all schema objects. Note that you need to purge recyclebin at certain points in some cases. (Yes, really, oracle has a recycle bin.) Again, which schema objects you need to drop varies, and this relies on certain naming conventions. For example if the schema has functions, etc., you need to make sure you cover everything.

{{{

!sql

---

-- Drop all schema objects.

set serveroutput on size 100000

BEGIN execute immediate 'purge recyclebin';

-- Tables FOR o IN (SELECT table_name name FROM user_tables WHERE tablename like 'T%') LOOP dbms_output.put_line ('Dropping table ' || o.name || ' with dependencies'); execute immediate 'drop table ' || o.name || ' cascade constraints'; END LOOP;

-- Sequences FOR o IN (SELECT sequence_name name FROM user_sequences WHERE sequencename like 'SEQ%') LOOP dbms_output.put_line ('Dropping sequence ' || o.name); execute immediate 'drop sequence ' || o.name; END LOOP;

execute immediate 'purge recyclebin';

-- Triggers FOR o IN (SELECT trigger_name name FROM user_triggers) LOOP dbms_output.put_line ('Dropping trigger ' || o.name); execute immediate 'drop trigger ' || o.name; END LOOP;

-- Synonyms FOR o IN (SELECT synonym_name name FROM user_synonyms) LOOP dbms_output.put_line ('Dropping synonym ' || o.name); execute immediate 'drop synonym ' || o.name; END LOOP;

execute immediate 'purge recyclebin'; END; / }}}

[ghost]

lat: Here's another variant of the above. Again, things vary depending on what the goal is.

{{{

!sql

---

-- Drop all schema objects.

set serveroutput on size 100000 BEGIN -- Tables FOR o IN (SELECT table_name name FROM user_tables) LOOP dbms_output.put_line ('Dropping table ' || o.name || ' with dependencies'); execute immediate 'drop table ' || o.name || ' cascade constraints'; END LOOP;

-- Sequences FOR o IN (SELECT sequence_name name FROM user_sequences) LOOP dbms_output.put_line ('Dropping sequence ' || o.name); execute immediate 'drop sequence ' || o.name; END LOOP;

-- Triggers FOR o IN (SELECT trigger_name name FROM user_triggers) LOOP dbms_output.put_line ('Dropping trigger ' || o.name); execute immediate 'drop trigger ' || o.name; END LOOP;

-- Functions FOR o IN (SELECT object_name FROM user_objects WHERE object_type = 'FUNCTION') LOOP dbms_output.put_line ('Dropping function ' || o.object_name); execute immediate 'drop function ' || o.object_name; END LOOP;

-- Synonyms FOR o IN (SELECT synonym_name name FROM user_synonyms) LOOP dbms_output.put_line ('Dropping synonym ' || o.name); execute immediate 'drop synonym ' || o.name; END LOOP; END; / }}}

[ghost]

lat: Here's how you revert a schema.

{{{

!sql

---

-- Copy all tables, constraints, indices, sequences and triggers. -- All the object names are prefixed with letter "X" to move the old -- schema out of the way of the new one.

set serveroutput on size 100000 BEGIN

---

-- Tables FOR o IN (SELECT table_name name FROM user_tables WHERE table_name LIKE 'X%') LOOP dbms_output.put_line ('Copying table ' || o.name); execute immediate 'rename ' || o.name || ' to ' || substr (o.name, 2, 29); END LOOP;

---

-- Sequences FOR o IN (SELECT sequence_name name FROM user_sequences WHERE sequence_name LIKE 'X%') LOOP dbms_output.put_line ('Renaming sequence ' || o.name); execute immediate 'rename ' || o.name || ' to ' || substr (o.name, 2, 29); END LOOP;

---

-- Constraints FOR o IN (SELECT constraint_name name, table_name FROM user_constraints WHERE constraint_name LIKE 'X%' AND constraint_name NOT LIKE 'SYS%') LOOP dbms_output.put_line ('Renaming constraint ' || o.name || ' [' || o.table_name || ']'); execute immediate 'alter table ' || o.table_name || ' rename constraint ' || o.name || ' to ' || substr (o.name, 2, 29); END LOOP;

---

-- Indices FOR o IN (SELECT index_name name, table_name FROM user_indexes WHERE index_name LIKE 'X%' AND index_name NOT LIKE 'SYS%') LOOP dbms_output.put_line ('Renaming index ' || o.name || ' [' || o.table_name || ']'); execute immediate 'alter index ' || o.name || ' rename to ' || substr (o.name, 2, 29); END LOOP;

---

-- Triggers FOR o IN (SELECT trigger_name name, table_name FROM user_triggers WHERE trigger_name LIKE 'X%') LOOP dbms_output.put_line ('Renaming trigger ' || o.name || ' [' || o.table_name || ']'); execute immediate 'alter trigger ' || o.name || ' rename to ' || substr (o.name, 2, 29); END LOOP; END; / }}}

[ghost]

lat: Here's how you stash a schema (which is what the above reverts).

{{{

!sql

---

-- Rename all tables, constraints, indices, sequences and triggers. -- All the object names are prefixed with letter "X" to move the old -- schema out of the way of the new one.

set serveroutput on size 100000 BEGIN

---

-- X-Tables

-- Drop all the X% tables first, just for fun? -- FOR o IN -- (SELECT table_name name FROM user_tables -- WHERE table_name LIKE 'X%') -- LOOP -- dbms_output.put_line ('Dropping table ' || o.name); -- execute immediate 'drop table ' || o.name || ' cascade constraints'; -- END LOOP;

-- Tables FOR o IN (SELECT table_name name FROM user_tables WHERE table_name NOT LIKE 'X%') LOOP dbms_output.put_line ('Renaming table ' || o.name); -- execute immediate 'drop table X' || o.name; execute immediate 'rename ' || o.name || ' to X' || substr (o.name, 1, 29); END LOOP;

---

-- Sequences -- Drop X% sequences too -- FOR o IN -- (SELECT sequence_name name FROM user_sequences -- WHERE sequence_name LIKE 'X%') -- LOOP -- dbms_output.put_line ('Dropping sequence ' || o.name); -- execute immediate 'drop sequence ' || o.name; -- END LOOP;

FOR o IN (SELECT sequence_name name FROM user_sequences WHERE sequence_name NOT LIKE 'X%') LOOP dbms_output.put_line ('Renaming sequence ' || o.name); execute immediate 'rename ' || o.name || ' to X' || substr (o.name, 1, 29); END LOOP;

---

-- Constraints FOR o IN (SELECT constraint_name name, table_name FROM user_constraints WHERE constraint_name NOT LIKE 'X%' AND constraint_name NOT LIKE 'SYS%') LOOP dbms_output.put_line ('Renaming constraint ' || o.name || ' [' || o.table_name || ']'); execute immediate 'alter table ' || o.table_name || ' rename constraint ' || o.name || ' to X' || substr (o.name, 1, 29); END LOOP;

---

-- Indices FOR o IN (SELECT index_name name, table_name FROM user_indexes WHERE index_name NOT LIKE 'X%' AND index_name NOT LIKE 'SYS%') LOOP dbms_output.put_line ('Renaming index ' || o.name || ' [' || o.table_name || ']'); execute immediate 'alter index ' || o.name || ' rename to X' || substr (o.name, 1, 29); END LOOP;

---

-- Triggers FOR o IN (SELECT trigger_name name, table_name FROM user_triggers WHERE trigger_name NOT LIKE 'X%') LOOP dbms_output.put_line ('Renaming trigger ' || o.name || ' [' || o.table_name || ']'); execute immediate 'alter trigger ' || o.name || ' rename to X' || substr (o.name, 1, 29); END LOOP; END; / }}}

[ghost]

lat: Depending on how you empty data from database, you may or may need to run something like this to reclaim unused table space.

{{{

!sql

---

-- These statements compact space in the tables that undergo most heavy -- insertion/deletion rates. You will want to run this every once in a -- while, especially when large changes have occurred, such as archiving -- of transfer history, completion of significant file transfers, or the

-- deactivation of large numbers of file blocks.

-- Remember to update table statistics afterwards.

-- This stuff is Oracle 10g only. For an explanation on how this could -- perhaps be done in 91 via dbms_redefinition package, please see see -- http://www.oracle-base.com/articles/9i/HighAvailabilityEnhancements9i.php

alter table t_transfer_tracking shrink space compact cascade; alter table t_transfer_tracking history shrink space cascade;

alter table t_transfer_history shrink space compact cascade; alter table t_transfer_history shrink space cascade;

alter table t_replica_state shrink space compact cascade; alter table t_replica_state shrink space cascade;

alter table t_transfer_state shrink space compact cascade; alter table t_transfer_state shrink space cascade;

alter table t_transfer_completed shrink space compact cascade; alter table t_transfer_completed shrink space cascade;

-- Can do separately (if "cascade" not used above): -- alter index ix_x coalesce; -- alter index ix_x rebuild pctfree 0 online; }}}

[ghost]

lat: And while you are in it, throw this in and fail any schema which produces any output.

{{{

!sql

select position, constraint_name, column_name from user_cons_columns cc where exists (select 1 from user_constraints c where c.constraint_type = 'R' and c.table_name = cc.table_name and c.constraint_name = cc.constraint_name) and not exists (select 1 from user_ind_columns i where i.table_name = cc.table_name and i.column_name = cc.column_name and i.column_position = cc.position) order by table_name, constraint_name; }}}

[hufnagel]

hufnagel: Renaming the ticket, as TestInit works just fine with Oracle. Also reassign to WMCore.

The solutions presented here to do a global database wipe are not really usable in the current WMCore schema teardown design because each module is only supposed to destroy it's own part of the schema. Doing more causes the next module in the chain to fail to destroy its part of the schema.

We might want to either extend this design to an "after all modules" step or replace it altogether with a global database wipe that is only run once and is only defined in WMCore and then used by everyone. Gradually building up a schema module by module makes sense, tearing it down in the same way maybe not so much.

[PerilousApricot]

meloam: Hey, I spoke to Dirk a bit offline. We decided that basically:

..replace it altogether with a global database wipe that is only run once and is only defined in WMCore and then used by everyone. Gradually building up a schema module by module makes sense, tearing it down in the same way maybe not so much

is the right way to do things. We need some sort of function that will accept a database connection and do whatever magic is required to do the right cleanup. Then, TestInit and whomever else (not sure who) will be able to use it.

[hufnagel]

hufnagel: Going ahead with this, is there any reason not to have that global takedown performed by WMCore.WMBS..Destroy ?

IMO it's the right place for it, unless we want to completely get rid of Destroy and just call this before any new schema setup (or even fully integrate it into the schema setup).

[sfoulkes]

sfoulkes: I'd vote for WMQuality..Destroy as this has nothing to do with WMBS. The code in WMQuality.TestInit.clearDatabase() will be calling it.

[hufnagel]

hufnagel: This is not just for unit tests, it's also for general schema deployment. As such, I do not really see why this should go into WMQuality.TestInit. Unit tests are just one user of this functionality, the real low level dependency is on the "setup a new schema" use case (which can only work if the db is empty).

What I was thinking about is to modify WMInit.py to call clearDatabase internally from the setSchema and initializeSchema methods.

Avoids having the same functionality (clean out a database) implemented twice in different places. Unit tests then would not need to care if the db is empty or not, because when they setUp a new schema, they'll clean out everything anyways.

[stuartw]

swakef: Replying to [comment:22 hufnagel]:

What I was thinking about is to modify WMInit.py to call clearDatabase internally from the setSchema and initializeSchema methods.

Avoids having the same functionality (clean out a database) implemented twice in different places. Unit tests then would not need to care if the db is empty or not, because when they setUp a new schema, they'll clean out everything anyways.

FWIW, +1

[sfoulkes]

sfoulkes: Replying to [comment:22 hufnagel]:

This is not just for unit tests, it's also for general schema deployment. As such, I do not really see why this should go into WMQuality.TestInit. Unit tests are just one user of this functionality, the real low level dependency is on the "setup a new schema" use case (which can only work if the db is empty).

Disagree, we should not automatically clean out the database before we deploy a schema. That's just asking for trouble.

[ghost]

lat: Replying to [comment:24 sfoulkes]:

Disagree, we should not automatically clean out the database before we deploy a schema. That's just asking for trouble.

I agree, this sort of thing escalates small problems into (very) big problems far too easily. For example if someone accidentally tries to load a schema into the wrong database instance, instead of getting errors on 'schema exists', the person ends up deleting the entire database.

The software design should not assume error-free operation, even if it should make standard operation easy and straightforward.

[hufnagel]

hufnagel: Ok, fine. But then unit tests should just fail when you run them on a non-empty database instead of calling their own clearDatabase at the beginning. The same argument applies there, can't have it both ways.

Which means we go back to my original plan, putting the global teardown into Destroy and you have to always call Destroy if you want to delete a database. And we of course completely remove any special clearDatabase code from the unit test framework.

I just want this treated in a consistent fashion and not have duplicated code. Which IMO basically leaves two options:

1) What I just suggested, tearing down a database is considered part of a new schema installation. And yes, this assumes that any existing data in a database we use is silently dropped, ie. the "you better know what you are doing" model.

2) Tearing down a database is only done at the end of unit tests as part of the cleanup. We never clean a database otherwise. This assumes that a database with existing content can in general not be used at all, you need to empty it before.

Btw, with a global teardown, even option 2 does not protect you against screwups. Yes, it will fail with schema exists errors if the same schema already exist. If you use a database that has an orthogonal schema though, you will happily install the WMBS schema into it, use it for unit tests and then teardown the whole schema globally.

To protect against that, we would explicitly have to include a check in the schema initialization that fails if there are any preexisting structures in the database.

[sfoulkes]

sfoulkes: Replying to [comment:26 hufnagel]:

Ok, fine. But then unit tests should just fail when you run them on a non-empty database instead of calling their own clearDatabase at the beginning. The same argument applies there, can't have it both ways.

I'm fine with this.

Btw, with a global teardown, even option 2 does not protect you against screwups. Yes, it will fail with schema exists errors if the same schema already exist. If you use a database that has an orthogonal schema though, you will happily install the WMBS schema into it, use it for unit tests and then teardown the whole schema globally.

To protect against that, we would explicitly have to include a check in the schema initialization that fails if there are any preexisting structures in the database.

This would be a good idea.

[hufnagel]

hufnagel: Attached is a working patch implementing a global teardown for Oracle. MySQL should remain functional as before.

The DBCreator call has a check for a non-empty database before it tries to setup a schema, if the db is non-empty, it dies. That check is then implemented in the Create plugins, for Oracle I added a check on user_objects. For MySQL I am not sure what to look at. Something like this might work

SELECT COUNT(*) AS count
FROM information_schema.tables
WHERE table_schema = (SELECT DATABASE())

but only looks at the table count in the default database. Not enough of an MySQL expert to know if there is something better.

For the MySQL global teardown, that's more tricky. Most recipes I found assume you want to do it from a shell command line, not from python. AFAICT, there is no way to do it in one shot MySQL SQL statement like in Oracle. Shortest is probably a "DROP DATABASE db_name" followed by a "CREATE DATABASE db_name", but I am not sure if that is usable (also would hardcode the db_name in code). Otherwise we would need to loop over the output of a query, which likely means some refactoring of the whole delete code (cannot be a sequence of independent calls anymore).

The only other functional change is removal of the call to clearDatabase() when setSchema() fails.

[DMWMBot]

mnorman: Okay, so I confirm that this works for what it does. I guess my question is, is this supposed to be a prototype for a later DBFormatter release? I thought we were developing something that would work to destroy the database regardless of what the database was, not just a change to WMBS.

If I'm wrong and all we want is for WMBS to work this way, this works great, but I don't see how this would help, say, ResourceControl or DBSBuffer tests. Was that part of this ticket?

[hufnagel]

hufnagel: ResourceControl and DBSBuffer just have their own separate schema, nothing else ? Yes, they would not benefit from this then. Hm, I do not want to duplicate code.

I wanted to do minimal changes to implement this, but I think it's better to change the clearDatabase() call and only ever have it call a single Destroy DAO, getting rid of the whole Destroy plugin system altogether. The clearDatabase() call calls a single DAQ which is defined in WMCore, no matter which and how many modules you have in your schema.

That would also mean that I can implement something sane for MySQL since the restriction to execute a sequence of not connected single SQL statement to teardown a database is gone and I can write something that first gets a list of all tables and then drops them.

For MySQL I would also appreciate feedback if purely looking at tables to determine non-emptiness and dropping all tables to clean the database is enough.

[stuartw]

swakef: Replying to [comment:30 hufnagel]:

For MySQL I would also appreciate feedback if purely looking at tables to determine non-emptiness and dropping all tables to clean the database is enough.

Personally I generally just do a drop database; create database - which as far as i can tell preserves permissions etc. From web searches i cant see any easy pure sql way of doing this:

Also, in TestInit.py, don't catch and rethrow (that rewrites the exception and removes the info about where in the db code it was raised). Either change "raise ex" to "raise" or better remove the try/except altogether.

[hufnagel]

hufnagel: The whole try/except section with clearDatabase in TestInit was already removed because it only applied to clearing the database on a schema install failure, which we do not want to do anymore. If there is content in the database and you try to use it, all the tests will fail before they can make any modifications to the database.

The problem I have with "DROP DATABASE" and "CREATE DATABASE" is that it needs a database name argument. I could query for the current database name of course, but in addition I would also have to make it the active database after recreating. Something like this:

SELECT DATABASE() => db_name DROP DATABASE db_name CREATE DATABASE db_name USE db_name

which makes it similar in complexity compared to just selecting all tables from the current database and dropping them.

There is also the complication that the mysql user might not have privileges to drop or create databases. You do not really need them to use the system. I already implemented something that loops over all tables and drops them, so I stick with that for the moment. We can always change later if the DROP/CREATE DATABASE method is preferred.

[hufnagel]

hufnagel: Please review, should be complete now. The MySQL changes are tested at the query, but not code level, since I do not have a MySQL setup to test. Just run a few unit tests that setup and teardown a database.

This is more invasive then I originally intended, but also cleaner.

PS: I had to comment out a UNIQUE constraint in the Oracle Create plugin because Oracle does not allow you to have two indexes on the same column list. There already was an index defined in CreateWMBSBase.py and the Oracle unique constraint is implemented via a unique index, hence the conflict. Will open another ticket for that later.

[DMWMBot]

mnorman: I see your patch and raise you my own...

There were a few problems that I found with your patch. The MySQL code didn't seem to work. A bigger problem is that when you check that the DB is empty, it depends on that Create code being run first. If you install another schema, like the Agent schema, this causes the WMBS schema to fail because the DB is not empty - and it means that the component unittests only work if things go in the right order.

I've redone the MySQL code, and moved both the Oracle and MySQL Delete code out of WMBS and into WMCore.Database. I've also moved the "Check database" code into WMQuality.TestInit so it's only called once, when we set up the DB connection.

I think that does everything we need to do, but I'm asking you to look at it to make sure I did what we wanted.

[PerilousApricot]

meloam: +1

[hufnagel]

hufnagel: Will look at it tomorrow in detail. Couple quick comments though:

Agree with moving the content check from the Create plugin into WMInit. Call the DAO something else though. It doesn't list tables, so why name it ListTables ? Call it DatabaseEmpty and return a boolean.

Second point, why do you use DROP DATABASE ? Seems it's more complicated than the DAO I specified since you need special code that has to work around the fact that the database was just removed/recreated. Also, I do not see any workarounds for dropping the current database. If you do that, you need to select to use the recreated one (it's not automatic).

[DMWMBot]

mnorman: I named it ListTables because if the database is not empty it means that something didn't get cleaned up properly. I thought it best to print out whatever didn't get cleaned up into the log stream so that someone would know instantly (really this is for things like Jenkins where the DB may not be preserved, so we might want to know who screwed up).

"Also, I do not see any workarounds for dropping the current database. If you do that, you need to select to use the recreated one (it's not automatic)."

I'm not quite sure what you mean by that. I choose the DROP/CREATE option because it seemed cleaner to me, especially when we want to delete the entire DB contents anyway. It also seemed more foolproof in that the DROP DATABASE is pretty robust, and I don't have to worry about a single table failing to delete somehow (and screwing everything up). Additionally, I don't quite trust setting the foreign key checks to OFF, mostly because we're having some problems with the DB right now in Jenkins where turning the FK constraints off may be staying in the system.

(On that note, we can talk about your code if you want to do it that way, but remember to turn the FKs back on at the end)

[hufnagel]

hufnagel: It's still not ListTables though, because for Oracle at least it could also be functions, procedures etc. Anything user created. Maybe call it ListUserContent or something.

Under what circumstances would we not preserve the database though ? Isn't that one of the points of this exercise, to not screw with the database if it contains anything ? At which point everything will fail and you can just check later what exactly is left over. I don't feel that strongly about it though, because under "normal" conditions, that check should return nothing, so there really isn't any overhead. It's only when something does go wrong that the query is more expensive and you get more output.

http://dev.mysql.com/doc/refman/5.0/en/drop-database.html

"If the default database is dropped, the default database is unset (the DATABASE() function returns NULL)."

[PerilousApricot]

meloam: >>Under what circumstances would we not preserve the database though ?

Automated unit testing. We're using that code to blow away the database between test cases.

[hufnagel]

hufnagel: Please review this version. Changed it mostly to what Matt suggested, except for:

1) ListTables -> ListUserContent 2) MySQL Destroy DAO discovers the dbName on it's own 3) MySQL Destroy DAO resets the current database to the newly created one 4) WMInit bails out in setDatabaseConnection if dialect is not mysql or oracle 5) WMInit.clearDatabase is simpler because of 2 and potentially 3 (resetting the connections might not be needed anymore) 6) WMInit.clearDatabase has fewer checks on whether the database connection is any good, don't think this is the place to test them (the should be tested earlier, like the dialect check I added in 4)

As for 6, just add the checks back if you feel there really is a good reason to have them. As for resetting the connections, just test it. I don't have a full WMAgent MySQL setup, but I played around with a CVS WMCore version and a MySQL backend (basically the ProdAgentDB in a Tie0setup) and this worked, ie. after going through the sequence of calls I put into the MySQL Destroy DAO I could still make queries with the same connection.

[hufnagel]

hufnagel: Replying to [comment:39 meloam]:

Under what circumstances would we not preserve the database though ?

Automated unit testing. We're using that code to blow away the database between test cases.

Ok, so the use case is a unit test baling out before it can call tearDown and you want the next unit test to start with a clean slate. That will still work, although what it'll do is to just call a regular clearDatabase before setting the schema.

In this environment we will blow away existing databases though, no matter what's in them. So "you better know what you are doing" when configuring the database to use for such tests.

[DMWMBot]

mnorman: Made my own adjustments.

I still choose to wrap the destroyDAO in its own transaction, mostly because it's several different transactions for the MySQL server, and I want it protected and not in a weird state if the connection goes down. Also did some exception handling for that case. Other then that I think the changes were just spelling stuff for MySQL.

See what you think.

[hufnagel]

hufnagel: Wrapping the destroy into a transaction is pretty useless because in MySQL any statements that define or modify database objects (alter table, drop table etc) cause an implicit commit.

Also, if we get a failure during the global teardown, we are pretty much hosed. At that point I'd rather prefer not to do anything further. Bail out with the error message, we'll take care of the rest after.

[hufnagel]

hufnagel: Also, what did you do to WMCore.Database..Destroy ? They are not present in your new patch.

[DMWMBot]

mnorman: Adding patch to remove transaction - must be applied on top of my other 100711 patch.

[hufnagel]

hufnagel: Steve, could you please review/commit ? I am ok with the last two patches from Matt combined.

[sfoulkes]

sfoulkes: (In 1330faa0cd1a77f5077a8552f5a2f86562edb00c) Support global database teardown. Fixes #2148.

From: Matt Norman mnorman@fnal.gov From: Dirk Hufngal hufnagel@cern.ch

[sfoulkes]

sfoulkes: (In 04490eba74b2507d374a4d2c4b0bfac409eccb54) Revert "Support global database teardown. Fixes #2148."

This reverts commit d1701a7d1a3dbea29850c845185a775ed497a052.

[sfoulkes]

sfoulkes: I backed this out as it kills the jenkins tests: http://dmwm.cern.ch:8080/job/WMCore-py2.6-mysql/299/

[sfoulkes]

sfoulkes: (In db6395bd02ddb5adb10ec1237828111857271e8e) Add back global database teardown with fixes from Matt. Fixes #2148.

From: Matt Norman mnorman@fnal.gov Signed-off-by: Steve Foulkes sfoulkes@fnal.gov