launchpad-reviewers team mailing list archive

["Jeroen T. Vermeulen" <jtv@xxxxxxxxxxxxx>]

This branch does not change the idea of using a single CobblerSession for the whole provisioning server, and thus there's still just a *single* authentication attempt in flight.  And the optimistic "cookie check" ensures that no callback chain will overwrite an authentication cookie that's already been refreshed concurrently.

Let me illustrate what happens when the auth token goes stale.  Requests start failing.  They vie for the authentication lock.  Exactly one callback chain obtains the lock and re-authenticates.  All of the requests that hit authentication errors call self.authenticate, and so they all wait for the lock in there.

Finally, authentication succeeds and the lock is released.  Now, one at a time in basically the same way they would execute anyway, the other requests obtain the lock.  Each immediately discovers that there is no further need to re-authenticate (because the session state cookie has changed since it first issued its call), and releases the lock immediately.  It then restarts its ongoing API call using the new authentication token.  So you've got the "quick return" right there.

Despite being optimistic, the approach is also conservative.  It would work just as well if we had proper preemptive threads, or separate processes, or purely synchronous code: apart from the explicitly locked section there's no window where it relies on there being no context switches.

The tests, of course, are synchronous.  The check itself is tested in test_authenticate_backs_off_from_overwriting_concurrent_auth, and the re-authenticate/re-issue is tested in test_call_reauthenticates_and_retries_on_auth_failure.  I just had an idea for an additional test: make sure that the fail/authenticate/retry sequence compares the right state cookies.

Yes, a drawback of the asynchronous approach is that the provisioning server happily continues issuing requests with a stale token when it's already known to be invalid.  All of those will fail, queue up, and be retried.  I originally prevented that by using synchronous XMLRPC for the login, but if it's a problem we can find other ways around it.

For example we could clear the auth token on error, and skip the initial attempt at issuing a call if the token is null.  That works by sending requests during the re-authentication window into the critical section.  They won't do anything in that section, but it tells the reactor to queue up those callbacks until the authentication is done.
-- 
https://code.launchpad.net/~jtv/maas/async-cobbler-login/+merge/90282
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/maas/async-cobbler-login into lp:maas.