This is an automated email from the ASF dual-hosted git repository. bmahler pushed a commit to branch 1.5.x in repository https://gitbox.apache.org/repos/asf/mesos.git
commit 0178e4fcb68136ac9cf0fda12e94b781e45244c7 Author: Benjamin Mahler <bmah...@apache.org> AuthorDate: Fri Aug 10 17:14:05 2018 -0700 Fixed an authentication request amplification issue in the master. Per MESOS-9144, re-enqueuing authentication requests leads to an amplification effect which can overwhelm the master if requests continue to arrive rapidly on a heavily loaded master. This patch avoids the re-enqueing and ensures the master immediately processes a new authentication request for a client. Review: https://reviews.apache.org/r/68306 --- src/master/master.cpp | 36 ++++++++++++++++++++---------------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/src/master/master.cpp b/src/master/master.cpp index 14db7c4..5fcdd63 100644 --- a/src/master/master.cpp +++ b/src/master/master.cpp @@ -9022,7 +9022,7 @@ void Master::authenticate(const UPID& from, const UPID& pid) // about this discrepancy via ping messages so that it can // re-register. - authenticated.erase(pid); + bool erased = authenticated.erase(pid) > 0; if (authenticator.isNone()) { // The default authenticator is CRAM-MD5 rather than none. @@ -9045,30 +9045,28 @@ void Master::authenticate(const UPID& from, const UPID& pid) return; } + // If a new authentication is occurring for a client that already + // has an authentication in progress, we discard the old one + // (since the client is no longer interested in it) and + // immediately proceed with the new authentication. if (authenticating.contains(pid)) { - LOG(INFO) << "Queuing up authentication request from " << pid - << " because authentication is still in progress"; - - // Try to cancel the in progress authentication by discarding the - // future. - authenticating[pid].discard(); - - // Retry after the current authenticator session finishes. - authenticating[pid] - .onAny(defer(self(), &Self::authenticate, from, pid)); + authenticating.at(pid).discard(); + authenticating.erase(pid); - return; + LOG(INFO) << "Re-authenticating " << pid << ";" + << " discarding outstanding authentication"; + } else { + LOG(INFO) << "Authenticating " << pid + << (erased ? "; clearing previous authentication" : ""); } - LOG(INFO) << "Authenticating " << pid; - // Start authentication. const Future<Option<string>> future = authenticator.get()->authenticate(from); // Save our state. authenticating[pid] = future; - future.onAny(defer(self(), &Self::_authenticate, pid, lambda::_1)); + future.onAny(defer(self(), &Self::_authenticate, pid, future)); // Don't wait for authentication to complete forever. delay(flags.authentication_v0_timeout, @@ -9082,6 +9080,13 @@ void Master::_authenticate( const UPID& pid, const Future<Option<string>>& future) { + // Ignore stale authentication results (if the authentication + // future has been overwritten). + if (authenticating.get(pid) != future) { + LOG(INFO) << "Ignoring stale authentication result of " << pid; + return; + } + if (future.isReady() && future->isSome()) { LOG(INFO) << "Successfully authenticated principal '" << future->get() << "' at " << pid; @@ -9097,7 +9102,6 @@ void Master::_authenticate( LOG(INFO) << "Authentication of " << pid << " was discarded"; } - CHECK(authenticating.contains(pid)); authenticating.erase(pid); }