aurora-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "David McLaughlin" <da...@dmclaughlin.com>
Subject Re: Review Request 24720: Expand actions in JobUpdateAction
Date Mon, 18 Aug 2014 17:25:10 GMT


> On Aug. 15, 2014, 6:02 p.m., Maxim Khutornenko wrote:
> > src/main/thrift/org/apache/aurora/gen/api.thrift, line 558
> > <https://reviews.apache.org/r/24720/diff/2/?file=661560#file661560line558>
> >
> >     Rolling back assumes removing new (INSTANCE_REMOVED) and adding old (INSTANCE_ADDED).
Do we want to distinct those actions from the forward roll?
> 
> David McLaughlin wrote:
>     INSTANCE_REMOVED is only used when you remove an instance because the new instanceCount
is less than previous one, it's not used if you're removing an instance as part of updating
it.
> 
> Maxim Khutornenko wrote:
>     I guess I did not see the JobUpdateAction as a post factum update story teller. I
thought it would be used more as a facilitator for internal actions to be done (i.e. add_instance,
kill_instance, and etc.) 
>     
>     I don't have a problem with this high level mapping as long as updater would not
rely on it to drive the state machine progress.

We should be careful either way not to leak internal state machines into the public API. But
yes, I think definitely the purpose of instance events is to retell a story of what happened
during this update.


- David


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24720/#review50752
-----------------------------------------------------------


On Aug. 15, 2014, 5:52 p.m., David McLaughlin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24720/
> -----------------------------------------------------------
> 
> (Updated Aug. 15, 2014, 5:52 p.m.)
> 
> 
> Review request for Aurora, Maxim Khutornenko and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Added some more items to the JobUpdateAction enum. Not married to the labels I've chosen,
and I wasn't sure if we want to break out INSTANCE_UPDATING into finer-grained actions. 
> 
> 
> Expected actions:
> 
> 1) instance update: INSTANCE_UPDATED
> 2) instance remove: INSTANCE_REMOVED
> 3) instance add: INSTANCE_ADDED -> INSTANCE_UPDATING -> INSTANCE_UPDATED
> 4) noop: INSTANCE_SKIPPED
> 5) instance rolled back: INSTANCE_ROLLED_BACK
> 
> 
> Diffs
> -----
> 
>   src/main/thrift/org/apache/aurora/gen/api.thrift af9f02ed1de487bc5cc2967d2edcece5b21e0be5

>   src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java f695b85514bcc5cedb16e962124af3db052cb17a

>   src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java ebcb9103d75909080f5b6a69db3a1bf46cfd9780

>   src/test/resources/org/apache/aurora/gen/api.thrift.md5 21a05f6939da1dd7fc15cf6336bc3fee283f16ab

> 
> Diff: https://reviews.apache.org/r/24720/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> David McLaughlin
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message