Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix rate limiter bug. #244

Merged
merged 1 commit into from
Nov 7, 2014
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@
import com.netflix.appinfo.EurekaClientIdentity;
import com.netflix.eureka.util.EurekaMonitors;
import com.netflix.eureka.util.RateLimiter;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/**
* Rate limiting filter, with configurable threshold above which non-privileged clients
Expand Down Expand Up @@ -81,6 +83,8 @@
*/
public class RateLimitingFilter implements Filter {

private static final Logger logger = LoggerFactory.getLogger(RateLimitingFilter.class);

private static final Set<String> DEFAULT_PRIVILEGED_CLIENTS = new HashSet<String>(
Arrays.asList(EurekaClientIdentity.DEFAULT_CLIENT_NAME, EurekaServerIdentity.DEFAULT_SERVER_NAME)
);
Expand Down Expand Up @@ -127,11 +131,12 @@ private static Target getTarget(ServletRequest request) {
Target target = Target.Other;
if (request instanceof HttpServletRequest) {
HttpServletRequest httpRequest = (HttpServletRequest) request;
String pathInfo = httpRequest.getRequestURI();

if ("GET".equals(httpRequest.getMethod())) {
Matcher matcher = TARGET_RE.matcher(httpRequest.getPathInfo());
if ("GET".equals(httpRequest.getMethod()) && pathInfo != null) {
Matcher matcher = TARGET_RE.matcher(pathInfo);
if (matcher.matches()) {
if (matcher.groupCount() == 0 || "/".equals(matcher.group(1))) {
if (matcher.groupCount() == 0 || matcher.group(1) == null || "/".equals(matcher.group(1))) {
target = Target.FullFetch;
} else if ("/delta".equals(matcher.group(1))) {
target = Target.DeltaFetch;
Expand All @@ -140,12 +145,24 @@ private static Target getTarget(ServletRequest request) {
}
}
}
if (target == Target.Other) {
logger.debug("URL path {} not matched by rate limiting filter", pathInfo);
}
}
return target;
}

private static boolean isRateLimited(HttpServletRequest request, Target target) {
return !isPrivileged(request) && isOverloaded(target);
if (isPrivileged(request)) {
logger.debug("Privileged {} request", target);
return false;
}
if (isOverloaded(target)) {
logger.debug("Overloaded {} request; discarding it", target);
return true;
}
logger.debug("{} request admitted", target);
return false;
}

private static boolean isPrivileged(HttpServletRequest request) {
Expand All @@ -162,7 +179,7 @@ private static boolean isOverloaded(Target target) {
int fetchWindowSize = config().getRateLimiterRegistryFetchAverageRate();
boolean overloaded = !registryFetchRateLimiter.acquire(maxInWindow, fetchWindowSize);

if (target == Target.FullFetch || target == Target.Application) {
if (target == Target.FullFetch) {
int fullFetchWindowSize = config().getRateLimiterFullFetchAverageRate();
overloaded |= !registryFullFetchRateLimiter.acquire(maxInWindow, fullFetchWindowSize);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import javax.ws.rs.core.Response;
import javax.ws.rs.core.Response.Status;

import com.netflix.eureka.util.EurekaMonitors;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand Down Expand Up @@ -75,6 +76,9 @@ public Response getApplication(@PathParam("version") String version,
if (!PeerAwareInstanceRegistry.getInstance().shouldAllowAccess(false)) {
return Response.status(Status.FORBIDDEN).build();
}

EurekaMonitors.GET_APPLICATION.increment();

CurrentRequestVersion.set(Version.toEnum(version));
KeyType keyType = KeyType.JSON;
if (acceptHeader == null || !acceptHeader.contains("json")) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ public enum EurekaMonitors {
GET_ALL("getAllCounter", "Number of total registry queries seen since startup"),
GET_ALL_WITH_REMOTE_REGIONS("getAllWithRemoteRegionCounter",
"Number of total registry queries with remote regions, seen since startup"),
GET_APPLICATION("getApplicationCounter", "Number of total application queries seen since startup"),
REGISTER("registerCounter", "Number of total registers seen since startup"),
EXPIRED("expiredCounter", "Number of total expired leases since startup"),
STATUS_UPDATE("statusUpdateCounter", "Number of total admin status updates since startup"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ public void testCustomClientThrottlingCandidatesCounter() throws Exception {

private void whenRequest(String path, String client) {
when(request.getMethod()).thenReturn("GET");
when(request.getPathInfo()).thenReturn(path);
when(request.getRequestURI()).thenReturn(path);
when(request.getHeader(AbstractEurekaIdentity.AUTH_NAME_HEADER_KEY)).thenReturn(client);
}
}
8 changes: 8 additions & 0 deletions eureka-server/WEB-INF/web.xml
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,14 @@
<url-pattern>/*</url-pattern>
</filter-mapping>

<!-- Uncomment this to enable rate limiter filter.
<filter-mapping>
<filter-name>rateLimitingFilter</filter-name>
<url-pattern>/v2/apps</url-pattern>
<url-pattern>/v2/apps/*</url-pattern>
</filter-mapping>
-->

<filter-mapping>
<filter-name>jersey</filter-name>
<url-pattern>/*</url-pattern>
Expand Down