incubator-cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Alex Huang <Alex.Hu...@citrix.com>
Subject RE: [REVIEW/PROPOSAL/DISCUSS] API refactoring story: goal and notes
Date Thu, 20 Dec 2012 13:51:30 GMT
I don't see db id to be removed.  Db ids are used for integrity checks.  See the following.

http://stackoverflow.com/questions/4813727/is-string-or-int-preferred-for-foreign-keys

A join in the db the size that cs deals with is not a big performance problem.  CS is not
a content system afterall.  The problem facing list* apis is the n+1 queries introduced by
the uuid mapping.  Min's work has proven that 90% of the operation is spent in those queries.
 It is also likely that the end user would like at least some sort of description from that
joint table to make sense of the object which makes the join necessary anyways.

No point in going there at all.

--Alex

> -----Original Message-----
> From: Anthony Xu [mailto:Xuefei.Xu@citrix.com]
> Sent: Wednesday, December 19, 2012 11:07 PM
> To: cloudstack-dev@incubator.apache.org
> Subject: RE: [REVIEW/PROPOSAL/DISCUSS] API refactoring story: goal and
> notes
>
> I agree it is too big to complete in 4.1 release.
> Any plan after 4.1?
> Is it possible to remove ID column from a table if this table has UUID, then
> ID<->UUID layer can be removed?
>
>
> Anthony
>
> > -----Original Message-----
> > From: Min Chen [mailto:min.chen@citrix.com]
> > Sent: Wednesday, December 19, 2012 11:00 PM
> > To: cloudstack-dev@incubator.apache.org
> > Subject: Re: [REVIEW/PROPOSAL/DISCUSS] API refactoring story: goal and
> > notes
> >
> > We thought of that, but the impact for that effort will be big,
> > considering that we need to populate those newly added foreign UUID
> > columns for all the tables through cross reference in upgrade, also
> > need
> > to consider adding code to set those foreign UUID columns in fresh
> > install
> > case. This scope may be too big to complete in 4.1 release.
> >
> > Thanks
> > -min
> >
> > On 12/19/12 10:52 PM, "Anthony Xu" <Xuefei.Xu@citrix.com> wrote:
> >
> > >Joining table still has cost. Each list API might become costly if the
> > >setup is big.
> > >Why not add foreign UUID in reference table?
> > >Do you see any issue with it?
> > >
> > >Anthony
> > >
> > >> -----Original Message-----
> > >> From: Min Chen [mailto:min.chen@citrix.com]
> > >> Sent: Wednesday, December 19, 2012 10:47 PM
> > >> To: cloudstack-dev@incubator.apache.org
> > >> Subject: Re: [REVIEW/PROPOSAL/DISCUSS] API refactoring story: goal
> > and
> > >> notes
> > >>
> > >> In that case, the extra DB query to fetch UUID is not avoidable. As
> > I
> > >> mentioned earlier, for some costly list APIs, we will create DB view
> > to
> > >> join these tables so that we can fetch those UUIDs in sort of batch
> > >> mode
> > >> while searching for objects to alleviate this effect.
> > >>
> > >> Thanks
> > >> -min
> > >>
> > >>
> > >>
> > >> On 12/19/12 10:41 PM, "Anthony Xu" <Xuefei.Xu@citrix.com> wrote:
> > >>
> > >> >If foreign key object is not needed, only foreign UUID is needed,
> > >> >does that mean extra DB view query is added for each listed item?
> > >> >Will this impact costly list APIs?
> > >> >
> > >> >
> > >> >Thanks,
> > >> >Anthony
> > >> >
> > >> >> -----Original Message-----
> > >> >> From: Min Chen [mailto:min.chen@citrix.com]
> > >> >> Sent: Wednesday, December 19, 2012 10:30 PM
> > >> >> To: cloudstack-dev@incubator.apache.org
> > >> >> Subject: Re: [REVIEW/PROPOSAL/DISCUSS] API refactoring story:
> > goal
> > >> and
> > >> >> notes
> > >> >>
> > >> >> We will replace all IdentityProxy fields from all response
> > classes
> > >> with
> > >> >> simple String to represent UUID directly. If you look through
API
> > >> code,
> > >> >> most times when we generate response from queried resources, we
> > >> already
> > >> >> query foreign key object once to fill in those related fields,
> > like
> > >> >> accountName, domainName, etc. By replacing IdentityProxy fields,
> > we
> > >> can
> > >> >> directly save those already queried uuid in response instead of
> > >> >> currently
> > >> >> querying them again during response serialization phase. For
> > those
> > >> more
> > >> >> costly list APIs, for example, listVms, we will create DB views
> > to
> > >> join
> > >> >> those uuids in the view to get them through one db view query.
> > >> >>
> > >> >> Thanks
> > >> >> -min
> > >> >>
> > >> >> On 12/19/12 10:20 PM, "Anthony Xu" <Xuefei.Xu@citrix.com>
wrote:
> > >> >>
> > >> >> >Thanks for explanation,
> > >> >> >
> > >> >> >How does CS handle ID->UUID translation in response object?
> > >> >> >
> > >> >> >Anthony
> > >> >> >
> > >> >> >> -----Original Message-----
> > >> >> >> From: Min Chen [mailto:min.chen@citrix.com]
> > >> >> >> Sent: Wednesday, December 19, 2012 10:13 PM
> > >> >> >> To: cloudstack-dev@incubator.apache.org
> > >> >> >> Subject: Re: [REVIEW/PROPOSAL/DISCUSS] API refactoring
story:
> > >> goal
> > >> >> and
> > >> >> >> notes
> > >> >> >>
> > >> >> >> Here Fang is talking about UUID translation to DB_ID
in
> > >> constructing
> > >> >> >> our
> > >> >> >> Command class (ApiDispatcher.setupParameters), not in
> > generating
> > >> >> >> response
> > >> >> >> object, so it will not impact list api performance.
> > >> >> >>
> > >> >> >> Thanks
> > >> >> >> -min
> > >> >> >>
> > >> >> >> On 12/19/12 10:07 PM, "Anthony Xu" <Xuefei.Xu@citrix.com>
> > wrote:
> > >> >> >>
> > >> >> >> >>2. The API layer provides a translation from
the UUID to
> > the
> > >> >> >> internal
> > >> >> >> >>DB_ID to the DB entity, but this translation
is done
> > internally.
> > >> >> >> Outside
> > >> >> >> >>users will never see DB_ID. Before the response
is sending
> > back
> > >> by
> > >> >> CS,
> > >> >> >> >>API layer replaces the internal DB_ID with the
UUID.
> > >> >> >> >
> > >> >> >> >I thought ID->UUID translation in API layer is
a workaround,
> > >> since
> > >> >> it
> > >> >> >> >might cause list API performance issue.
> > >> >> >> >Any plan to fix it completely in backend?
> > >> >> >> >
> > >> >> >> >
> > >> >> >> >Anthony
> > >> >> >> >
> > >> >> >> >> -----Original Message-----
> > >> >> >> >> From: Fang Wang [mailto:fang.wang@citrix.com]
> > >> >> >> >> Sent: Wednesday, December 19, 2012 10:02 PM
> > >> >> >> >> To: cloudstack-dev@incubator.apache.org
> > >> >> >> >> Subject: RE: [REVIEW/PROPOSAL/DISCUSS] API refactoring
> > story:
> > >> >> goal
> > >> >> >> and
> > >> >> >> >> notes
> > >> >> >> >>
> > >> >> >> >> Prachi did an excellent FS document for this,
and I added
> > the
> > >> >> high
> > >> >> >> >> level goals section
> > >> >> >> >> To her FS link at wiki:
> > >> >> >> >>
> > >> >> >> >>
> > >> >> >>
> > >> >>
> > >>
> >
> https://cwiki.apache.org/confluence/display/CLOUDSTACK/CloudStack+API+
> r
> > >> >> >> >> efactoring
> > >> >> >> >>
> > >> >> >> >> Lots of implementation details are covered in
the FS. We'll
> > >> >> update
> > >> >> >> it
> > >> >> >> >> along the way.
> > >> >> >> >> Thanks,
> > >> >> >> >> -Fang
> > >> >> >> >>
> > >> >> >> >> -----Original Message-----
> > >> >> >> >> From: Fang Wang [mailto:fang.wang@citrix.com]
> > >> >> >> >> Sent: Wednesday, December 19, 2012 9:46 PM
> > >> >> >> >> To: cloudstack-dev@incubator.apache.org
> > >> >> >> >> Subject: [REVIEW/PROPOSAL] API refactoring story:
goal and
> > >> notes
> > >> >> >> >>
> > >> >> >> >> Hi all,
> > >> >> >> >> After answering questions about why/what we
are doing to
> > API
> > >> >> >> >> refactoring, I'll add this to the FS document.
> > >> >> >> >> Probably lots of people are not clear what is
our
> > motivation
> > >> and
> > >> >> >> what
> > >> >> >> >> we want to achieve here.
> > >> >> >> >> (Will add this to the wiki page. )
> > >> >> >> >>
> > >> >> >> >> The goal of the API refactoring
> > >> >> >> >> To make the system more modular and dynamic,
API
> > refactoring
> > >> is
> > >> >> part
> > >> >> >> of
> > >> >> >> >> the 4.1 new architecture efforts.
> > >> >> >> >> Another goal is to allow developers and partners
to quickly
> > >> add
> > >> >> >> plugins
> > >> >> >> >> and new service modules and access control,
making
> > cloudstack
> > >> >> more
> > >> >> >> >> adaptive.
> > >> >> >> >> I am new on this as well In the beginning, I
do not know
> > too
> > >> much
> > >> >> >> about
> > >> >> >> >> it. Now after working on API refactoring, I
got a better
> > >> >> >> understanding
> > >> >> >> >> of the existing problems and our solutions.
The solution is
> > >> not
> > >> >> the
> > >> >> >> >> final perfect solution, for sure, feedback and
suggestion
> > is
> > >> >> always
> > >> >> >> >> welcome to make it a much better and adaptive
product.
> > >> >> >> >>
> > >> >> >> >> Current problem:
> > >> >> >> >> 1.        Security and access check is lying
around in
> > different
> > >> >> layers.
> > >> >> >> >> For example, in apiSevlet we check the web access
> > >> >> username/password
> > >> >> >> >> credentials, apiServer checks the command existence,
and
> > >> several
> > >> >> >> other
> > >> >> >> >> checks, then the DB access check mainly is done
at the
> > service
> > >> >> layer.
> > >> >> >> >> So it is hard for developers and system admin
to follow all
> > >> the
> > >> >> >> access
> > >> >> >> >> and validations, to make sure all the checks
are done
> > >> correctly.
> > >> >> To
> > >> >> >> do
> > >> >> >> >> it, developers need to be familiar with the
various parts
> > of
> > >> the
> > >> >> CS
> > >> >> >> >> code.
> > >> >> >> >>
> > >> >> >> >> 2.        Static: Majority of the DB access
check and security
> > >> >> validation
> > >> >> >> >> is tightly coupled with the lower layer service
class. This
> > >> not
> > >> >> only
> > >> >> >> >> made the code hard to follow, also inherently
made the
> > policy
> > >> >> static.
> > >> >> >> >> This makes it impossible for system admin to
apply a
> > different
> > >> >> >> security
> > >> >> >> >> policy adaptively. If one wants to adopt a different
policy,
> > >> not
> > >> >> >> only
> > >> >> >> >> he needs to understand code scattered around,
he also needs
> > to
> > >> >> >> rebuild
> > >> >> >> >> the CS after any changes.  Any security role
modification
> > is
> > >> not
> > >> >> >> >> dynamic.
> > >> >> >> >>
> > >> >> >> >> 3.        Performance implication: Access check
done at the
> > lower
> > >> >> service
> > >> >> >> >> layer makes the error code path long.
> > >> >> >> >>
> > >> >> >> >> 4.        Docs generated, CLI and APIs are loosely
coupled.
> > >> >> >> >>
> > >> >> >> >> 5.        Over the wire(OTW) entity is not well
defined. For
> > example,
> > >> >> to
> > >> >> >> >> listVMsCmd, it involves multiple DB access,
with the large
> > >> amount
> > >> >> of
> > >> >> >> >> data showing NICs, Vols, Secondary storage etc,
the command
> > >> can
> > >> >> take
> > >> >> >> >> quite a while.
> > >> >> >> >>
> > >> >> >> >> 6.        Admin and user have basically the
same end-point
> > access.
> > >> >> >> >>
> > >> >> >> >>        The goal of the API refactoring is aiming
to tackle
> > >> these
> > >> >> >> >> problems:
> > >> >> >> >>
> > >> >> >> >> 1.        We would pull security checks, DB
access checks, any
> > >> >> related
> > >> >> >> >> checks up from service layer/orchestration engine
to the
> > API
> > >> >> layer
> > >> >> >> as
> > >> >> >> >> much as possible. This makes the necessary checking
done
> > more
> > >> >> >> >> centralized and easy to follow. Conceptually
the cloud
> > >> >> orchestration
> > >> >> >> >> engine layer handles the orchestration, the
security check
> > and
> > >> >> the
> > >> >> >> >> access check should have done before reaching
this layer.
> > This
> > >> >> has
> > >> >> >> >> performance benefits, since checks are done
earlier instead
> > of
> > >> >> >> reaching
> > >> >> >> >> deep in the code path; this also has the benefits
of a
> > clear
> > >> >> >> >> architecture. The API layer does the necessary
access check
> > >> and
> > >> >> role
> > >> >> >> >> based authentication, makes it easier and dynamic
for
> > future
> > >> >> policy
> > >> >> >> >> change. New policy can be added easily and dynamically
as a
> > >> >> plugin
> > >> >> >> to
> > >> >> >> >> the system.
> > >> >> >> >>
> > >> >> >> >> 2.        The ACL and security checks also are
written as
> > adapter
> > >> >> plugins,
> > >> >> >> >> hence make them dynamic. Users and developers
and easily
> > adapt
> > >> >> new
> > >> >> >> >> policy if needed. This made the code more modular
and more
> > >> >> adaptive.
> > >> >> >> >>
> > >> >> >> >> 3.        Help improves performance: Instead
of finding access
> > error
> > >> >> layer
> > >> >> >> >> into the service layer, by doing 1, we would
do possible
> > >> checks
> > >> >> >> early
> > >> >> >> >> in the code path, which helps stop the wrong
access earlier
> > in
> > >> >> the
> > >> >> >> code
> > >> >> >> >> path.
> > >> >> >> >>
> > >> >> >> >> 4.        API layer is more tightly coupled
with Doc generation,
> > and
> > >> >> CLI.
> > >> >> >> >> Related commands are grouped together, and the
new @Doc
> > >> >> annotation
> > >> >> >> will
> > >> >> >> >> help show the related commands in document.
> > >> >> >> >>
> > >> >> >> >> 5.        We define new view objects as response
objects,
> > avoiding
> > >> >> big DB
> > >> >> >> >> joins at run time.
> > >> >> >> >>
> > >> >> >> >> 6.        Separate the admin and user APIs.
This is for
> > developers to
> > >> >> >> >> understand the code, which should be accessible
by users,
> > >> which
> > >> >> >> should
> > >> >> >> >> only be handled by Admin.  Hence developers
will have
> > better
> > >> >> grasp
> > >> >> >> of
> > >> >> >> >> the role and pay attention to the new code added.
It also
> > >> helps
> > >> >> the
> > >> >> >> >> document generated.
> > >> >> >> >>
> > >> >> >> >>        Notes:
> > >> >> >> >> 1.        For end users, the new APIs after
refactoring looks
> > pretty
> > >> >> much
> > >> >> >> >> the same. One big change is the ID, we will
always use UUID
> > in
> > >> >> the
> > >> >> >> over
> > >> >> >> >> the wire APIs. The UUID can be created by Cloudstack,
or
> > can
> > >> be
> > >> >> >> >> provided by users (we call it Xid - external
ID). Every
> > UUID
> > >> >> should
> > >> >> >> be
> > >> >> >> >> unique in the cloudstack system.
> > >> >> >> >>
> > >> >> >> >> 2.        The API layer provides a translation
from the UUID to
> > the
> > >> >> >> >> internal DB_ID to the DB entity, but this translation
is
> > done
> > >> >> >> >> internally. Outside users will never see DB_ID.
Before the
> > >> >> response
> > >> >> >> is
> > >> >> >> >> sending back by CS,  API layer replaces the
internal DB_ID
> > >> with
> > >> >> the
> > >> >> >> >> UUID.
> > >> >> >> >>
> > >> >> >> >> 3.        In original FS document, the annotation
of entityType
> > in
> > >> >> >> >> @Parameter points to a resource class, this
is replaced by
> > a
> > >> >> >> response
> > >> >> >> >> class. So entityType points to a response object,
and the
> > >> >> response
> > >> >> >> >> class has a one-to-one mapping from the response
to the
> > >> physical
> > >> >> >> entity
> > >> >> >> >> itself. This translation work is done by the
API layer and
> > the
> > >> >> >> >> entityManagerImpl.
> > >> >> >> >>
> > >> >> >> >> 4.        The packages for the new API commands
are all moved
> > from
> > >> >> the
> > >> >> >> >> current com.cloud.api.commands to new location:
> > >> >> >> >> org.apache.cloudstack.api.commands.user.[group
name]
> > >> >> >> >> org.apache.cloudstack.api.commands.admin.[group_name]
> > >> >> >> >>
> > >> >> >> >> The responses are also moved to new location
at
> > >> >> >> >> org.apache.cloudstack.api.response
> > >> >> >> >>
> > >> >> >> >> More implementation details can refer to the
FS document.
> > We
> > >> will
> > >> >> >> also
> > >> >> >> >> update the document along the way. The code
is branch from
> > >> master
> > >> >> at
> > >> >> >> >> api_refactoring.  Since the change is not minimum,
we would
> > >> like
> > >> >> the
> > >> >> >> >> community to know and give feedback.
> > >> >> >> >>
> > >> >> >> >> Thanks,
> > >> >> >> >> -Fang
> > >> >> >> >>
> > >> >> >> >>
> > >> >> >
> > >> >
> > >


Mime
View raw message