<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
Hi Michael,<br>
<br>
Comments below..<br>
<br>
<div class="moz-cite-prefix">On 7/26/2017 1:08 PM, Michael Bayer
wrote:<br>
</div>
<blockquote
cite="mid:CAGi8UVdDCUAED6cxSFSqOwvySJy+-6v1LG3uSoL2wch3uKo_1Q@mail.gmail.com"
type="cite">
<div dir="auto">
<div><br>
<div class="gmail_extra"><br>
<div class="gmail_quote">On Jul 25, 2017 3:38 PM, "Octave J.
Orgeron" <<a moz-do-not-send="true"
href="mailto:octave.orgeron@oracle.com">octave.orgeron@oracle.com</a>>
wrote:<br type="attribution">
<blockquote class="quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex">Hi
Michael,<br>
<br>
I understand that you want to abstract this completely
away inside of oslo.db. However, the reality is that
making column changes based purely on the size and type
of that column, without understanding what that column
is being used for is extremely dangerous. You could end
up clobbering a column that needs a specific length for
a value, </blockquote>
</div>
</div>
</div>
<div dir="auto"><br>
</div>
<div dir="auto"><br>
</div>
<div dir="auto">Nowhere in my example is the current length
truncated. Also, if two distinct lengths truly must be
maintained we add a field "minimum_length".</div>
<div dir="auto"><br>
</div>
<div dir="auto"><br>
</div>
<div dir="auto">prevent</div>
<div dir="auto">
<div class="gmail_extra">
<div class="gmail_quote">
<blockquote class="quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex"> an
index from working, etc. </blockquote>
</div>
</div>
</div>
<div dir="auto"><br>
</div>
<div dir="auto">That's what the indexable flag would achieve. </div>
<div dir="auto"><br>
</div>
<div dir="auto">It</div>
<div dir="auto">
<div class="gmail_extra">
<div class="gmail_quote">
<blockquote class="quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex"> wouldn't
make sense to just do global changes on a column based
on the size.</blockquote>
</div>
</div>
</div>
<div dir="auto"><br>
</div>
<div dir="auto">This seems to be what your patches are doing,
however.</div>
</div>
</blockquote>
<br>
This is incorrect. I only change columns that meet my criteria for
being changed. I'm not globally changing columns across every table
and service. So to be clear and make sure we are on the same page..<br>
<br>
Are you proposing that we continue to select specific columns and
adjust their size by using the below, instead of the ndb.Auto*
functions?<br>
<br>
oslo_db.sqlalchemy.String(<value>, indexable=<value>,
ndb_size=<value>, ndb_type=<value>)<br>
<br>
i.e.<br>
<br>
oslo_db.sqlalchemy.String(255, ndb_type=TINYTEXT) -> VARCHAR(255)
for most dbs, TINYTEXT for ndb
<br>
oslo_db.sqlalchemy.String(4096, ndb_type=TEXT) -> VARCHAR(4096)
for most dbs, TEXT for ndb
<br>
oslo_db.sqlalchemy.String(255, ndb_size=64) -> VARCHAR(255) on
most dbs, VARCHAR(64) on ndb
<br>
<br>
So if I need to change a column that today says:<br>
<br>
sa.String(4096)<br>
<br>
I would modify it to:<br>
<br>
oslo_db.sqlalchemy.String(4096, ndb_type=TEXT)<br>
<br>
OR <br>
<br>
Are you proposing that we change very single column across every
single database blindly using some logic in oslo.db, where even if a
column doesn't need to be changed, it gets changed based on the
database engine type and the size of the column?<br>
<br>
So even if we have a table that doesn't need to be changed or
touched, we would end up with:<br>
<br>
mysql_enable_ndb = True<br>
<br>
sa.String(255) -> TINYTEXT<br>
<br>
If that is the type of behavior you are aiming for, I think don't
that makes sense.<br>
<br>
<br>
<br>
<blockquote
cite="mid:CAGi8UVdDCUAED6cxSFSqOwvySJy+-6v1LG3uSoL2wch3uKo_1Q@mail.gmail.com"
type="cite">
<div dir="auto">
<div dir="auto"><br>
</div>
<div dir="auto"><br>
</div>
<div dir="auto">
<div class="gmail_extra">
<div class="gmail_quote">
<blockquote class="quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex"> There
are far more tables that fit in both InnoDB and NDB
already than those that don't. As I've stated many times
before, the columns that I make changes to are evaluated
to understand:<br>
<br>
1. What populates it?<br>
2. Who consumes it?<br>
3. What are the possible values and required lengths?<br>
4. What is the impact of changing the size or type?<br>
5. Evaluated against the other columns in the table,
which one makes the most sense to adjust?<br>
<br>
I don't see a way of automating that and making it
maintainable without a lot more overhead in code and
people. </blockquote>
</div>
</div>
</div>
<div dir="auto"><br>
</div>
<div dir="auto">My proposal is intended to *reduce* the great
verbosity in the current patches I see and remove the burden
of every project having to be aware of "ndb" every time a
column is added.</div>
</div>
</blockquote>
<br>
I agree with using as few arguments to the oslo.db.sqlalchemy.String
function. But at the same time, if a column needs to be adjusted,
someone has to put the right arguments there. As far as the burden
goes, Oracle is already taking the ownership of making MySQL Cluster
work across services, which means maintaining patches and creating
new ones as projects evolve.<br>
<br>
Also, if we want one behavior for NDB, another for PostgreSQL, and
yet another for DB2 or Oracle DB, wouldn't we need to be somewhat
verbose on what we want?<br>
<br>
i.e.<br>
<br>
String(8192, ndb_type=TEXT, pgs_type=text, db2_type=CLOB,
ora_type=CLOB)<br>
<br>
<br>
<blockquote
cite="mid:CAGi8UVdDCUAED6cxSFSqOwvySJy+-6v1LG3uSoL2wch3uKo_1Q@mail.gmail.com"
type="cite">
<div dir="auto">
<div dir="auto"><br>
</div>
<div dir="auto"><br>
</div>
<div dir="auto">If</div>
<div dir="auto">
<div class="gmail_extra">
<div class="gmail_quote">
<blockquote class="quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex"> we
really want to remove the complexity here, why don't we
just change the sizes and types on these handful of
table columns so that they fit within both InnoDB and
NDB? </blockquote>
</div>
</div>
</div>
<div dir="auto"><br>
</div>
<div dir="auto"><br>
</div>
<div dir="auto">Because that requires new migrations which are a
great risk and inconvenience to projects. <br>
</div>
</div>
</blockquote>
<br>
When it comes to projects that need table columns adjusted, so far
we are only talking about Cinder, Neutron, Nova, and Magnum. Also,
let's keep in mind that it's only a handful of tables that are being
touched. I still feel this is being blown out of proportion. Here
are some metrics to consider, the 4 services with tables that need
to be adjusted:<br>
<br>
Service: # of Tables with columns changed:<br>
Cinder 1<br>
Neutron 5<br>
Nova 1<br>
Magnum 2<br>
<br>
With the exception of Magnum, those services tend to have over 75 or
100 tables. So I ask, are we blowing this out of proportion compared
the normal churn on tables in these services? For example, Neutron
dropped 30+ tables and changed dozens. These databases are not so
static over time to begin with.<br>
<br>
<blockquote
cite="mid:CAGi8UVdDCUAED6cxSFSqOwvySJy+-6v1LG3uSoL2wch3uKo_1Q@mail.gmail.com"
type="cite">
<div dir="auto">
<div dir="auto"><br>
</div>
<div dir="auto">That</div>
<div dir="auto">
<div class="gmail_extra">
<div class="gmail_quote">
<blockquote class="quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex"> way
we don't need these functions and the tables are exactly
the same? That would only leave us with the createtable,
savepoint/rollback, etc. stuff to address which is
already taken care of in the ndb module in oslo.db? Then
we just fix the foreign key stuff as I've been doing,
since it has zero impact on InnoDB deployments and if
anything ensures things are consistent. That would then
leave us to really focus on fixing migrations to use
oslo.db and pass the correct flags, which is a more
lengthy process than the rest of this.<br>
<br>
I don't see the point in trying to make this stuff
anymore complicated.</blockquote>
</div>
</div>
</div>
<div dir="auto"><br>
</div>
<div dir="auto">The proposal is to make it simpler than it is
right now.</div>
<div dir="auto"><br>
</div>
<div dir="auto">Run though every column change youve proposed
and show me which ones don't fit into my proposed ruleset. I
will add additional declarative flags to ensure those use
cases are covered. </div>
<div dir="auto"><br>
</div>
<div dir="auto"><br>
</div>
<div dir="auto"><br>
</div>
<div dir="auto">
<div class="gmail_extra">
<div class="gmail_quote">
<blockquote class="quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex"><font
color="#888888"><br>
<br>
Octave</font>
<div class="elided-text"><br>
<br>
On 7/25/2017 12:20 PM, Michael Bayer wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="elided-text">
On Mon, Jul 24, 2017 at 5:41 PM, Michael Bayer <<a
moz-do-not-send="true"
href="mailto:mbayer@redhat.com" target="_blank"><a class="moz-txt-link-abbreviated" href="mailto:mbayer@redhat.com">mbayer@redhat.com</a></a>>
wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0
.8ex;border-left:1px #ccc solid;padding-left:1ex">
<blockquote class="gmail_quote" style="margin:0 0
0 .8ex;border-left:1px #ccc
solid;padding-left:1ex">
oslo_db.sqlalchemy.String(255,
ndb_type=TINYTEXT) -> VARCHAR(255) for most<br>
dbs, TINYTEXT for ndb<br>
oslo_db.sqlalchemy.String(4096<wbr>,
ndb_type=TEXT) -> VARCHAR(4096) for most<br>
dbs, TEXT for ndb<br>
oslo_db.sqlalchemy.String(255, ndb_size=64)
-> VARCHAR(255) on most dbs,<br>
VARCHAR(64) on ndb<br>
<br>
This way, we can override the String with
TINYTEXT or TEXT or change the<br>
size for ndb.<br>
<blockquote class="gmail_quote" style="margin:0
0 0 .8ex;border-left:1px #ccc
solid;padding-left:1ex">
oslo_db.sqlalchemy.String(255)<wbr> ->
VARCHAR(255) on most dbs,<br>
TINYTEXT() on ndb<br>
oslo_db.sqlalchemy.String(255, ndb_size=64)
-> VARCHAR(255) on<br>
most dbs, VARCHAR(64) on ndb<br>
oslo_db.sqlalchemy.String(50) ->
VARCHAR(50) on all dbs<br>
oslo_db.sqlalchemy.String(64) ->
VARCHAR(64) on all dbs<br>
oslo_db.sqlalchemy.String(80) ->
VARCHAR(64) on most dbs, TINYTEXT()<br>
on ndb<br>
oslo_db.sqlalchemy.String(80, ndb_size=55)
-> VARCHAR(64) on most<br>
dbs, VARCHAR(55) on ndb<br>
<br>
don't worry about implementation, can the
above declaration -><br>
datatype mapping work ?<br>
<br>
<br>
</blockquote>
In my patch for Neutron, you'll see a lot of the
AutoStringText() calls to<br>
replace exceptionally long String columns (4096,
8192, and larger).<br>
</blockquote>
MySQL supports large VARCHAR now, OK. yeah this
could be<br>
String(8192, ndb_type=TEXT) as well.<br>
</blockquote>
OK, no, sorry each time I think of this I keep
seeing the verbosity of<br>
imports etc. in the code, because if we had:<br>
<br>
String(80, ndb_type=TEXT)<br>
<br>
then we have to import both String and TEXT, and
then what if there's<br>
ndb.TEXT, the code is still making an ndb-specific
decision, etc.<br>
<br>
I still see that this can be mostly automated from a
simple ruleset<br>
based on the size:<br>
<br>
length <= 64 : VARCHAR(length) on all backends<br>
length > 64, length <= 255: VARCHAR(length)
for most backends,<br>
TINYTEXT for ndb<br>
length > 4096: VARCHAR(length) for most
backends, TEXT for ndb<br>
<br>
the one case that seems outside of this is:<br>
<br>
String(255) where they have an index or key on the
VARCHAR, and in<br>
fact they only need < 64 characters to be
indexed. In that case you<br>
don't want to use TINYTEXT, right? So one
exception:<br>
<br>
oslo_db.sqlalchemy.types.Strin<wbr>g(255,
indexable=True)<br>
<br>
e.g. a declarative hint to the oslo_db backend to
not use a LOB type.<br>
<br>
then we just need oslo_db.sqlalchemy.types.Strin<wbr>g,
and virtually<br>
nothing except the import has to change, and a few
keywords.<br>
<br>
What we're trying to do in oslo_db is as much as
possible state the<br>
intent of a structure or datatype declaratively, and
leave as much of<br>
the implementation up to oslo_db itself.<br>
<br>
</div>
<div class="quoted-text">
______________________________<wbr>______________________________<wbr>______________<br>
OpenStack Development Mailing List (not for usage
questions)<br>
Unsubscribe: <a moz-do-not-send="true"
href="http://OpenStack-dev-request@lists.openstack.org?subject:unsubscribe"
rel="noreferrer" target="_blank">OpenStack-dev-request@lists.op<wbr>enstack.org?subject:unsubscrib<wbr>e</a><br>
<a moz-do-not-send="true"
href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev"
rel="noreferrer" target="_blank">http://lists.openstack.org/cgi<wbr>-bin/mailman/listinfo/openstac<wbr>k-dev</a><br>
</div>
</blockquote>
<div class="elided-text">
<br>
<br>
<br>
______________________________<wbr>______________________________<wbr>______________<br>
OpenStack Development Mailing List (not for usage
questions)<br>
Unsubscribe: <a moz-do-not-send="true"
href="http://OpenStack-dev-request@lists.openstack.org?subject:unsubscribe"
rel="noreferrer" target="_blank">OpenStack-dev-request@lists.op<wbr>enstack.org?subject:unsubscrib<wbr>e</a><br>
<a moz-do-not-send="true"
href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev"
rel="noreferrer" target="_blank">http://lists.openstack.org/cgi<wbr>-bin/mailman/listinfo/openstac<wbr>k-dev</a><br>
</div>
</blockquote>
</div>
<br>
</div>
</div>
</div>
<br>
<fieldset class="mimeAttachmentHeader"></fieldset>
<br>
<pre wrap="">__________________________________________________________________________
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: <a class="moz-txt-link-abbreviated" href="mailto:OpenStack-dev-request@lists.openstack.org?subject:unsubscribe">OpenStack-dev-request@lists.openstack.org?subject:unsubscribe</a>
<a class="moz-txt-link-freetext" href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a>
</pre>
</blockquote>
<br>
<div class="moz-signature">-- <br>
<br>
<font color="#666666" size="2" face="Verdana, Arial, Helvetica,
sans-serif"><font color="#666666" size="2" face="Verdana, Arial,
Helvetica, sans-serif">
</font></font></div>
</body>
</html>