[MediaWiki-commits] [Gerrit] wikidata...rdf[master]: Ensure that wdqs-updater is not throttled.

2017-08-08 Thread jenkins-bot (Code Review) 
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 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 null, 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 bucketing,
 Callable createThrottlingState,
-Cache stateStore) {
+Cache 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

[MediaWiki-commits] [Gerrit] wikidata...rdf[master]: Ensure that wdqs-updater is not throttled.

2017-08-08 Thread Gehel (Code Review) 
Gehel has uploaded a new change for review. ( 
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.

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, 71 insertions(+), 11 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/wikidata/query/rdf 
refs/changes/74/370674/1

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..30c1186 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 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 null, 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 bucketing,
 Callable createThrottlingState,
-Cache stateStore) {
+Cache stateStore,
+String enableThrottlingIfHeader) {
 this.requestTimeThreshold = requestTimeThreshold;
 this.bucketing = bucketing;
 this.state = stateStore;
 this.createThrottlingState = createThrottlingState;
+this.enableThrottlingIfHeader = enableThrottlingIfHeader;
 }
 
 /**
@@ -68,10 +82,20 @@
  * @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();
+}
+
+private boolean shouldBypassThrottling(HttpServletRequest request) {
+if (enableThrottlingIfHeader == null) {
+return false;
+}
+return request.getHeader(enableThrottlingIfHeader) == null;
 }
 
 /**
@@ -81,6 +105,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 +132,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.
+ *