jenkins-bot has submitted this change and it was merged. ( 
https://gerrit.wikimedia.org/r/370674 )

Change subject: Ensure that wdqs-updater is not throttled.
......................................................................


Ensure that wdqs-updater is not throttled.

We trust the updater to not overload blazegraph, it should not be throttled.

The implementation relies on a HTTP header being set by nginx for all proxied
requests. The updater does not go through nginx but talks directly to
blazegraph, so the header set by nginx is not present.

Bug: T172782

Change-Id: Ic6ba59990d6e74f05b7ac51288fb1b21f85c9da0
---
M 
blazegraph/src/main/java/org/wikidata/query/rdf/blazegraph/throttling/Throttler.java
M 
blazegraph/src/main/java/org/wikidata/query/rdf/blazegraph/throttling/ThrottlingFilter.java
M 
blazegraph/src/test/java/org/wikidata/query/rdf/blazegraph/throttling/ThrottlerTest.java
M war/src/main/webapp/WEB-INF/web.xml
4 files changed, 76 insertions(+), 11 deletions(-)

Approvals:
  Smalyshev: Looks good to me, approved
  jenkins-bot: Verified



diff --git 
a/blazegraph/src/main/java/org/wikidata/query/rdf/blazegraph/throttling/Throttler.java
 
b/blazegraph/src/main/java/org/wikidata/query/rdf/blazegraph/throttling/Throttler.java
index da237d2..2b09df1 100644
--- 
a/blazegraph/src/main/java/org/wikidata/query/rdf/blazegraph/throttling/Throttler.java
+++ 
b/blazegraph/src/main/java/org/wikidata/query/rdf/blazegraph/throttling/Throttler.java
@@ -38,6 +38,17 @@
     private final Callable<ThrottlingState> createThrottlingState;
 
     /**
+     * Throttling is only enabled if this header is set.
+     *
+     * This can be used to throttle only request coming through a revers proxy,
+     * which will set this specific header. Only the presence of the header is
+     * checked, not its value.
+     *
+     * If <code>null</code>, all requests will be throttled.
+     */
+    private String enableThrottlingIfHeader;
+
+    /**
      * Constructor.
      *
      * Note that a bucket represent our approximation of a single client.
@@ -49,16 +60,19 @@
      *                              when we start tracking a specific client
      * @param stateStore the cache in which we store the per client state of
      *                   throttling
+     * @param enableThrottlingIfHeader throttling is only enabled if this 
header is present.
      */
     public Throttler(
             Duration requestTimeThreshold,
             Bucketing<B> bucketing,
             Callable<ThrottlingState> createThrottlingState,
-            Cache<B, ThrottlingState> stateStore) {
+            Cache<B, ThrottlingState> stateStore,
+            String enableThrottlingIfHeader) {
         this.requestTimeThreshold = requestTimeThreshold;
         this.bucketing = bucketing;
         this.state = stateStore;
         this.createThrottlingState = createThrottlingState;
+        this.enableThrottlingIfHeader = enableThrottlingIfHeader;
     }
 
     /**
@@ -68,10 +82,25 @@
      * @return true if the request should be throttled
      */
     public boolean isThrottled(HttpServletRequest request) {
+        if (shouldBypassThrottling(request)) {
+            return false;
+        }
         ThrottlingState throttlingState = 
state.getIfPresent(bucketing.bucket(request));
         if (throttlingState == null) return false;
 
         return throttlingState.isThrottled();
+    }
+
+    /**
+     * Check whether this request should have throttling enabled.
+     * @param request
+     * @return if true then skip throttling
+     */
+    private boolean shouldBypassThrottling(HttpServletRequest request) {
+        if (enableThrottlingIfHeader == null) {
+            return false;
+        }
+        return request.getHeader(enableThrottlingIfHeader) == null;
     }
 
     /**
@@ -81,6 +110,9 @@
      * @param elapsed how long that request took
      */
     public void success(HttpServletRequest request, Duration elapsed) {
+        if (shouldBypassThrottling(request)) {
+            return;
+        }
         try {
             B bucket = bucketing.bucket(request);
             ThrottlingState throttlingState;
@@ -105,6 +137,9 @@
      * @param elapsed how long that request took
      */
     public void failure(HttpServletRequest request, Duration elapsed) {
+        if (shouldBypassThrottling(request)) {
+            return;
+        }
         try {
             ThrottlingState throttlingState = 
state.get(bucketing.bucket(request), createThrottlingState);
 
diff --git 
a/blazegraph/src/main/java/org/wikidata/query/rdf/blazegraph/throttling/ThrottlingFilter.java
 
b/blazegraph/src/main/java/org/wikidata/query/rdf/blazegraph/throttling/ThrottlingFilter.java
index 72a1198..9477ed4 100644
--- 
a/blazegraph/src/main/java/org/wikidata/query/rdf/blazegraph/throttling/ThrottlingFilter.java
+++ 
b/blazegraph/src/main/java/org/wikidata/query/rdf/blazegraph/throttling/ThrottlingFilter.java
@@ -96,14 +96,13 @@
     /**
      * Initialise the filter.
      *
-     * Configuration is loaded from TODO.
+     * See {@link ThrottlingFilter#loadStringParam(String, FilterConfig)} for
+     * the details of where the configuration is loaded from.
+     *
      * @param filterConfig {@inheritDoc}
      */
     @Override
     public void init(FilterConfig filterConfig) throws ServletException {
-        // TODO: the value of the parameters are mostly invented, we need to
-        // find the correct ones.
-
         int requestDurationThresholdInSeconds = 
loadIntParam("request-duration-threshold-in-seconds", filterConfig, 10);
         int timeBucketCapacityInSeconds = 
loadIntParam("time-bucket-capacity-in-seconds", filterConfig, 60);
         int timeBucketRefillAmountInSeconds = 
loadIntParam("time-bucket-refill-amount-in-seconds", filterConfig, 60);
@@ -113,6 +112,8 @@
         int errorBucketRefillPeriodInMinutes = 
loadIntParam("error-bucket-refill-period-in-minutes", filterConfig, 1);
         int maxStateSize = loadIntParam("max-state-size", filterConfig, 10000);
         int stateExpirationInMinutes = 
loadIntParam("state-expiration-in-minutes", filterConfig, 15);
+
+        String enableThrottlingIfHeader = 
loadStringParam("enable-throttling-if-header", filterConfig);
 
         this.enabled = loadBooleanParam("enabled", filterConfig, true);
         throttler = new Throttler<>(
@@ -128,7 +129,8 @@
                 CacheBuilder.newBuilder()
                         .maximumSize(maxStateSize)
                         .expireAfterAccess(stateExpirationInMinutes, 
TimeUnit.MINUTES)
-                        .build());
+                        .build(),
+                enableThrottlingIfHeader);
     }
 
     /**
diff --git 
a/blazegraph/src/test/java/org/wikidata/query/rdf/blazegraph/throttling/ThrottlerTest.java
 
b/blazegraph/src/test/java/org/wikidata/query/rdf/blazegraph/throttling/ThrottlerTest.java
index 53b0a5a..58ff756 100644
--- 
a/blazegraph/src/test/java/org/wikidata/query/rdf/blazegraph/throttling/ThrottlerTest.java
+++ 
b/blazegraph/src/test/java/org/wikidata/query/rdf/blazegraph/throttling/ThrottlerTest.java
@@ -33,11 +33,7 @@
                 .maximumSize(10000)
                 .expireAfterAccess(15, TimeUnit.MINUTES)
                 .build();
-        throttler = new Throttler<>(
-                Duration.of(20, SECONDS),
-                new UserAgentIpAddressBucketing(),
-                createThrottlingState(),
-                stateStore);
+        throttler = createThrottlerWithThrottlingHeader(stateStore, null);
     }
 
     private static Callable<ThrottlingState> createThrottlingState() {
@@ -157,6 +153,34 @@
         assertThat(backoffDelay.compareTo(Duration.of(60, SECONDS)), 
lessThan(0));
     }
 
+    @Test
+    public void throttleOnlyRequestsWithThrottlingHeader() {
+        MockHttpServletRequest request = createRequest("UA1", "1.2.3.4");
+        request.addHeader("do-throttle", "ignored value");
+
+        throttler = createThrottlerWithThrottlingHeader(stateStore, 
"do-throttle");
+
+        throttler.success(request, Duration.of(60, SECONDS));
+
+        assertTrue(throttler.isThrottled(request));
+
+        request = createRequest("UA1", "1.2.3.4");
+
+        throttler.success(request, Duration.of(60, SECONDS));
+
+        assertFalse(throttler.isThrottled(request));
+    }
+
+    private Throttler<Bucket> createThrottlerWithThrottlingHeader(
+            Cache<Bucket, ThrottlingState> stateStore,
+            String enableThrottlingIfHeader) {
+        return new Throttler<>(
+                Duration.of(20, SECONDS),
+                new UserAgentIpAddressBucketing(),
+                createThrottlingState(),
+                stateStore,
+                enableThrottlingIfHeader);
+    }
 
     private MockHttpServletRequest createRequest(String userAgent, String 
remoteAddr) {
         MockHttpServletRequest request = new MockHttpServletRequest();
diff --git a/war/src/main/webapp/WEB-INF/web.xml 
b/war/src/main/webapp/WEB-INF/web.xml
index d6a33e0..ff2ea0e 100644
--- a/war/src/main/webapp/WEB-INF/web.xml
+++ b/war/src/main/webapp/WEB-INF/web.xml
@@ -103,6 +103,10 @@
   <filter>
       <filter-name>throttling-filter</filter-name>
       
<filter-class>org.wikidata.query.rdf.blazegraph.throttling.ThrottlingFilter</filter-class>
+      <init-param>
+          <param-name>enable-throttling-if-header</param-name>
+          <param-value>X-BIGDATA-READ-ONLY</param-value>
+      </init-param>
   </filter>
   <filter-mapping>
     <filter-name>throttling-filter</filter-name>

-- 
To view, visit https://gerrit.wikimedia.org/r/370674
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Ic6ba59990d6e74f05b7ac51288fb1b21f85c9da0
Gerrit-PatchSet: 3
Gerrit-Project: wikidata/query/rdf
Gerrit-Branch: master
Gerrit-Owner: Gehel <guillaume.leder...@wikimedia.org>
Gerrit-Reviewer: Smalyshev <smalys...@wikimedia.org>
Gerrit-Reviewer: jenkins-bot <>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to