madrob commented on a change in pull request #1686:
URL: https://github.com/apache/lucene-solr/pull/1686#discussion_r466985880
##########
File path:
solr/core/src/test/org/apache/solr/servlet/TestRequestRateLimiter.java
##########
@@ -102,31 +103,101 @@ public Boolean call() throws Exception {
try {
future.get();
} catch (Exception e) {
- assertTrue("Not true " + e.getMessage(),
e.getMessage().contains("non ok status: 429, message:Too Many Requests"));
+ assertThat(e.getMessage(), containsString("non ok status: 429,
message:Too Many Requests"));
}
}
MockRequestRateLimiter mockQueryRateLimiter = (MockRequestRateLimiter)
rateLimitManager.getRequestRateLimiter(SolrRequest.SolrRequestType.QUERY);
- assertTrue("Incoming request count did not match. Expected == 25
incoming " + mockQueryRateLimiter.incomingRequestCount.get(),
- mockQueryRateLimiter.incomingRequestCount.get() == 25);
+ assertEquals(mockQueryRateLimiter.incomingRequestCount.get(),25);
assertTrue("Incoming accepted new request count did not match. Expected
5 incoming " + mockQueryRateLimiter.acceptedNewRequestCount.get(),
mockQueryRateLimiter.acceptedNewRequestCount.get() < 25);
assertTrue("Incoming rejected new request count did not match. Expected
20 incoming " + mockQueryRateLimiter.rejectedRequestCount.get(),
mockQueryRateLimiter.rejectedRequestCount.get() > 0);
- assertTrue("Incoming total processed requests count did not match.
Expected " + mockQueryRateLimiter.incomingRequestCount.get() + " incoming "
- + (mockQueryRateLimiter.acceptedNewRequestCount.get() +
mockQueryRateLimiter.rejectedRequestCount.get()),
- (mockQueryRateLimiter.acceptedNewRequestCount.get() +
mockQueryRateLimiter.rejectedRequestCount.get()) ==
mockQueryRateLimiter.incomingRequestCount.get());
+ assertEquals(mockQueryRateLimiter.acceptedNewRequestCount.get() +
mockQueryRateLimiter.rejectedRequestCount.get(),
+ mockQueryRateLimiter.incomingRequestCount.get());
+ } finally {
+ executor.shutdown();
+ }
+ }
+
+ @Test
+ public void testSlotBorrowing() throws Exception {
+ CloudSolrClient client = cluster.getSolrClient();
+ client.setDefaultCollection(SECOND_COLLECTION);
+
+ CollectionAdminRequest.createCollection(SECOND_COLLECTION, 1,
1).process(client);
+ cluster.waitForActiveCollection(SECOND_COLLECTION, 1, 1);
+
+
+ SolrDispatchFilter solrDispatchFilter =
cluster.getJettySolrRunner(0).getSolrDispatchFilter();
+
+ RequestRateLimiter.RateLimiterConfig queryRateLimiterConfig = new
RequestRateLimiter.RateLimiterConfig(SolrRequest.SolrRequestType.QUERY,
+ true, 1, DEFAULT_SLOT_ACQUISITION_TIMEOUT_MS, 5 /* allowedRequests */,
true /* isSlotBorrowing */);
+ RequestRateLimiter.RateLimiterConfig indexRateLimiterConfig = new
RequestRateLimiter.RateLimiterConfig(SolrRequest.SolrRequestType.UPDATE,
+ true, 1, DEFAULT_SLOT_ACQUISITION_TIMEOUT_MS, 5 /* allowedRequests */,
true /* isSlotBorrowing */);
+ // We are fine with a null FilterConfig here since we ensure that
MockBuilder never invokes its parent
+ RateLimitManager.Builder builder = new MockBuilder(null /*dummy
FilterConfig */, new MockRequestRateLimiter(queryRateLimiterConfig, 5), new
MockRequestRateLimiter(indexRateLimiterConfig, 5));
+ RateLimitManager rateLimitManager = builder.build();
+
+ solrDispatchFilter.replaceRateLimitManager(rateLimitManager);
+
+ for (int i = 0; i < 100; i++) {
+ SolrInputDocument doc = new SolrInputDocument();
+
+ doc.setField("id", i);
+ doc.setField("text", "foo");
+ client.add(doc);
+ }
+
+ client.commit();
+
+ ExecutorService executor =
ExecutorUtil.newMDCAwareCachedThreadPool("threadpool");
+ List<Callable<Boolean>> callableList = new ArrayList<>();
+ List<Future<Boolean>> futures;
+
+ try {
+ for (int i = 0; i < 25; i++) {
+ callableList.add(() -> {
+ try {
+ QueryResponse response = client.query(new SolrQuery("*:*"));
+
+ if (response.getResults().getNumFound() > 0) {
+ assertEquals(100, response.getResults().getNumFound());
+ }
+ } catch (Exception e) {
+ throw new RuntimeException(e.getMessage());
+ }
+
+ return true;
+ });
+ }
+
+ futures = executor.invokeAll(callableList);
+
+ for (Future<?> future : futures) {
+ try {
+ future.get();
Review comment:
I’m just going by memory here, but I thought future.get gives you the
return value of the callable which should always be true in this case for
success?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
