Re: [openstack-dev] [heat]Heat Db Model updates

2014-07-25 Thread Clint Byrum
Excerpts from Zane Bitter's message of 2014-07-24 12:09:39 -0700:
 On 17/07/14 07:51, Ryan Brown wrote:
  On 07/17/2014 03:33 AM, Steven Hardy wrote:
  On Thu, Jul 17, 2014 at 12:31:05AM -0400, Zane Bitter wrote:
  On 16/07/14 23:48, Manickam, Kanagaraj wrote:
  SNIP
  *Resource*
 
  Status  action should be enum of predefined status
 
  +1
 
  Rsrc_metadata - make full name resource_metadata
 
  -0. I don't see any benefit here.
 
  Agreed
 
 
  I'd actually be in favor of the change from rsrc-resource, I feel like
  rsrc is a pretty opaque abbreviation.
 
 I'd just like to remind everyone that these changes are not free. 
 Database migrations are a pain to manage, and every new one slows down 
 our unit tests.
 
 We now support multiple heat-engines connected to the same database and 
 people want to upgrade their installations, so that means we have to be 
 able to handle different versions talking to the same database. Unless 
 somebody has a bright idea I haven't thought of, I assume that means 
 carrying code to handle both versions for 6 months before actually being 
 able to implement the migration. Or are we saying that you have to 
 completely shut down all instances of Heat to do an upgrade?
 
 The name of the nova_instance column is so egregiously misleading that 
 it's probably worth the pain. Using an enumeration for the states will 
 save a lot of space in the database (though it would be a much more 
 obvious win if we were querying on those columns). Changing a random 
 prefix that was added to avoid a namespace conflict to a slightly 
 different random prefix is well below the cost-benefit line IMO.

In past lives managing apps like Heat, We've always kept supporting the
previous schema in new code versions. So the process is:

* Upgrade all code
* Restart all services
* Upgrade database schema
* Wait a bit for reverts
* Remove backward compatibility

Now this was always in more of a continuous delivery environment, so
there was not more than a few weeks of waiting for reverts. In OpenStack
we'd have a single release to wait.

We're not special though, doesn't Nova have some sort of object versioning
code that helps them manage the versions of each type of data for this
very purpose?

___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


Re: [openstack-dev] [heat]Heat Db Model updates

2014-07-25 Thread Zane Bitter

On 25/07/14 13:50, Clint Byrum wrote:

Excerpts from Zane Bitter's message of 2014-07-24 12:09:39 -0700:

On 17/07/14 07:51, Ryan Brown wrote:

On 07/17/2014 03:33 AM, Steven Hardy wrote:

On Thu, Jul 17, 2014 at 12:31:05AM -0400, Zane Bitter wrote:

On 16/07/14 23:48, Manickam, Kanagaraj wrote:

SNIP
*Resource*

Status  action should be enum of predefined status


+1


Rsrc_metadata - make full name resource_metadata


-0. I don't see any benefit here.


Agreed



I'd actually be in favor of the change from rsrc-resource, I feel like
rsrc is a pretty opaque abbreviation.


I'd just like to remind everyone that these changes are not free.
Database migrations are a pain to manage, and every new one slows down
our unit tests.

We now support multiple heat-engines connected to the same database and
people want to upgrade their installations, so that means we have to be
able to handle different versions talking to the same database. Unless
somebody has a bright idea I haven't thought of, I assume that means
carrying code to handle both versions for 6 months before actually being
able to implement the migration. Or are we saying that you have to
completely shut down all instances of Heat to do an upgrade?

The name of the nova_instance column is so egregiously misleading that
it's probably worth the pain. Using an enumeration for the states will
save a lot of space in the database (though it would be a much more
obvious win if we were querying on those columns). Changing a random
prefix that was added to avoid a namespace conflict to a slightly
different random prefix is well below the cost-benefit line IMO.


In past lives managing apps like Heat, We've always kept supporting the
previous schema in new code versions. So the process is:

* Upgrade all code
* Restart all services
* Upgrade database schema
* Wait a bit for reverts
* Remove backward compatibility


OK, so we can put the migration in now but we still have to support the 
old DB format for 6 months. I'm fine with that; I think my point that 
trivial cosmetic changes don't justify the extra cruft stands.


I'm curious now if this is the upgrade process that we're documenting, 
though? I can think of a bunch of places where we added e.g. new tables 
or columns to the DB and AFAIK the code assumes that they'll be there 
(i.e. that the schema upgrade happened before trying to use that feature).


cheers,
Zane.


Now this was always in more of a continuous delivery environment, so
there was not more than a few weeks of waiting for reverts. In OpenStack
we'd have a single release to wait.

We're not special though, doesn't Nova have some sort of object versioning
code that helps them manage the versions of each type of data for this
very purpose?

___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev




___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


Re: [openstack-dev] [heat]Heat Db Model updates

2014-07-24 Thread Zane Bitter

On 17/07/14 07:51, Ryan Brown wrote:

On 07/17/2014 03:33 AM, Steven Hardy wrote:

On Thu, Jul 17, 2014 at 12:31:05AM -0400, Zane Bitter wrote:

On 16/07/14 23:48, Manickam, Kanagaraj wrote:

SNIP
*Resource*

Status  action should be enum of predefined status


+1


Rsrc_metadata - make full name resource_metadata


-0. I don't see any benefit here.


Agreed



I'd actually be in favor of the change from rsrc-resource, I feel like
rsrc is a pretty opaque abbreviation.


I'd just like to remind everyone that these changes are not free. 
Database migrations are a pain to manage, and every new one slows down 
our unit tests.


We now support multiple heat-engines connected to the same database and 
people want to upgrade their installations, so that means we have to be 
able to handle different versions talking to the same database. Unless 
somebody has a bright idea I haven't thought of, I assume that means 
carrying code to handle both versions for 6 months before actually being 
able to implement the migration. Or are we saying that you have to 
completely shut down all instances of Heat to do an upgrade?


The name of the nova_instance column is so egregiously misleading that 
it's probably worth the pain. Using an enumeration for the states will 
save a lot of space in the database (though it would be a much more 
obvious win if we were querying on those columns). Changing a random 
prefix that was added to avoid a namespace conflict to a slightly 
different random prefix is well below the cost-benefit line IMO.


cheers,
Zane.

___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


Re: [openstack-dev] [heat]Heat Db Model updates

2014-07-17 Thread Steven Hardy
On Thu, Jul 17, 2014 at 12:31:05AM -0400, Zane Bitter wrote:
 On 16/07/14 23:48, Manickam, Kanagaraj wrote:
 I have gone thru the Heat Database and found the drawbacks in the
 existing model as listed below.  Could you review and add anything
 missing here. Thanks.
 
 Heat Database model is having following drawbacks:
 
 1.Duplicate information
 
 2.Incomplete naming of columns
 
 3.Inconsistency in the identifiers (id) and deleted_at columns across
 the tables
 
 4.resource table is specific to nova and make it generic
 
 5.Pre-defined constants are not using enum.
 
 And the section provided below describes these problem on  table vice.
 
 *Stack*
 
 Duplicate info
 
 Tenant  stack_user_project_id
 
 These are different things; stack_user_project_id is the project/tenant in
 which Heat creates users (in a different domain); tenant is the
 project/tenant in which the stack itself was created.

+1

See this blog post for further info on the stack user project:

http://hardysteven.blogspot.co.uk/2014/04/heat-auth-model-updates-part-2-stack.html

A better cleanup related to the tenant entry imo would be to migrate all
references to project_id, but there are some issues with that:

- The code inconsistently stores the tenant ID and name in the tenant
  fields (in the stack and user_creds tables repectively)
- Some clients (the swift Connection class in particular) seem to need the
  tenant *name*, which seems wrong, and probably won't work for non-default
  domains, so we need to investigate this and figure out if we can migrate
  to just storing and using the project_id everywhere.

 Credentials_id  username , owner_id.
 
 Tenant is also part of user_creds and Stack always has credentials_id,
 so what is the need of having tenant info in stack table and in stack
 table only the credentials_id is sufficient.
 
 tenant is in the Stack table because we routinely query by tenant and we
 don't want to have to do a join.
 
 There may be a legitimate reason for the UserCreds table to exist separately
 from the Stack table but I don't know what it is, so merging the two is an
 option.

Yes, there is IMO - the user_creds record is not necessarily unique to a
stack - we've been creating a new row for all stacks (including backup and
nested stacks), but really you only need one set of stored credentials for
the top level stack which is then inherited by all child stacks.

I posted two patches yesterday implementing this:

https://review.openstack.org/#/q/status:open+project:openstack/heat+branch:master+topic:bug/1342593_2,n,z

So when they land, we won't (easily) be able to merge the usre_creds and
stack tables.  Note I'm also working on a cleanup which abstracts the
user_creds data behind a class, similar to Stack/Template etc.

 
 Status  action should be enum of predefined status
 
 +1. I assume it is still easy to add more actions later?

Well they're already defined via tuples in the respective classes, but OK.

 *User_creads*
 
 correct the spelling in Truststore_id
 
 trustor_user_id is correct.

Yeah, see the trust documentation and my trusts blog post:

https://github.com/openstack/identity-api/blob/master/v3/src/markdown/identity-api-v3-os-trust-ext.md

http://hardysteven.blogspot.co.uk/2014/04/heat-auth-model-updates-part-1-trusts.html

 *Resource*
 
 Status  action should be enum of predefined status
 
 +1
 
 Rsrc_metadata - make full name resource_metadata
 
 -0. I don't see any benefit here.

Agreed

 Why only nova_instance column how about for other services like cinder,
 glance resource, could be renamed to be generic enough??
 
 +1 this should have been called physical_resource_id.

+1, the nova_instance thing is just a historical mistake which never got
cleaned up.

 
 Last_evaluated - append _at
 
 I really don't see the point.

Me neither, particularly since we plan to deprecate the WatchRule
implementation (possibly for Juno):

https://blueprints.launchpad.net/heat/+spec/deprecate-stack-watch

 
 State should be an enum
 
 +1
 
 *Event*
 
 Why uuid and id both used?
 
 I believe it's because you should always use an integer as the primary key.
 I'm not sure if it makes a difference even though we _never_ do a lookup by
 the (integer) id.
 
 Resource_action is being used in both event and resource table, so it
 should be moved to common table
 
 I'm not sure what this means. Do you mean a common base class?

The resource action/status in the event is a snapshot of the resource
action/status at the time of the event, ergo we can't reference some common
data (such as the resource table) as the data will change.

 Resource_status should be any enum
 
 +1

Ok, but note we're doing a weird thing in resource.Resource.signal which
may need to be fixed first (using signal as the action, which isn't a
real resource action and gives odd output combined with the status)

Steve

___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org

Re: [openstack-dev] [heat]Heat Db Model updates

2014-07-17 Thread Ryan Brown

On 07/17/2014 03:33 AM, Steven Hardy wrote:

On Thu, Jul 17, 2014 at 12:31:05AM -0400, Zane Bitter wrote:

On 16/07/14 23:48, Manickam, Kanagaraj wrote:

SNIP
*Resource*

Status  action should be enum of predefined status


+1


Rsrc_metadata - make full name resource_metadata


-0. I don't see any benefit here.


Agreed



I'd actually be in favor of the change from rsrc-resource, I feel like 
rsrc is a pretty opaque abbreviation.


--
Ryan Brown / Software Engineer, Openstack / Red Hat, Inc.

___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


Re: [openstack-dev] [heat]Heat Db Model updates

2014-07-17 Thread Angus Salkeld
On Thu, 2014-07-17 at 08:51 -0400, Ryan Brown wrote:
 On 07/17/2014 03:33 AM, Steven Hardy wrote:
  On Thu, Jul 17, 2014 at 12:31:05AM -0400, Zane Bitter wrote:
  On 16/07/14 23:48, Manickam, Kanagaraj wrote:
  SNIP
  *Resource*
 
  Status  action should be enum of predefined status
 
  +1
 
  Rsrc_metadata - make full name resource_metadata
 
  -0. I don't see any benefit here.
 
  Agreed
 
 
 I'd actually be in favor of the change from rsrc-resource, I feel like 
 rsrc is a pretty opaque abbreviation.
 
It really has nothing to do with resource, when I added this field 
I wanted to call it metadata, but it conflicts with an internal
attribute in sqla. So I had to change it somehow.

Are we planning data migrations for these changes?

-Angus


signature.asc
Description: This is a digitally signed message part
___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


Re: [openstack-dev] [heat]Heat Db Model updates

2014-07-17 Thread Clint Byrum
Excerpts from Manickam, Kanagaraj's message of 2014-07-16 20:48:04 -0700:
 Event
 Why uuid and id both used?

The event uuid is the user-facing ID. However, we need to return events
to the user in insertion order. So we use an auto-increment primary key,
and order by that in 'heat event-list stack_name'.

We don't want to expose that integer to the user though, because knowing
the rate at which these integers increase would reveal a lot about the
goings on inside Heat.

 Resource_action is being used in both event and resource table, so it should 
 be moved to common table

If we're joining to resource already o-k, but it is worth noting that
there is a desire to not use a SQL table for event storage. Maintaining
those events on a large, busy stack will be expensive. The simpler
solution is to just write batches of event files into swift.

___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


[openstack-dev] [heat]Heat Db Model updates

2014-07-16 Thread Manickam, Kanagaraj
I have gone thru the Heat Database and found the drawbacks in the existing 
model as listed below.  Could you review and add anything missing here. Thanks.

Heat Database model is having following drawbacks:
1.   Duplicate information
2.   Incomplete naming of columns
3.   Inconsistency in the identifiers (id) and deleted_at columns across 
the tables
4.   resource table is specific to nova and make it generic
5.   Pre-defined constants are not using enum.

And the section provided below describes these problem on  table vice.

Stack
Duplicate info
Tenant  stack_user_project_id
Credentials_id  username , owner_id.
Tenant is also part of user_creds and Stack always has credentials_id,  so what 
is the need of having tenant info in stack table and in stack table only the 
credentials_id is sufficient.

Status  action should be enum of predefined status

User_creads
correct the spelling in Truststore_id

Resource
Status  action should be enum of predefined status
Rsrc_metadata - make full name resource_metadata
Why only nova_instance column how about for other services like cinder, glance 
resource, could be renamed to be generic enough??

Watch_rule
Last_evaluated - append _at
State should be an enum

Event
Why uuid and id both used?
Resource_action is being used in both event and resource table, so it should be 
moved to common table
Resource_status should be any enum



Regards
Kanagaraj M

___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


Re: [openstack-dev] [heat]Heat Db Model updates

2014-07-16 Thread Zane Bitter

On 16/07/14 23:48, Manickam, Kanagaraj wrote:

I have gone thru the Heat Database and found the drawbacks in the
existing model as listed below.  Could you review and add anything
missing here. Thanks.

Heat Database model is having following drawbacks:

1.Duplicate information

2.Incomplete naming of columns

3.Inconsistency in the identifiers (id) and deleted_at columns across
the tables

4.resource table is specific to nova and make it generic

5.Pre-defined constants are not using enum.

And the section provided below describes these problem on  table vice.

*Stack*

Duplicate info

Tenant  stack_user_project_id


These are different things; stack_user_project_id is the 
project/tenant in which Heat creates users (in a different domain); 
tenant is the project/tenant in which the stack itself was created.



Credentials_id  username , owner_id.

Tenant is also part of user_creds and Stack always has credentials_id,
so what is the need of having tenant info in stack table and in stack
table only the credentials_id is sufficient.


tenant is in the Stack table because we routinely query by tenant and we 
don't want to have to do a join.


There may be a legitimate reason for the UserCreds table to exist 
separately from the Stack table but I don't know what it is, so merging 
the two is an option.



Status  action should be enum of predefined status


+1. I assume it is still easy to add more actions later?


*User_creads*

correct the spelling in Truststore_id


trustor_user_id is correct.


*Resource*

Status  action should be enum of predefined status


+1


Rsrc_metadata - make full name resource_metadata


-0. I don't see any benefit here.


Why only nova_instance column how about for other services like cinder,
glance resource, could be renamed to be generic enough??


+1 this should have been called physical_resource_id.


*Watch_rule*

Last_evaluated - append _at


I really don't see the point.


State should be an enum


+1


*Event*

Why uuid and id both used?


I believe it's because you should always use an integer as the primary 
key. I'm not sure if it makes a difference even though we _never_ do a 
lookup by the (integer) id.



Resource_action is being used in both event and resource table, so it
should be moved to common table


I'm not sure what this means. Do you mean a common base class?


Resource_status should be any enum


+1

cheers,
Zane.

___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


Re: [openstack-dev] [heat]Heat Db Model updates

2014-07-16 Thread Manickam, Kanagaraj
Hi Zane,

Please find inline answer.

Regards
Kanagaraj M

-Original Message-
From: Zane Bitter [mailto:zbit...@redhat.com] 
Sent: Thursday, July 17, 2014 10:01 AM
To: openstack-dev@lists.openstack.org
Subject: Re: [openstack-dev] [heat]Heat Db Model updates

On 16/07/14 23:48, Manickam, Kanagaraj wrote:
 I have gone thru the Heat Database and found the drawbacks in the 
 existing model as listed below.  Could you review and add anything 
 missing here. Thanks.

 Heat Database model is having following drawbacks:

 1.Duplicate information

 2.Incomplete naming of columns

 3.Inconsistency in the identifiers (id) and deleted_at columns across 
 the tables

 4.resource table is specific to nova and make it generic

 5.Pre-defined constants are not using enum.

 And the section provided below describes these problem on  table vice.

 *Stack*

 Duplicate info

 Tenant  stack_user_project_id

These are different things; stack_user_project_id is the project/tenant in 
which Heat creates users (in a different domain); tenant is the 
project/tenant in which the stack itself was created.


KanagarajM  


 Credentials_id  username , owner_id.

 Tenant is also part of user_creds and Stack always has credentials_id, 
 so what is the need of having tenant info in stack table and in stack 
 table only the credentials_id is sufficient.

tenant is in the Stack table because we routinely query by tenant and we don't 
want to have to do a join.

There may be a legitimate reason for the UserCreds table to exist separately 
from the Stack table but I don't know what it is, so merging the two is an 
option. 

Kanagaraj M user_creds are being consumed by stack only and I feel that for 
one user say 'admin1' there will be one row in user_Creds table and who can 
have more than one stacks owned. I assumed this could be the reason. 
But to validate,  I created two stacks with same user and for each stack, a 
seprate row is created. So as you suggested, I also feel that stack and 
user_creds could be merged if there is no other fact. And it will remove the 
redundancy too.

 Status  action should be enum of predefined status

+1. I assume it is still easy to add more actions later?

 *User_creads*

 correct the spelling in Truststore_id

trustor_user_id is correct.

Kanagaraj M sure.

 *Resource*

 Status  action should be enum of predefined status

+1

 Rsrc_metadata - make full name resource_metadata

-0. I don't see any benefit here.

KanagarajM  It is to just make consistency in naming format 

 Why only nova_instance column how about for other services like 
 cinder, glance resource, could be renamed to be generic enough??

+1 this should have been called physical_resource_id.

 *Watch_rule*

 Last_evaluated - append _at

I really don't see the point.

KanagarajM  It is to just make consistency in naming format when we say 
created_at, updated_at and so last_evaluated_at. 

 State should be an enum

+1

 *Event*

 Why uuid and id both used?

I believe it's because you should always use an integer as the primary key. I'm 
not sure if it makes a difference even though we _never_ do a lookup by the 
(integer) id.

Kanagaraj M In openstack, most of the services migrated from using INT to UUID 
for the primary key.  And more than that, it would be nice to make consistency. 
The reason is, when the user access the resource over REST API, if we use UUID 
for the all the entities used in the heat project, it will make user/developer 
experience easier.

 Resource_action is being used in both event and resource table, so it 
 should be moved to common table

I'm not sure what this means. Do you mean a common base class?

KanagarajM  Yes, its an implementation specific and have a common python base 
class.


 Resource_status should be any enum

+1

cheers,
Zane.

___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev