Fix plugin component configuration. Project: http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/commit/4d0c850d Tree: http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/tree/4d0c850d Diff: http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/diff/4d0c850d
Branch: refs/heads/api_limit Commit: 4d0c850dc82509d66dc88eed05586ee06f682393 Parents: 57e67c5 Author: Min Chen <[email protected]> Authored: Mon Jan 14 17:13:18 2013 -0800 Committer: Min Chen <[email protected]> Committed: Mon Jan 14 17:13:18 2013 -0800 ---------------------------------------------------------------------- .../org/apache/cloudstack/acl/APILimitChecker.java | 4 +- client/pom.xml | 5 + client/tomcatconf/components.xml.in | 8 ++ .../command/admin/ratelimit/ResetApiLimitCmd.java | 94 +++++++++++++++ .../api/command/user/ratelimit/GetApiLimitCmd.java | 2 +- .../commands/admin/ratelimit/ResetApiLimitCmd.java | 94 --------------- .../cloudstack/ratelimit/ApiRateLimitService.java | 2 +- .../ratelimit/ApiRateLimitServiceImpl.java | 75 +++++++----- .../cloudstack/ratelimit/StoreEntryImpl.java | 2 +- .../cloudstack/ratelimit/ApiRateLimitTest.java | 41 ++++--- server/src/com/cloud/api/ApiServer.java | 20 ++-- server/src/com/cloud/configuration/Config.java | 4 +- server/test/com/cloud/api/ListPerfTest.java | 54 ++++++++ setup/db/db/schema-40to410.sql | 2 + 14 files changed, 243 insertions(+), 164 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/blob/4d0c850d/api/src/org/apache/cloudstack/acl/APILimitChecker.java ---------------------------------------------------------------------- diff --git a/api/src/org/apache/cloudstack/acl/APILimitChecker.java b/api/src/org/apache/cloudstack/acl/APILimitChecker.java index 3a1db70..110742c 100644 --- a/api/src/org/apache/cloudstack/acl/APILimitChecker.java +++ b/api/src/org/apache/cloudstack/acl/APILimitChecker.java @@ -16,6 +16,8 @@ // under the License. package org.apache.cloudstack.acl; +import org.apache.cloudstack.api.ServerApiException; + import com.cloud.user.Account; import com.cloud.utils.component.Adapter; @@ -24,5 +26,5 @@ import com.cloud.utils.component.Adapter; */ public interface APILimitChecker extends Adapter { // Interface for checking if the account is over its api limit - boolean isUnderLimit(Account account); + void checkLimit(Account account) throws ServerApiException; } http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/blob/4d0c850d/client/pom.xml ---------------------------------------------------------------------- diff --git a/client/pom.xml b/client/pom.xml index 1bbae1f..7ebe50c 100644 --- a/client/pom.xml +++ b/client/pom.xml @@ -32,6 +32,11 @@ </dependency> <dependency> <groupId>org.apache.cloudstack</groupId> + <artifactId>cloud-plugin-api-limit-account-based</artifactId> + <version>${project.version}</version> + </dependency> + <dependency> + <groupId>org.apache.cloudstack</groupId> <artifactId>cloud-plugin-api-discovery</artifactId> <version>${project.version}</version> </dependency> http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/blob/4d0c850d/client/tomcatconf/components.xml.in ---------------------------------------------------------------------- diff --git a/client/tomcatconf/components.xml.in b/client/tomcatconf/components.xml.in index bb39839..dcff94a 100755 --- a/client/tomcatconf/components.xml.in +++ b/client/tomcatconf/components.xml.in @@ -56,6 +56,9 @@ under the License. <adapters key="org.apache.cloudstack.acl.APIChecker"> <adapter name="StaticRoleBasedAPIAccessChecker" class="org.apache.cloudstack.acl.StaticRoleBasedAPIAccessChecker"/> </adapters> + <adapters key="org.apache.cloudstack.acl.APILimitChecker"> + <adapter name="AccountBasedAPIRateLimit" class="org.apache.cloudstack.ratelimit.ApiRateLimitServiceImpl" singleton="true"/> + </adapters> <adapters key="com.cloud.agent.manager.allocator.HostAllocator"> <adapter name="FirstFitRouting" class="com.cloud.agent.manager.allocator.impl.FirstFitRoutingAllocator"/> <!--adapter name="FirstFitRouting" class="com.cloud.agent.manager.allocator.impl.RecreateHostAllocator"/--> @@ -180,6 +183,11 @@ under the License. <pluggableservice name="ApiDiscoveryService" key="org.apache.cloudstack.discovery.ApiDiscoveryService" class="org.apache.cloudstack.discovery.ApiDiscoveryServiceImpl"/> <pluggableservice name="VirtualRouterElementService" key="com.cloud.network.element.VirtualRouterElementService" class="com.cloud.network.element.VirtualRouterElement"/> <pluggableservice name="NiciraNvpElementService" key="com.cloud.network.element.NiciraNvpElementService" class="com.cloud.network.element.NiciraNvpElement"/> + <pluggableservice name="ApiRateLimitService" key="org.apache.cloudstack.api.ratelimit.ApiRateLimitService" class="org.apache.cloudstack.ratelimit.ApiRateLimitServiceImpl"> + <param name="api.throttling.interval">1</param> + <param name="api.throttling.max">25</param> + <param name="api.throttling.cachesize">50000</param> + </pluggableservice> <dao name="OvsTunnelInterfaceDao" class="com.cloud.network.ovs.dao.OvsTunnelInterfaceDaoImpl" singleton="false"/> <dao name="OvsTunnelAccountDao" class="com.cloud.network.ovs.dao.OvsTunnelNetworkDaoImpl" singleton="false"/> <dao name="NiciraNvpDao" class="com.cloud.network.dao.NiciraNvpDaoImpl" singleton="false"/> http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/blob/4d0c850d/plugins/api/rate-limit/src/org/apache/cloudstack/api/command/admin/ratelimit/ResetApiLimitCmd.java ---------------------------------------------------------------------- diff --git a/plugins/api/rate-limit/src/org/apache/cloudstack/api/command/admin/ratelimit/ResetApiLimitCmd.java b/plugins/api/rate-limit/src/org/apache/cloudstack/api/command/admin/ratelimit/ResetApiLimitCmd.java new file mode 100644 index 0000000..3c612fa --- /dev/null +++ b/plugins/api/rate-limit/src/org/apache/cloudstack/api/command/admin/ratelimit/ResetApiLimitCmd.java @@ -0,0 +1,94 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package org.apache.cloudstack.api.command.admin.ratelimit; + +import org.apache.cloudstack.api.*; +import org.apache.cloudstack.api.response.AccountResponse; +import org.apache.cloudstack.api.response.ApiLimitResponse; +import org.apache.cloudstack.api.response.SuccessResponse; +import org.apache.log4j.Logger; + +import org.apache.cloudstack.api.APICommand; +import org.apache.cloudstack.ratelimit.ApiRateLimitService; + +import com.cloud.user.Account; +import com.cloud.user.UserContext; + +@APICommand(name = "resetApiLimit", responseObject=ApiLimitResponse.class, description="Reset api count") +public class ResetApiLimitCmd extends BaseCmd { + private static final Logger s_logger = Logger.getLogger(ResetApiLimitCmd.class.getName()); + + private static final String s_name = "resetapilimitresponse"; + + @PlugService + ApiRateLimitService _apiLimitService; + + ///////////////////////////////////////////////////// + //////////////// API parameters ///////////////////// + ///////////////////////////////////////////////////// + + @ACL + @Parameter(name=ApiConstants.ACCOUNT, type=CommandType.UUID, entityType=AccountResponse.class, + description="the ID of the acount whose limit to be reset") + private Long accountId; + + ///////////////////////////////////////////////////// + /////////////////// Accessors /////////////////////// + ///////////////////////////////////////////////////// + + + public Long getAccountId() { + return accountId; + } + + + public void setAccountId(Long accountId) { + this.accountId = accountId; + } + + + ///////////////////////////////////////////////////// + /////////////// API Implementation/////////////////// + ///////////////////////////////////////////////////// + + + @Override + public String getCommandName() { + return s_name; + } + + @Override + public long getEntityOwnerId() { + Account account = UserContext.current().getCaller(); + if (account != null) { + return account.getId(); + } + + return Account.ACCOUNT_ID_SYSTEM; // no account info given, parent this command to SYSTEM so ERROR events are tracked + } + + @Override + public void execute(){ + boolean result = _apiLimitService.resetApiLimit(this); + if (result) { + SuccessResponse response = new SuccessResponse(getCommandName()); + this.setResponseObject(response); + } else { + throw new ServerApiException(BaseCmd.INTERNAL_ERROR, "Failed to reset api limit counter"); + } + } +} http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/blob/4d0c850d/plugins/api/rate-limit/src/org/apache/cloudstack/api/command/user/ratelimit/GetApiLimitCmd.java ---------------------------------------------------------------------- diff --git a/plugins/api/rate-limit/src/org/apache/cloudstack/api/command/user/ratelimit/GetApiLimitCmd.java b/plugins/api/rate-limit/src/org/apache/cloudstack/api/command/user/ratelimit/GetApiLimitCmd.java index 3f9e4eb..0397fa8 100644 --- a/plugins/api/rate-limit/src/org/apache/cloudstack/api/command/user/ratelimit/GetApiLimitCmd.java +++ b/plugins/api/rate-limit/src/org/apache/cloudstack/api/command/user/ratelimit/GetApiLimitCmd.java @@ -27,7 +27,7 @@ import org.apache.cloudstack.api.Parameter; import org.apache.cloudstack.api.PlugService; import org.apache.cloudstack.api.ServerApiException; import org.apache.cloudstack.api.BaseCmd.CommandType; -import org.apache.cloudstack.api.commands.admin.ratelimit.ResetApiLimitCmd; +import org.apache.cloudstack.api.command.admin.ratelimit.ResetApiLimitCmd; import org.apache.cloudstack.api.response.AccountResponse; import org.apache.cloudstack.api.response.ApiLimitResponse; import org.apache.cloudstack.api.response.PhysicalNetworkResponse; http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/blob/4d0c850d/plugins/api/rate-limit/src/org/apache/cloudstack/api/commands/admin/ratelimit/ResetApiLimitCmd.java ---------------------------------------------------------------------- diff --git a/plugins/api/rate-limit/src/org/apache/cloudstack/api/commands/admin/ratelimit/ResetApiLimitCmd.java b/plugins/api/rate-limit/src/org/apache/cloudstack/api/commands/admin/ratelimit/ResetApiLimitCmd.java deleted file mode 100644 index 8029ab3..0000000 --- a/plugins/api/rate-limit/src/org/apache/cloudstack/api/commands/admin/ratelimit/ResetApiLimitCmd.java +++ /dev/null @@ -1,94 +0,0 @@ -// Licensed to the Apache Software Foundation (ASF) under one -// or more contributor license agreements. See the NOTICE file -// distributed with this work for additional information -// regarding copyright ownership. The ASF licenses this file -// to you under the Apache License, Version 2.0 (the -// "License"); you may not use this file except in compliance -// with the License. You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, -// software distributed under the License is distributed on an -// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY -// KIND, either express or implied. See the License for the -// specific language governing permissions and limitations -// under the License. -package org.apache.cloudstack.api.commands.admin.ratelimit; - -import org.apache.cloudstack.api.*; -import org.apache.cloudstack.api.response.AccountResponse; -import org.apache.cloudstack.api.response.ApiLimitResponse; -import org.apache.cloudstack.api.response.SuccessResponse; -import org.apache.log4j.Logger; - -import org.apache.cloudstack.api.APICommand; -import org.apache.cloudstack.ratelimit.ApiRateLimitService; - -import com.cloud.user.Account; -import com.cloud.user.UserContext; - -@APICommand(name = "resetApiLimit", responseObject=ApiLimitResponse.class, description="Reset api count") -public class ResetApiLimitCmd extends BaseCmd { - private static final Logger s_logger = Logger.getLogger(ResetApiLimitCmd.class.getName()); - - private static final String s_name = "resetapilimitresponse"; - - @PlugService - ApiRateLimitService _apiLimitService; - - ///////////////////////////////////////////////////// - //////////////// API parameters ///////////////////// - ///////////////////////////////////////////////////// - - @ACL - @Parameter(name=ApiConstants.ACCOUNT, type=CommandType.UUID, entityType=AccountResponse.class, - description="the ID of the acount whose limit to be reset") - private Long accountId; - - ///////////////////////////////////////////////////// - /////////////////// Accessors /////////////////////// - ///////////////////////////////////////////////////// - - - public Long getAccountId() { - return accountId; - } - - - public void setAccountId(Long accountId) { - this.accountId = accountId; - } - - - ///////////////////////////////////////////////////// - /////////////// API Implementation/////////////////// - ///////////////////////////////////////////////////// - - - @Override - public String getCommandName() { - return s_name; - } - - @Override - public long getEntityOwnerId() { - Account account = UserContext.current().getCaller(); - if (account != null) { - return account.getId(); - } - - return Account.ACCOUNT_ID_SYSTEM; // no account info given, parent this command to SYSTEM so ERROR events are tracked - } - - @Override - public void execute(){ - boolean result = _apiLimitService.resetApiLimit(this); - if (result) { - SuccessResponse response = new SuccessResponse(getCommandName()); - this.setResponseObject(response); - } else { - throw new ServerApiException(BaseCmd.INTERNAL_ERROR, "Failed to reset api limit counter"); - } - } -} http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/blob/4d0c850d/plugins/api/rate-limit/src/org/apache/cloudstack/ratelimit/ApiRateLimitService.java ---------------------------------------------------------------------- diff --git a/plugins/api/rate-limit/src/org/apache/cloudstack/ratelimit/ApiRateLimitService.java b/plugins/api/rate-limit/src/org/apache/cloudstack/ratelimit/ApiRateLimitService.java index e7ba9d4..8c9d49b 100644 --- a/plugins/api/rate-limit/src/org/apache/cloudstack/ratelimit/ApiRateLimitService.java +++ b/plugins/api/rate-limit/src/org/apache/cloudstack/ratelimit/ApiRateLimitService.java @@ -16,8 +16,8 @@ // under the License. package org.apache.cloudstack.ratelimit; +import org.apache.cloudstack.api.command.admin.ratelimit.ResetApiLimitCmd; import org.apache.cloudstack.api.command.user.ratelimit.GetApiLimitCmd; -import org.apache.cloudstack.api.commands.admin.ratelimit.ResetApiLimitCmd; import org.apache.cloudstack.api.response.ApiLimitResponse; import org.apache.cloudstack.api.response.ListResponse; http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/blob/4d0c850d/plugins/api/rate-limit/src/org/apache/cloudstack/ratelimit/ApiRateLimitServiceImpl.java ---------------------------------------------------------------------- diff --git a/plugins/api/rate-limit/src/org/apache/cloudstack/ratelimit/ApiRateLimitServiceImpl.java b/plugins/api/rate-limit/src/org/apache/cloudstack/ratelimit/ApiRateLimitServiceImpl.java index e14f65d..00f39af 100644 --- a/plugins/api/rate-limit/src/org/apache/cloudstack/ratelimit/ApiRateLimitServiceImpl.java +++ b/plugins/api/rate-limit/src/org/apache/cloudstack/ratelimit/ApiRateLimitServiceImpl.java @@ -25,19 +25,18 @@ import net.sf.ehcache.CacheManager; import org.apache.log4j.Logger; -import com.cloud.configuration.Config; -import com.cloud.configuration.dao.ConfigurationDao; import org.apache.cloudstack.acl.APILimitChecker; +import org.apache.cloudstack.api.BaseCmd; +import org.apache.cloudstack.api.ServerApiException; +import org.apache.cloudstack.api.command.admin.ratelimit.ResetApiLimitCmd; import org.apache.cloudstack.api.command.user.ratelimit.GetApiLimitCmd; -import org.apache.cloudstack.api.commands.admin.ratelimit.ResetApiLimitCmd; import org.apache.cloudstack.api.response.ApiLimitResponse; -import com.cloud.network.element.NetworkElement; import com.cloud.user.Account; import com.cloud.user.UserContext; +import com.cloud.utils.PropertiesUtil; import com.cloud.utils.component.AdapterBase; -import com.cloud.utils.component.Inject; -@Local(value = NetworkElement.class) +@Local(value = APILimitChecker.class) public class ApiRateLimitServiceImpl extends AdapterBase implements APILimitChecker, ApiRateLimitService { private static final Logger s_logger = Logger.getLogger(ApiRateLimitServiceImpl.class); @@ -51,33 +50,40 @@ public class ApiRateLimitServiceImpl extends AdapterBase implements APILimitChec */ private int maxAllowed = 30; - @Inject - ConfigurationDao _configDao; - - private LimitStore _store; + private LimitStore _store = null; @Override public boolean configure(String name, Map<String, Object> params) throws ConfigurationException { super.configure(name, params); - // get global configured duration and max values - String duration = _configDao.getValue(Config.ApiLimitInterval.key()); - if (duration != null ){ - timeToLive = Integer.parseInt(duration); - } - String maxReqs = _configDao.getValue(Config.ApiLimitMax.key()); - if ( maxReqs != null){ - maxAllowed = Integer.parseInt(maxReqs); + + if (_store == null) { + // not configured yet, note that since this class is both adapter + // and pluggableService, so this method + // may be invoked twice in ComponentLocator. + // get global configured duration and max values + Object duration = params.get("api.throttling.interval"); + if (duration != null) { + timeToLive = Integer.parseInt((String) duration); + } + Object maxReqs = params.get("api.throttling.max"); + if (maxReqs != null) { + maxAllowed = Integer.parseInt((String) maxReqs); + } + // create limit store + EhcacheLimitStore cacheStore = new EhcacheLimitStore(); + int maxElements = 10000; + Object cachesize = params.get("api.throttling.cachesize"); + if ( cachesize != null ){ + maxElements = Integer.parseInt((String)cachesize); + } + CacheManager cm = CacheManager.create(); + Cache cache = new Cache("api-limit-cache", maxElements, false, false, timeToLive, timeToLive); + cm.addCache(cache); + s_logger.info("Limit Cache created: " + cache.toString()); + cacheStore.setCache(cache); + _store = cacheStore; } - // create limit store - EhcacheLimitStore cacheStore = new EhcacheLimitStore(); - int maxElements = 10000; //TODO: what should be the proper number here? - CacheManager cm = CacheManager.create(); - Cache cache = new Cache("api-limit-cache", maxElements, true, false, timeToLive, timeToLive); - cm.addCache(cache); - s_logger.info("Limit Cache created: " + cache.toString()); - cacheStore.setCache(cache); - _store = cacheStore; return true; @@ -123,9 +129,8 @@ public class ApiRateLimitServiceImpl extends AdapterBase implements APILimitChec } - @Override - public boolean isUnderLimit(Account account) { + public void checkLimit(Account account) throws ServerApiException { Long accountId = account.getId(); StoreEntry entry = _store.get(accountId); @@ -140,17 +145,21 @@ public class ApiRateLimitServiceImpl extends AdapterBase implements APILimitChec int current = entry.incrementAndGet(); if (current <= maxAllowed) { - return true; + return; } else { - return false; + long expireAfter = entry.getExpireDuration(); + s_logger.warn("The given user has reached his/her account api limit, please retry after " + expireAfter + " ms."); + throw new ServerApiException(BaseCmd.API_LIMIT_EXCEED, "The given user has reached his/her account api limit, please retry after " + + expireAfter + " ms."); } } @Override - public String[] getPropertiesFiles() { - return new String[] { "api-limit_commands.properties" }; + public Map<String, String> getProperties() { + return PropertiesUtil.processConfigFile(new String[] + { "api-limit_commands.properties" }); } http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/blob/4d0c850d/plugins/api/rate-limit/src/org/apache/cloudstack/ratelimit/StoreEntryImpl.java ---------------------------------------------------------------------- diff --git a/plugins/api/rate-limit/src/org/apache/cloudstack/ratelimit/StoreEntryImpl.java b/plugins/api/rate-limit/src/org/apache/cloudstack/ratelimit/StoreEntryImpl.java index 40965d9..e8143e5 100644 --- a/plugins/api/rate-limit/src/org/apache/cloudstack/ratelimit/StoreEntryImpl.java +++ b/plugins/api/rate-limit/src/org/apache/cloudstack/ratelimit/StoreEntryImpl.java @@ -47,7 +47,7 @@ public class StoreEntryImpl implements StoreEntry { if ( isExpired() ) return 0; // already expired else { - return (expiry - System.currentTimeMillis()) * 1000; + return expiry - System.currentTimeMillis(); } } http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/blob/4d0c850d/plugins/api/rate-limit/test/org/apache/cloudstack/ratelimit/ApiRateLimitTest.java ---------------------------------------------------------------------- diff --git a/plugins/api/rate-limit/test/org/apache/cloudstack/ratelimit/ApiRateLimitTest.java b/plugins/api/rate-limit/test/org/apache/cloudstack/ratelimit/ApiRateLimitTest.java index 0e2080a..ef3cf6d 100644 --- a/plugins/api/rate-limit/test/org/apache/cloudstack/ratelimit/ApiRateLimitTest.java +++ b/plugins/api/rate-limit/test/org/apache/cloudstack/ratelimit/ApiRateLimitTest.java @@ -23,8 +23,9 @@ import java.util.concurrent.Executors; import javax.naming.ConfigurationException; +import org.apache.cloudstack.api.ServerApiException; +import org.apache.cloudstack.api.command.admin.ratelimit.ResetApiLimitCmd; import org.apache.cloudstack.api.command.user.ratelimit.GetApiLimitCmd; -import org.apache.cloudstack.api.commands.admin.ratelimit.ResetApiLimitCmd; import org.apache.cloudstack.api.response.ApiLimitResponse; import org.apache.cloudstack.ratelimit.ApiRateLimitServiceImpl; import org.junit.Before; @@ -43,16 +44,10 @@ import static org.mockito.Mockito.*; public class ApiRateLimitTest { static ApiRateLimitServiceImpl _limitService = new ApiRateLimitServiceImpl(); - static ConfigurationDao _configDao = mock(ConfigurationDao.class); private static long acctIdSeq = 0L; @BeforeClass public static void setUp() throws ConfigurationException { - _limitService._configDao = _configDao; - - // No global configuration set, will set in each test case - when(_configDao.getValue(Config.ApiLimitInterval.key())).thenReturn(null); - when(_configDao.getValue(Config.ApiLimitMax.key())).thenReturn(null); _limitService.configure("ApiRateLimitTest", Collections.<String, Object> emptyMap()); } @@ -62,6 +57,16 @@ public class ApiRateLimitTest { return new AccountVO(acctIdSeq++); } + private boolean isUnderLimit(Account key){ + try{ + _limitService.checkLimit(key); + return true; + } + catch (ServerApiException ex){ + return false; + } + } + @Test public void sequentialApiAccess() { int allowedRequests = 1; @@ -69,10 +74,10 @@ public class ApiRateLimitTest { _limitService.setTimeToLive(1); Account key = createFakeAccount(); - assertTrue("Allow for the first request", _limitService.isUnderLimit(key)); + assertTrue("Allow for the first request", isUnderLimit(key)); assertFalse("Second request should be blocked, since we assume that the two api " - + " accesses take less than a second to perform", _limitService.isUnderLimit(key)); + + " accesses take less than a second to perform", isUnderLimit(key)); } @Test @@ -84,11 +89,11 @@ public class ApiRateLimitTest { Account key = createFakeAccount(); for (int i = 0; i < allowedRequests; i++) { - assertTrue("We should allow " + allowedRequests + " requests per second", _limitService.isUnderLimit(key)); + assertTrue("We should allow " + allowedRequests + " requests per second", isUnderLimit(key)); } - assertFalse("We should block >" + allowedRequests + " requests per second", _limitService.isUnderLimit(key)); + assertFalse("We should block >" + allowedRequests + " requests per second", isUnderLimit(key)); } @Test @@ -121,7 +126,7 @@ public class ApiRateLimitTest { try { startGate.await(); - isUsable[j] = _limitService.isUnderLimit(key); + isUsable[j] = isUnderLimit(key); } catch (InterruptedException e) { e.printStackTrace(); @@ -155,12 +160,12 @@ public class ApiRateLimitTest { Account key = this.createFakeAccount(); - assertTrue("The first request should be allowed", _limitService.isUnderLimit(key)); + assertTrue("The first request should be allowed", isUnderLimit(key)); // Allow the token to expire Thread.sleep(1001); - assertTrue("Another request after interval should be allowed as well", _limitService.isUnderLimit(key)); + assertTrue("Another request after interval should be allowed as well", isUnderLimit(key)); } @Test @@ -171,16 +176,16 @@ public class ApiRateLimitTest { Account key = this.createFakeAccount(); - assertTrue("The first request should be allowed", _limitService.isUnderLimit(key)); + assertTrue("The first request should be allowed", isUnderLimit(key)); - assertFalse("Another request should be blocked", _limitService.isUnderLimit(key)); + assertFalse("Another request should be blocked", isUnderLimit(key)); ResetApiLimitCmd cmd = new ResetApiLimitCmd(); cmd.setAccountId(key.getId()); _limitService.resetApiLimit(cmd); - assertTrue("Another request should be allowed after reset counter", _limitService.isUnderLimit(key)); + assertTrue("Another request should be allowed after reset counter", isUnderLimit(key)); } /* Disable this since I cannot mock Static method UserContext.current() @@ -193,7 +198,7 @@ public class ApiRateLimitTest { Account key = this.createFakeAccount(); for ( int i = 0; i < 5; i++ ){ - assertTrue("Issued 5 requests", _limitService.isUnderLimit(key)); + assertTrue("Issued 5 requests", isUnderLimit(key)); } GetApiLimitCmd cmd = new GetApiLimitCmd(); http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/blob/4d0c850d/server/src/com/cloud/api/ApiServer.java ---------------------------------------------------------------------- diff --git a/server/src/com/cloud/api/ApiServer.java b/server/src/com/cloud/api/ApiServer.java index b2a6a87..1d15acf 100755 --- a/server/src/com/cloud/api/ApiServer.java +++ b/server/src/com/cloud/api/ApiServer.java @@ -556,12 +556,9 @@ public class ApiServer implements HttpRequestHandler { if (userId != null) { User user = ApiDBUtils.findUserById(userId); if (apiThrottlingEnabled){ - // go through each API limit checker - if (!isRequestAllowed(user)) { - //FIXME: more detailed message regarding when he/she can retry - s_logger.warn("The given user has reached his/her account api limit, please retry later"); - throw new ServerApiException(BaseCmd.API_LIMIT_EXCEED, "The given user has reached his/her account api limit"); - } + // go through each API limit checker, throw exception inside adapter implementation so that message + // can contain some detailed information only known for each adapter implementation. + checkRequestLimit(user); } if (!isCommandAvailable(user, commandName)) { s_logger.debug("The given command:" + commandName + " does not exist or it is not available for user with id:" + userId); @@ -803,20 +800,19 @@ public class ApiServer implements HttpRequestHandler { } - private boolean isRequestAllowed(User user) { + private void checkRequestLimit(User user) throws ServerApiException { Account account = ApiDBUtils.findAccountById(user.getAccountId()); if ( _accountMgr.isRootAdmin(account.getType()) ){ // no api throttling for root admin - return true; + return; } for (APILimitChecker apiChecker : _apiLimitCheckers) { // Fail the checking if any checker fails to verify - if (!apiChecker.isUnderLimit(account)) - return false; - } - return true; + apiChecker.checkLimit(account); + } } + private boolean doesCommandExist(String apiName) { for (APIChecker apiChecker : _apiAccessCheckers) { // If any checker has api info on the command, return true http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/blob/4d0c850d/server/src/com/cloud/configuration/Config.java ---------------------------------------------------------------------- diff --git a/server/src/com/cloud/configuration/Config.java b/server/src/com/cloud/configuration/Config.java index ae7651c..e6bf3d5 100755 --- a/server/src/com/cloud/configuration/Config.java +++ b/server/src/com/cloud/configuration/Config.java @@ -361,9 +361,7 @@ public enum Config { null, "Limits number of snapshots that can be handled by the host concurrently; default is NULL - unlimited", null), // API throttling - ApiLimitInterval("Advanced", ManagementServer.class, Long.class, "api.throttling.interval", "1", "The default time interval in seconds used to set account based api limit", null), - ApiLimitMax("Advanced", ManagementServer.class, Long.class, "api.throttling.max", "25", "The max number of API requests within api.throttling.interval duration", null), - ApiLimitEnabled("Advanced", ManagementServer.class, Boolean.class, "api.throttling.enabled", "true", "If true, api throttline feature is enabled", "true,false"); + ApiLimitEnabled("Advanced", ManagementServer.class, Boolean.class, "api.throttling.enable", "true", "If true, api throttline feature is enabled", "true,false"); private final String _category; private final Class<?> _componentClass; http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/blob/4d0c850d/server/test/com/cloud/api/ListPerfTest.java ---------------------------------------------------------------------- diff --git a/server/test/com/cloud/api/ListPerfTest.java b/server/test/com/cloud/api/ListPerfTest.java index eb98d91..e5d277a 100644 --- a/server/test/com/cloud/api/ListPerfTest.java +++ b/server/test/com/cloud/api/ListPerfTest.java @@ -17,6 +17,9 @@ package com.cloud.api; import java.util.HashMap; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; import org.junit.Before; import org.junit.Test; @@ -163,6 +166,57 @@ public class ListPerfTest extends APITest { } + @Test + public void testMultiListAccounts() throws Exception { + // log in using normal user + login("demo", "password"); + // issue list Accounts calls + final HashMap<String, String> params = new HashMap<String, String>(); + params.put("response", "json"); + params.put("listAll", "true"); + params.put("sessionkey", sessionKey); + int clientCount = 6; + Runnable[] clients = new Runnable[clientCount]; + final boolean[] isUsable = new boolean[clientCount]; + + final CountDownLatch startGate = new CountDownLatch(1); + + final CountDownLatch endGate = new CountDownLatch(clientCount); + + + for (int i = 0; i < isUsable.length; ++i) { + final int j = i; + clients[j] = new Runnable() { + /** + * {@inheritDoc} + */ + @Override + public void run() { + try { + startGate.await(); + + System.out.println(sendRequest("listAccounts", params)); + + } catch (InterruptedException e) { + e.printStackTrace(); + } finally { + endGate.countDown(); + } + } + }; + } + + ExecutorService executor = Executors.newFixedThreadPool(clientCount); + + for (Runnable runnable : clients) { + executor.execute(runnable); + } + + startGate.countDown(); + + endGate.await(); + + } } http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/blob/4d0c850d/setup/db/db/schema-40to410.sql ---------------------------------------------------------------------- diff --git a/setup/db/db/schema-40to410.sql b/setup/db/db/schema-40to410.sql index bf3fb30..a6b102a 100644 --- a/setup/db/db/schema-40to410.sql +++ b/setup/db/db/schema-40to410.sql @@ -142,6 +142,8 @@ UPDATE `cloud`.`conditions` set uuid=id WHERE uuid is NULL; INSERT IGNORE INTO `cloud`.`configuration` VALUES ('Advanced', 'DEFAULT', 'management-server', '"detail.batch.query.size"', '2000', 'Default entity detail batch query size for listing'); +INSERT IGNORE INTO `cloud`.`configuration` VALUES ('Advanced', 'DEFAULT', 'management-server', 'api.throttling.enabled', 'true, 'enable api rate limiting'); + --- DB views for list api --- use cloud;
