> On Aug. 15, 2014, 6:20 p.m., Bill Farner wrote:
> > src/main/thrift/org/apache/aurora/gen/api.thrift, line 751
> > <https://reviews.apache.org/r/24744/diff/1/?file=661623#file661623line751>
> >
> >     We really should not expose the lock.  Any attempt to do anything with 
> > the lock will ~certainly interfere with the updater.
> 
> Maxim Khutornenko wrote:
>     How do we ensure pause/resume/abort is authorized to act on the update 
> then? Sure, storing the lock on the client is not a good idea but unless we 
> have some secondary way to authorize the action anyone could interfere with 
> the update given its ID.
> 
> Bill Farner wrote:
>     That's exactly what we need.  Abort/pause/resume are unscoped, big red 
> buttons.
> 
> Maxim Khutornenko wrote:
>     So, anyone authorized to act on the job would be able to interfere? I 
> thought the big red button would be the cancel_update with the 
> abort/pause/resume allowed only to the update owner.
>     
>     In that case, I need to clear all job update apis from the Lock concept.
> 
> Maxim Khutornenko wrote:
>     Actually, do we even have to expose updateId from the startJobUpdate api? 
> Given that anyone with the job access could act on it now, requiring to enter 
> udpateID any time abort/resume/pause is needed feels redundant and not user 
> friendly. Should we rather drop updateID completely and assume 
> abort/resume/pause would attempt to act on any existing job update (if one 
> exists)?
> 
> Bill Farner wrote:
>     Yes, i think the user-intervention functions should only be scoped by the 
> job key.  Returning the update ID from startJobUpdate is still valid for an 
> external controller to monitor a specific update without risk of crosstalk.
> 
> Maxim Khutornenko wrote:
>     I don't see a risk of a crosstalk. Once startJobUpdate succeeds, it 
> acquires a job lock and any attempt to call it again (e.g. race on retry) 
> will fail. Do you have a specific scenario in mind?
> 
> Bill Farner wrote:
>     Back-to-back updates, or aborted then restarted update.

Discussed offline. The race does not seem to be the problem here. However, we 
probably still want to keep the updateId in the startJobUpdate result to 
facilitate easier monitoring (i.e. start update -> getUpdateDetails) story.


- Maxim


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


On Aug. 15, 2014, 6:14 p.m., Maxim Khutornenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24744/
> -----------------------------------------------------------
> 
> (Updated Aug. 15, 2014, 6:14 p.m.)
> 
> 
> Review request for Aurora, Mark Chu-Carroll and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> The job lock is now acquired by the startJobUpdate call.
> 
> 
> Diffs
> -----
> 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  7ef28858ad290c74248b89c49d2a684eb1c7127e 
>   src/main/python/apache/aurora/client/api/__init__.py 
> 62de93bb942adab47590112b76c365fac7877371 
>   src/main/thrift/org/apache/aurora/gen/api.thrift 
> af9f02ed1de487bc5cc2967d2edcece5b21e0be5 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  649afa24b2cfc9a1d67d350473e439d209bd720c 
>   src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 
> 38bc9ed1ea17cac00920a2f1d066458badd7b4bb 
>   src/test/python/apache/aurora/client/api/test_api.py 
> 96db25d9f7492a0d49e98af27f17b6cee19f5a49 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 
> ab74db34d61e72c50d4ac9252b02cbf69377d194 
>   src/test/resources/org/apache/aurora/gen/api.thrift.md5 
> 21a05f6939da1dd7fc15cf6336bc3fee283f16ab 
>   src/test/resources/org/apache/aurora/gen/storage.thrift.md5 
> 45762990d33969bedde7340887cde16a535e99fe 
> 
> Diff: https://reviews.apache.org/r/24744/diff/
> 
> 
> Testing
> -------
> 
> gradle -Pq build
> ./pants src/test/python/apache/aurora/client/api::
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Reply via email to