aurora-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Igor Morozov <igm...@gmail.com>
Subject Re: Review Request 50168: Add rollback functionality to the scheduler
Date Wed, 27 Jul 2016 23:02:35 GMT


> On July 21, 2016, 4:05 p.m., Joshua Cohen wrote:
> > src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java,
lines 279-280
> > <https://reviews.apache.org/r/50168/diff/3/?file=1448819#file1448819line279>
> >
> >     What is the purpose of this call if it's not using the return value? As far
as I can tell there are no side effects from the call.

This is additional verification step similar to start() function. I think this step is not
nessesary if updateFactory.newUpdate(update.getInstructions(), true) == updateFactory.newUpdate(update.getInstructions(),
false). It was not clear from a code that this is the case.
https://github.com/apache/aurora/blob/master/src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java#L162


> On July 21, 2016, 4:05 p.m., Joshua Cohen wrote:
> > src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java, lines
129-131
> > <https://reviews.apache.org/r/50168/diff/3/?file=1448818#file1448818line129>
> >
> >     What if the update is in `ROLLED_FORWARD` and another update has already run
and completed since this one (i.e. there's no update in `ROLLING_FORWARD`, but there is a
more recent update in `ROLLED_FORWARD`)?

we should be able to rollback any update to its initial state. I'll correct the comment.


> On July 21, 2016, 4:05 p.m., Joshua Cohen wrote:
> > src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java,
line 251
> > <https://reviews.apache.org/r/50168/diff/3/?file=1448819#file1448819line251>
> >
> >     I might be missing something, but I don't see a check in this method that if
the update is complete, it's in the `ROLLED_FORWARD` state (as described by the javadoc on
the interface declaration).

I am relying here on JobUpdate state machine that whould prevent unallowed state transitions


- Igor


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


On July 21, 2016, 3:37 a.m., Igor Morozov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50168/
> -----------------------------------------------------------
> 
> (Updated July 21, 2016, 3:37 a.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Maxim Khutornenko, Bill Farner, and Zameer Manji.
> 
> 
> Bugs: AURORA-1721
>     https://issues.apache.org/jira/browse/AURORA-1721
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Add rollback functionality to the scheduler
> 
> Thic change also includes an addition to storage replicated log. I needed a way to re-insert
lock token for an existing job update.
> 
> 
> Diffs
> -----
> 
>   api/src/main/thrift/org/apache/aurora/gen/api.thrift 1d66208490aff6ea8af4c737845fa2cf13617529

>   api/src/main/thrift/org/apache/aurora/gen/storage.thrift 9e4213f13255a182df938bea44ca87fa03a25318

>   src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 52c3c6618a3cf1009435ca8a9cece36365913e55

>   src/main/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStore.java d2673e6b328cb1e249fbe91d18e0d9e935636eaa

>   src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java 39924c62108f6a68f545f90d77ceab4265d41fad

>   src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java d0de063fd78e6c4f62fae4a598d1d22f9775772d

>   src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java b534abf95bab6e1657e3ef993cf34c0d6ec460be

>   src/main/java/org/apache/aurora/scheduler/thrift/aop/AnnotatedAuroraAdmin.java 9243c92b11040b68ed6014b3991db69fc08bcddf

>   src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java f8357c46df1b025bf4e38a7ce1cb1c13a50c39f9

>   src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java 364c5c753f884a2d89e27802d7bbf3b2b6d3a08e

>   src/main/python/apache/aurora/client/api/__init__.py 68baf8fdb90cd26100159401c46c9963c24332b3

>   src/main/python/apache/aurora/client/cli/update.py bb526f7bf94d7bfe02fe2786493c85be1bfeb86f

>   src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java e157c0dfde5efc418448e138aa008ade742fe816

>   src/test/python/apache/aurora/client/api/test_scheduler_client.py afbd385b7eda64cb1f7d118b695e65e4045eac6c

> 
> Diff: https://reviews.apache.org/r/50168/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Igor Morozov
> 
>


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