[DBIx-Class-Devel] [dbix-class] Disconnect after schema version check. (#57)

[Eric Miller]

With the original commit I was simply closing a redundant connection that in most cases is only used to check the schema version in the underlying 'dbix_class_schema_versions' table but remains open throughout the life a web server or daemon process when using the Schema::Versioned component.

The redundant connections became apparent troubleshooting bug reports of users exhausting database connections and having to increase the connection limit in our [Netdisco](https://metacpan.org/release/App-Netdisco) application when it should not have been necessary.  It was discovered that for every daemon or web server process there was a second idle connection with the last statement issued being 'SELECT me.version FROM dbix_class_schema_versions me ORDER BY installed DESC LIMIT $1'. 

Looking into Schema::Versioned a complete schema object to include another connection is created in the primary schema as 'vschema ($self->{vschema})'.  This appears to have been in the code base since inception and was why I didn't originally modify it, just issuing a disconnect after the check.

However, as you indicated in the first review, we are ultimately connecting to the same database where the 'dbix_class_schema_versions' table exists.  It would be preferable to only connect to the database once, run the version check, and then use the existing connection for subsequent work to conserve system resources.

The second set of commits only uses one connection, except for when the 'dbix_class_schema_versions' table is created.  Within the original Schema::Versioned, deploy has been overridden to call an install method after any other deploy methods the inheritance stack.  The install method is a documented method which can be called on its own to install the 'dbix_class_schema_versions' table.  In the original code this is called via '$self->{vschema}->deploy;' limiting it to only installing the 'dbix_class_schema_versions' table.  The sqlt_deploy_hook was added to prevent the 'dbix_class_schema_versions' table from being dropped if { add_drop_table => 1 } is passed in \%sqlt_args to an inherited deploy method which is problematic because we would lose the version history and was caught in unit testing.  The _deploy_version_table method was added because it is not apparent how to call deploy for a single result class, which does not exist as a ddl file.  This should only be called 
 once in 
 the versioned database lifetime and so the redundant connection that is closed on completion should be acceptable.

All documented methods work as they did before these commits and the unit tests in '94versioning.t' and 'admin/02ddl.t' which exercise the api both pass all tests.  The only potential issue might be a name collision with 'register_class('Version::Table', 'DBIx::Class::Version::Table');', but this could be changed to a fully qualified source name if deemed necessary.

I'm uncertain why the class wasn't added to the schema in the past, could it have been that supporting methods weren't available to ensure that it wasn't touched during deploy?  The table exists in the underlying database.

It seems more clunky to register the class and then unregister source in multiple methods to try and achieve the same objective.

I would prefer a solution that only connects to the database once.  If you have thoughts on a better approach that would only connect to the database once I'm happy to try and implement it.  With this explanation, let me know which path you would like me to take.

---
Reply to this email directly or view it on GitHub:
https://github.com/dbsrgits/dbix-class/pull/57#issuecomment-54103071
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.scsys.co.uk/pipermail/dbix-class-devel/attachments/20140901/d8e96dd5/attachment.htm>