<div dir="ltr"><div class="gmail_extra">I would prefer to pick and choose which parts of oslo common db code to reuse in glance. Most parts there look great and very useful. However, some parts seem like they would conflict with several goals we have.</div>
<div class="gmail_extra"><br></div><div class="gmail_extra">1) To improve code sanity, we need to break away from the idea of having one giant db api interface</div><div class="gmail_extra">2) We need to improve our position with respect to new, non SQL drivers</div>
<div class="gmail_extra"> - mostly, we need to focus first on removing business logic (especially authz) from database driver code</div><div class="gmail_extra"> - we also need to break away from the strict functional interface, because it limits our ability to express query filters and tends to lump all filter handling for a given function into a single code block (which ends up being defect-rich and confusing as hell to reimplement)</div>
<div class="gmail_extra">3) It is unfortunate, but I must admit that Glance's code in general is pretty heavily coupled to the database code and in particular the schema. Basically the only tool we have to manage that problem until we can fix it is to try to be as careful as possible about how we change the db code and schema. By importing another project, we lose some of that control. Also, even with the copy-paste model for oslo incubator, code in oslo does have some of its own reasons to change, so we could potentially end up in a conflict where glance db migrations (which are operationally costly) have to happen for reasons that don't really matter to glance.</div>
<div class="gmail_extra"><br></div><div class="gmail_extra">So rather than framing this as "glance needs to use oslo common db code", I would appreciate framing it as "glance database code should have features X, Y, and Z, some of which it can get by using oslo code." Indeed, I believe in IRC we discussed the idea of writing up a wiki listing these feature improvements, which would allow a finer granularity for evaluation. I really prefer that format because it feels more like planning and less like debate :-)</div>
<div class="gmail_extra"><br></div><div class="gmail_extra"> I have a few responses inline below.</div><div class="gmail_extra">
<br><div class="gmail_quote">On Fri, Aug 16, 2013 at 6:31 AM, Victor Sergeyev <span dir="ltr"><<a href="mailto:vsergeyev@mirantis.com" target="_blank">vsergeyev@mirantis.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div dir="ltr"><div>Hello All.</div><div><br></div><div>Glance cores (Mark Washenberger, Flavio Percoco, Iccha Sethi) have some questions about Oslo DB code, and why is it so important to use it instead of custom implementation and so on. As there were a lot of questions it was really hard to answer on all this questions in IRC. So we decided that mailing list is better place for such things.</div>
<div><br></div><div>List of main questions:</div><div><br></div><div>1. What includes oslo DB code? </div><div>2. Why is it safe to replace custom implementation by Oslo DB code? </div><div>3. Why oslo DB code is better than custom implementation?</div>
<div>4. Why oslo DB code won’t slow up project development progress?</div><div>5. What we are going actually to do in Glance?</div><div>6. What is the current status?</div><div><br></div><div>Answers:</div><div><br></div>
<div>1. What includes oslo DB code?</div><div><br></div><div>Currently Oslo code improves different aspects around DB:</div><div>-- Work with SQLAlchemy models, engine and session</div><div>-- Lot of tools for work with SQLAlchemy </div>
</div></blockquote><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">
<div>-- Work with unique keys</div><div>-- Base test case for work with database</div><div>-- Test migrations against different backends</div><div>-- Sync DB Models with actual schemas in DB (add test that they are equivalent)</div>
<div><br></div><div><br></div><div>2. Why is it safe to replace custom implementation by Oslo DB code? </div><div><br></div><div>Oslo module, as base openstack module, takes care about code quality. Usually, common code more readable (most of flake8 checks enabled in Oslo) and have better test coverage. Also it was tested in different use-cases (in production also) in an other projects so bugs in Oslo code were already fixed. So we can be sure, that we use high-quality code.</div>
</div></blockquote><div><br></div><div>Alas, while testing and static style analysis are important, they are not the only relevant aspects of code quality. Architectural choices are also relevant. The best reusable code places few requirements on the code that reuses it architecturally--in some cases it may make sense to refactor oslo db code so that glance can reuse the correct parts.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">
<div><br></div><div><br></div><div>3. Why oslo DB code is better than custom implementation?</div><div><br></div><div>There are some arguments pro Oslo database code </div><div><br></div><div>-- common code collects useful features from different projects</div>
<div>Different utils, for work with database, common test class, module for database migration, and other features are already in Oslo db code. Patch on automatic retry db.api query if db connection lost on review at the moment. If we use Oslo db code we should not care, how to port these (and others - in the future) features to Glance - it will came to all projects automaticly when it will came to Oslo. </div>
<div><br></div><div>-- unified project work with database</div><div>As it was already said, It can help developers work with database in a same way in different projects. It’s useful if developer work with db in a few projects - he use same base things and got no surprises from them. </div>
</div></blockquote><div><br></div><div>I'm not very motivated by this argument. I rarely find novelty that challenging to understand when working with a project, personally. Usually I'm much more stumped when code is heavily coupled to other modules or too many responsibilities are lumped together in one module. In general, I'd take new ravioli code over familiar spaghetti code any day.</div>
<div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">
<div><br></div><div>-- it’s will reduce time for running tests.</div><div>Maybe it’s minor feature, but it’s also can be important. We can removed some tests for base `DB` classes (such as session, engines, etc) and replaced for work with DB to mock calls.</div>
</div></blockquote><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">
<div><br></div><div><br></div><div>4. Why oslo DB code won’t slow up project development progress?</div><div><br></div><div>Oslo code for work with database already in such projects as Nova, Neutron, Celiometer and Ironic. AFAIK, these projects development speed doesn’t decelerated (please fix me, If I’m wrong). Work with database level already improved and tested in Oslo project, so we can concentrate on work with project features. All features, that already came to oslo code will be available in Glance, but if you want to add some specific feature to project *just now* you will be able to do it in project code.</div>
</div></blockquote><div><br></div><div>I think the issue here for glance is whether or not oslo common code makes it easier or harder to make other planned improvements. In particular, using openstack.common.db.api will make it harder to refactor away from a giant procedural interface for the database driver.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">
<div><br></div><div><br></div><div>5. What we are going actually to do in Glance?</div><div><br></div><div>-- Improve test coverage of DB API layer</div><div>We are going to increase test coverage of glance/db/sqlalchemy/api module and fix bugs, if found. </div>
<div><br></div><div>-- Run DB API tests on all backends</div><div>-- Use Oslo migrations base test case for test migrations against different backends</div><div>There are lot of different things in SQl backends. For example work with casting.</div>
<div>In current SQLite we are able to store everything in column (with any type). Mysql will try to convert value to required type, and postgresql will raise IntegrityError. </div><div>If we will improve this feature, we will be sure, that all Glance DB migrations will run correctly on all backends.</div>
<div><br></div><div>-- Use Oslo code for SA models, engine and session</div><div>-- Use Oslo SA utils</div><div>Using common code for work with database was already discussed and approved for all projects. So we are going to implement common code for work with database instead of Glance implementation.</div>
</div></blockquote><div><br></div><div>I'm not sure what you mean by "discussed and approved for all projects". Isn't that discussion basically happening now for glance? </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div dir="ltr">
<div><br></div><div>-- Fix work with session and transactions</div><div>Our work items in Glance:</div><div>- don't pass session instances to public DB methods</div><div>- use explicit transactions only when necessary</div>
<div>- fix incorrect usage of sessions throughout the DB-related code</div><div><br></div><div>-- Optimize methods</div><div>When we will have tests for all functions in glance/db/sqlalchemy/api module it’s will be safe to refactor api methods. It will make these functions more clean, readable and faster.</div>
<div><br></div><div>The main ideas are:</div><div>- identify and remove unused methods</div><div>- consolidate duplicate methods when possible</div><div>- ensure SQLAlchemy objects are not leaking out of the API</div><div>
- ensure related methods are grouped together and named consistently</div><div><br></div><div>-- Add missing unique constraints</div><div>We should provide missed unique constraints, based on database queries from glance.db.sqlalchemy.api module. It’s will reduce data duplication and became one more step to Glance database normalization.</div>
<div><br></div><div>-- Sync models definitions with DB migrate scripts</div><div>At the moment we do not use Glance db models for db creation. To create db we use migrations. We don't have any tests that checks, that our models are up-to-date. Also we are testing it only on sqlite, which couldn't test such things as nullable constraints.</div>
<div>So we plain fix all mistakes in models and migration, sync effects of migrations in different backends and add tests that ensures that models are up-to-date.</div><div>-- Use alembic for database migrations </div><div>
SQLAlchemy-migrate (Glance database migration tool) is fine for straight-line migrations, but does not really support branching, needed for clean backports of some but not all migrations to a stable branch. Also this project doesn't seems to be live.</div>
<div>Alembic has better branching support and will meet our needs better. We should switch migration frameworks, and convert our existing migration scripts to Alembic syntax.</div><div><br></div><div>-- DB Reconnect</div>
<div>There are a variety of circumstances which can cause a transient failure in database connections, for example: restart / upgrade of the database, just a network failure, etc. Glance (as all projects connecting to a database) would benefit from the db/api catching these "db-has-gone-away" errors and automatically reconnecting and retrying the last db api method call. It is not necessary to abort long-running operations (such as glance image-create) just because of a momentary interruption in db connectivity.</div>
<div><br></div><div>-- No downtime database upgrade</div><div>No comments</div><div><br></div><div><br></div><div>6. What is the current status?<br></div><div><br></div><div>-- Improve test coverage of DB API layer</div>
<div>
Not started</div><div><br></div><div>-- Run DB API tests on all backends</div><div>Not started</div><div><br></div><div>-- Use Oslo code for SA models, engine and session</div><div>Started</div><div><br></div><div>-- Use Oslo migrations base test case for test migrations against different backends</div>
<div>-- Use Oslo SA utils</div><div>In progress at the moment. During the implementation, such changes was made:</div><div>- DB layer code cleanup. We removed project-specific functions for work with DB layer in db.api. Now we use common Oslo code.</div>
<div>- Base class for DB models from common code was used, instead of project base model class.</div><div>- Refactored migration module. Also we plan to use common module for migrations from Oslo </div><div>- Race condition in migration 012 was fixed.</div>
<div>- Tests, based on module glance.tests.integration.legacy_functional.base use sqlite in memory now.</div><div>- Modified work with config - we use some config values from Oslo code.</div><div>- Removed some tests for base class for work with DB - functions for work with DB was already tested in Oslo project.</div>
<div>- Renamed unique constraints due to common name convention. </div><div>- Patched sqlalchemy-migrate to fix UC bugs in SQLite.</div><div><br></div><div>-- Fix work with session and transactions</div><div>Not started</div>
<div><br></div><div>-- Optimize methods</div><div>Not started</div><div><br></div><div>-- Add missing unique constraints</div><div>Not started</div><div><br></div><div>-- Sync models definitions with DB migrate scripts</div>
<div>Mostly done. </div><div><br></div><div>-- Use alembic for database migrations </div><div>Not started</div><div><br></div><div>-- DB Reconnect</div><div>On review in Oslo. We will be able to use this feature after we will fix work with session and transactions</div>
<div><br></div><div>Thanks, Victor.</div><div><br></div></div>
<br>_______________________________________________<br>
OpenStack-dev mailing list<br>
<a href="mailto:OpenStack-dev@lists.openstack.org" target="_blank">OpenStack-dev@lists.openstack.org</a><br>
<a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" target="_blank">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a><br>
<br></blockquote></div><br></div></div>