Code review comment for lp:~dimitern/juju-core/300-lp-1174610-unit-ids-unique

On 2014/02/18 10:25:48, fwereade wrote:
> On 2014/02/18 10:25:23, fwereade wrote:
> > On 2014/02/17 17:04:34, rog wrote:
> > > On 2014/02/17 16:44:18, dimitern wrote:
> > > > Please take a look.
> > >
> > > Looks reasonable, but I'm concerned that it can't be applied
> > > until we have mongo-schema-upgrade capability, which might be
> > > a while off yet.
> > >
> > > How about making the schema change backwardly compatible?
> > >
> > > For example, when a service is created, we mark it with a "uses
global
> > sequence
> > > numbering" flag
> > > (this will be false in already existing services).
> > > When a service is destroyed that doesn't have that flag set, we
could update
> > the
> > > sequences collection
> > > with the most recent unit number for that service.
> > > When a unit is created, if allocates it from the service sequence
number
> > unless
> > > the "uses global sequence" flag is set
> > > in which case it allocates it using the sequences collection.
> > >
> > > In that way, we can remain compatible with the old schema but
update to the
> > new
> > > schema over time.
> > >
> > > Eventually we can delete the flag and its associated logic when we
don't
> need
> > to
> > > support 1.16
> > > environments (assuming this fix goes into 1.18)
> >
> > This isn't quite enough. While clients still have direct DB access
(ie until
> > past 1.18) we need to *always* read from, and write to, the old
location, as
> > well as the new one -- and this actually renders any change
pointless, because
> > we could *always* have an old client removing then creating a new
service. We
> > actually *can't* do this yet. Sorry Dimiter, my enthusiasm for
fixing
> something
> > that annoyed hazmat got me over-excited and I failed to think it
through.
> >
> > FWIW this *is* exactly what we *should* do once we have mongo fully
locked
> > down...

> so, to be explicit, this sadly does not lgtm

(it's a shame to lose the incidental fixes though -- can they be easily
extracted?)

https://codereview.appspot.com/64890044/