Hi Michael,
Comments below..
On 7/24/2017 2:49 PM, Michael Bayer wrote:
On Mon, Jul 24, 2017 at 3:37 PM, Octave J. Orgeron
<octave.orge...@oracle.com> wrote:
For these, here is a brief synopsis:
AutoStringTinyText, will convert a column to the TinyText type. This is used
for cases where a 255 varchar string needs to be converted to a text blob to
make the row fit within the NDB limits. If you are using ndb, it'll convert
it to TinyText, otherwise it leaves it alone. The reason that TinyText type
was chosen is because it'll hold the same 255 varchars and saves on space.
AutoStringText, does the same as the above, but converts the type to Text
and is meant for use cases where you need more than 255 varchar worth of
space. Good examples of these uses are where outputs of hypervisor and OVS
commands are dumped into the database.
AutoStringSize, you pass two parameters, one being the non-NDB size and the
second being the NDB size. The point here is where you need to reduce the
size of the column to fit within the NDB limits, but you want to preserve
the String varchar type because it might be used in a key, index, etc. I
only use these in cases where the impacts are very low.. for example where a
column is used for keeping track of status (up, down, active, inactive,
etc.) that don't require 255 varchars.
Can the "auto" that is supplied by AutoStringTinyText and
AutoStringSize be merged?
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.
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).
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: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
__________________________________________________________________________
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 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 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
