On Mon, Jul 24, 2017 at 5:10 PM, Octave J. Orgeron <[email protected]> wrote: > I don't think it makes sense to make these global. We don't need to change > all occurrences of String(255) to TinyText for example. We make that > determination through understanding the table structure and usage. But I do > like the idea of changing the second option to ndb_size=, I think that makes > things very clear. If you want to collapse the use cases.. what about?: > > oslo_db.sqlalchemy.String(255, ndb_type=TINYTEXT) -> VARCHAR(255) for most > dbs, TINYTEXT for ndb > oslo_db.sqlalchemy.String(4096, ndb_type=TEXT) -> VARCHAR(4096) for most > dbs, TEXT for ndb > oslo_db.sqlalchemy.String(255, ndb_size=64) -> VARCHAR(255) on most dbs, > VARCHAR(64) on ndb > > This way, we can override the String with TINYTEXT or TEXT or change the > size for ndb.
OK. See, originally when I was pushing for an ndb "dialect", that hook lets us say String(255).with_variant(TEXT, "ndb") which is what I was going for originally. However, since we went with a special flag and not a dialect, using ndb_type / ndb_size is *probably* fine. > >> >> oslo_db.sqlalchemy.String(255) -> VARCHAR(255) on most dbs, >> TINYTEXT() on ndb >> oslo_db.sqlalchemy.String(255, ndb_size=64) -> VARCHAR(255) on >> most dbs, VARCHAR(64) on ndb >> oslo_db.sqlalchemy.String(50) -> VARCHAR(50) on all dbs >> oslo_db.sqlalchemy.String(64) -> VARCHAR(64) on all dbs >> oslo_db.sqlalchemy.String(80) -> VARCHAR(64) on most dbs, TINYTEXT() >> on ndb >> oslo_db.sqlalchemy.String(80, ndb_size=55) -> VARCHAR(64) on most >> dbs, VARCHAR(55) on ndb >> >> don't worry about implementation, can the above declaration -> >> datatype mapping work ? >> >> Also where are we using AutoStringText(), it sounds like this is just >> what SQLAlchemy calls the Text() datatype? (e.g. an unlengthed >> string type, comes out as CLOB etc). >> > In my patch for Neutron, you'll see a lot of the AutoStringText() calls to > replace exceptionally long String columns (4096, 8192, and larger). MySQL supports large VARCHAR now, OK. yeah this could be String(8192, ndb_type=TEXT) as well. > > > > >> >> >>> In many cases, the use of these could be removed by simply changing the >>> columns to more appropriate types and sizes. There is a tremendous amount >>> of >>> wasted space in many of the databases. I'm more than willing to help out >>> with this if teams decide they would rather do that instead as the >>> long-term >>> solution. Until then, these functions enable the use of both with minimal >>> impact. >>> >>> Another thing to keep in mind is that the only services that I've had to >>> adjust column sizes for are: >>> >>> Cinder >>> Neutron >>> Nova >>> Magnum >>> >>> The other services that I'm working on like Keystone, Barbican, Murano, >>> Glance, etc. only need changes to: >>> >>> 1. Ensure that foreign keys are dropped and created in the correct order >>> when changing things like indexes, constraints, etc. Many services do >>> these >>> proper steps already, there are just cases where this has been missed >>> because InnoDB is very forgiving on this. But other databases are not. >>> 2. Fixing the database migration and sync operations to use oslo.db, pass >>> the right parameters, etc. Something that should have been done in the >>> first >>> place, but hasn't. So this more of a house cleaning step to insure that >>> services are using oslo.db correctly. >>> >>> The only other oddball use case is deal with disabling nested >>> transactions, >>> where Neutron is the only one that does this. >>> >>> On the flip side, here is a short list of services that I haven't had to >>> make ANY changes for other than having oslo.db 4.24 or above: >>> >>> aodh >>> gnocchi >>> heat >>> ironic >>> manila >>> >>>> 3. it's not clear (I don't even know right now by looking at these >>>> reviews) when one would use "AutoStringTinyText" or "AutoStringSize". >>>> For example in >>>> >>>> https://review.openstack.org/#/c/446643/10/nova/db/sqlalchemy/migrate_repo/versions/216_havana.py >>>> I see a list of String(255)'s changed to one type or the other without >>>> any clear notion why one would use one or the other. Having names >>>> that define simply the declared nature of the type would be most >>>> appropriate. >>> >>> >>> One has to look at what the column is being used for and decide what >>> appropriate remediation steps are. This takes time and one must research >>> what kind of data goes in the column, what puts it there, what consumes >>> it, >>> and what remediation would have the least amount of impact. >>> >>>> I can add these names up to oslo.db and then we would just need to >>>> spread these out through all the open ndb reviews and then also patch >>>> up Cinder which seems to be the only ndb implementation that's been >>>> merged so far. >>>> >>>> Keep in mind this is really me trying to correct my own mistake, as I >>>> helped design and approved of the original approach here where >>>> projects would be consuming against the "ndb." namespace. However, >>>> after seeing it in reviews how prevalent the use of this extremely >>>> backend-specific name is, I think the use of the name should be much >>>> less frequent throughout projects and only surrounding logic that is >>>> purely to do with the ndb backend and no others. At the datatype >>>> level, the chance of future naming conflicts is very high and we >>>> should fix this mistake (my mistake) before it gets committed >>>> throughout many downstream projects. >>>> >>>> >>>> [1] https://review.openstack.org/#/c/427970/ >>>> >>>> [2] https://review.openstack.org/#/c/446643/ >>>> >>>> [3] https://review.openstack.org/#/c/446136/ >>>> >>>> >>>> __________________________________________________________________________ >>>> OpenStack Development Mailing List (not for usage questions) >>>> Unsubscribe: >>>> [email protected]?subject:unsubscribe >>>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev >>> >>> >>> >>> >>> __________________________________________________________________________ >>> OpenStack Development Mailing List (not for usage questions) >>> Unsubscribe: >>> [email protected]?subject:unsubscribe >>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev >> >> __________________________________________________________________________ >> OpenStack Development Mailing List (not for usage questions) >> Unsubscribe: [email protected]?subject:unsubscribe >> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev > > > > __________________________________________________________________________ > OpenStack Development Mailing List (not for usage questions) > Unsubscribe: [email protected]?subject:unsubscribe > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev __________________________________________________________________________ OpenStack Development Mailing List (not for usage questions) Unsubscribe: [email protected]?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
