> On Jan. 27, 2015, 5:49 p.m., Joshua Cohen wrote: > > src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java, > > line 227 > > <https://reviews.apache.org/r/30225/diff/1/?file=832014#file832014line227> > > > > Given how sensitive we are to storage lock contention, is there any > > work in this block that can be done outside of the lock? From a casual > > glance the only potentially time-consuming (non-write) operation is the > > fetch from the job update store, but I'm not sure how intensive an > > operation that will be (or the impact of performing it outside of the > > context of a lock). > > Maxim Khutornenko wrote: > Moving a fetch call out of the write transaction would create a potential > for a race that may compromise the feature and create hard to reason/trace > corner cases. This is not worth the arguable gain in contention improvement > in my opinion. > > David McLaughlin wrote: > AFAICT the worst race condition is sending a heartbeat for a job that is > already complete. Are there other race conditions we are protecting against? > > Also, I don't see the value in persisting the heartbeats. What is the > rationale for this?
"AFAICT the worst race condition is sending a heartbeat for a job that is already complete. Are there other race conditions we are protecting against?" - anything inside the evaluation logic is sensitive to the data provided in the details pull. This ranges from an invalid "OK" request sent back for a completed update to an internal updater failure due to outdated expectations. This pattern of querying inside of a write transaction is common in the updater (e.g. look at changeJobUpdateStatus() called any time an update is evaluated) and I'd rather not second-guess the correctness of the split. "Also, I don't see the value in persisting the heartbeats. What is the rationale for this?" - what makes you think they are persisted? The heartbeats are stored in-memory only (look for coordinatedUpdateStates map). - Maxim ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30225/#review69834 ----------------------------------------------------------- On Jan. 23, 2015, 8:37 p.m., Maxim Khutornenko wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/30225/ > ----------------------------------------------------------- > > (Updated Jan. 23, 2015, 8:37 p.m.) > > > Review request for Aurora, David McLaughlin, Joshua Cohen, and Bill Farner. > > > Bugs: AURORA-1010 > https://issues.apache.org/jira/browse/AURORA-1010 > > > Repository: aurora > > > Description > ------- > > Added pulsing support into the JobUpdateController. The qualified coordinated > updates get blocked until a pulse arrives. An update then becomes active and > proceeds until `blockIfNoPulsesAfterMs` expires or the update reaches a > terminal state (whichever comes first). > > Not particularly happy with plumbing through OneWayJobUpdater but the > alternative is a state machine change, which is much hairier and will require > more changes in the JobUpdaterController. Going with the minimal diff here. > > > Diffs > ----- > > src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java > d3b30d48b76d8d7c64cda006a34f7ed3296526f2 > > src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java > a992938d4e12b20f81608be6bbdc24c0a211c3fd > src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdater.java > 27a5b9026f5ac3b3bdeb32813b10435bc3dab173 > src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java > 4c827b183a87b4d97774edbfaa960bd1c3de72a5 > src/test/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterTest.java > 7d0a7438b4a517e5e0d44f4e99aceb1a6d19f987 > > Diff: https://reviews.apache.org/r/30225/diff/ > > > Testing > ------- > > ./gradlew -Pq build > > > Thanks, > > Maxim Khutornenko > >
