Keyboard Shortcuts

File
u :up to issue
m :publish + mail comments
M :edit review message
j / k :jump to file after / before current file
J / K :jump to next file with a comment after / before current file
Side-by-side diff
i :toggle intra-line diffs
e :expand all comments
c :collapse all comments
s :toggle showing all comments
n / p :next / previous diff chunk or comment
N / P :next / previous comment
<Up> / <Down> :next / previous line
<Enter> :respond to / edit current comment
d :mark current comment as done
Issue
u :up to list of issues
m :publish + mail comments
j / k :jump to patch after / before current patch
o / <Enter> :open current patch in side-by-side view
i :open current patch in unified diff view
Issue List
j / k :jump to issue after / before current issue
o / <Enter> :open current issue
# : close issue
Comment/message editing
<Ctrl> + s or <Ctrl> + Enter :save comment
<Esc> :cancel edit
Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(16)
Issues Repositories Search
Open Issues | Closed Issues | All Issues | Sign in with your Google Account to create issues and add comments

Issue 64890044: state: Make unit ids unique. Fix #1174610

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 years, 10 months ago by dimitern
Modified:
11 years, 10 months ago
Reviewers:
mp+206765, fwereade , rog
Visibility:
Public.
state: Make unit ids unique. Fix #1174610 This changes the way unit names are generated when adding a new unit to a service. Before, we used serviceDoc.UnitSeq to generate the last part of a unit name. Now, we use the sequence collection with a key "s#<svcname>#unit" as the unit ids sequence (as we do for machine, containers, etc.). This is a schema change (removing of UnitSeq from the service document). So it should be handled the same way as any schema change during an upgrade, but this CL does not do that. I realized the sequence collection is not created as all the other collections in state.Initialize, so I added it there. Also added a test to guarantee unit ids increase each time for the same service name (like the bug suggests and how it was before). Per fwereade's suggestion, I changed the error message AddUnit() returns when a service is not found: "service is no longer alive" rather than "service "blah" not found", and also fixed a test that relied on that. https://code.launchpad.net/~dimitern/juju-core/300-lp-1174610-unit-ids-unique/+merge/206765 (do not edit description out of merge proposal)

Patch Set 1 #

Created: 11 years, 10 months ago
Download [raw] [tar.bz2]
Unified diffs Side-by-side diffs Delta from patch set Stats (+35 lines, -14 lines) Patch
A [revision details] View 1 chunk +2 lines, -0 lines 0 comments Download
M state/open.go View 1 chunk +1 line, -0 lines 0 comments Download
M state/sequence.go View 1 chunk +1 line, -1 line 0 comments Download
M state/service.go View 4 chunks +6 lines, -11 lines 0 comments Download
M state/service_test.go View 3 chunks +24 lines, -2 lines 0 comments Download
M state/state.go View 1 chunk +1 line, -0 lines 0 comments Download
Total messages: 5
|
dimitern
Please take a look.
11 years, 10 months ago (2014年02月17日 16:44:18 UTC) #1
Please take a look.
Sign in to reply to this message.
rog
On 2014年02月17日 16:44:18, dimitern wrote: > Please take a look. Looks reasonable, but I'm concerned ...
11 years, 10 months ago (2014年02月17日 17:04:34 UTC) #2
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)
Sign in to reply to this message.
fwereade
On 2014年02月17日 17:04:34, rog wrote: > On 2014年02月17日 16:44:18, dimitern wrote: > > Please take ...
11 years, 10 months ago (2014年02月18日 10:25:23 UTC) #3
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...
Sign in to reply to this message.
fwereade
On 2014年02月18日 10:25:23, fwereade wrote: > On 2014年02月17日 17:04:34, rog wrote: > > On 2014年02月17日 ...
11 years, 10 months ago (2014年02月18日 10:25:48 UTC) #4
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
Sign in to reply to this message.
fwereade
On 2014年02月18日 10:25:48, fwereade wrote: > On 2014年02月18日 10:25:23, fwereade wrote: > > On 2014年02月17日 ...
11 years, 10 months ago (2014年02月18日 10:26:37 UTC) #5
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?)
Sign in to reply to this message.
|
Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b

AltStyle によって変換されたページ (->オリジナル) /