Gilad Chaplik has posted comments on this change.
Change subject: core: Quota refactor - QuotaManager
......................................................................
Patch Set 15: (13 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CommandBase.java
Line 640: QuotaConsumptionParametersWrapper
quotaConsumptionParametersWrapper = new QuotaConsumptionParametersWrapper(this,
Line 641: getReturnValue().getCanDoActionMessages());
Line 642:
quotaConsumptionParametersWrapper.setParameters(getQuotaConsumptionParameters());
Line 643:
Line 644: return
quotaConsumptionParametersWrapper.getParameters().isEmpty() ||
getQuotaManager().consume(quotaConsumptionParametersWrapper);
why 'quotaConsumptionParametersWrapper.getParameters().isEmpty()' is a valid
flow?
Line 645: }
Line 646:
Line 647: /**
Line 648: * @return true if all parameters class and its inner members
passed
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/quota/QuotaManager.java
Line 332: }
Line 333:
Line 334: private boolean
validateAndSetStorageQuotaHelper(QuotaConsumptionParametersWrapper parameters) {
Line 335: lock.readLock().lock();
Line 336: try {
check that storagepoolid isn't null, before using synchronized on it
Line 337: synchronized
(storagePoolQuotaMap.get(parameters.getAuditLogable().getStoragePool().getId()))
{
Line 338: if
(storagePoolQuotaMap.get(parameters.getAuditLogable().getStoragePool().getId())
== null) {
Line 339: return false;
Line 340: }
Line 333:
Line 334: private boolean
validateAndSetStorageQuotaHelper(QuotaConsumptionParametersWrapper parameters) {
Line 335: lock.readLock().lock();
Line 336: try {
Line 337: synchronized
(storagePoolQuotaMap.get(parameters.getAuditLogable().getStoragePool().getId()))
{
have a method for getStoragePoolId() in wrapper
Line 338: if
(storagePoolQuotaMap.get(parameters.getAuditLogable().getStoragePool().getId())
== null) {
Line 339: return false;
Line 340: }
Line 341: Map<Guid, Quota> quotaMap =
storagePoolQuotaMap.get(parameters.getAuditLogable().getStoragePool().getId());
Line 334: private boolean
validateAndSetStorageQuotaHelper(QuotaConsumptionParametersWrapper parameters) {
Line 335: lock.readLock().lock();
Line 336: try {
Line 337: synchronized
(storagePoolQuotaMap.get(parameters.getAuditLogable().getStoragePool().getId()))
{
Line 338: if
(storagePoolQuotaMap.get(parameters.getAuditLogable().getStoragePool().getId())
== null) {
and use that one instead of
'parameters.getAuditLogable().getStoragePool().getId()'
Line 339: return false;
Line 340: }
Line 341: Map<Guid, Quota> quotaMap =
storagePoolQuotaMap.get(parameters.getAuditLogable().getStoragePool().getId());
Line 342: Map<Guid, Map<Guid, Double>>
desiredStorageSizeQuotaMap = new HashMap<Guid, Map<Guid, Double>>();
Line 337: synchronized
(storagePoolQuotaMap.get(parameters.getAuditLogable().getStoragePool().getId()))
{
Line 338: if
(storagePoolQuotaMap.get(parameters.getAuditLogable().getStoragePool().getId())
== null) {
Line 339: return false;
Line 340: }
Line 341: Map<Guid, Quota> quotaMap =
storagePoolQuotaMap.get(parameters.getAuditLogable().getStoragePool().getId());
same here.
Line 342: Map<Guid, Map<Guid, Double>>
desiredStorageSizeQuotaMap = new HashMap<Guid, Map<Guid, Double>>();
Line 343:
Line 344: Map<Guid, Double> newUsedGlobalStorageSize = new
HashMap<Guid, Double>();
Line 345: Map<Guid, Map<Guid, Double>>
newUsedSpecificStorageSize = new HashMap<Guid, Map<Guid, Double>>();
Line 408: }
Line 409: if (!hasStorageId) {
Line 410: parameters.getCanDoActionMessages()
Line 411:
.add(VdcBllMessages.ACTION_TYPE_FAILED_NO_QUOTA_SET_FOR_DOMAIN.toString());
Line 412: return true;
I return true when it's bad? confusing
Line 413: }
Line 414: }
Line 415: return false;
Line 416: }
Line 436: quota.getGlobalQuotaStorage().getStorageSizeGB(),
Line 437: storageUsagePercentage, storageRequestPercentage,
Line 438: parameters.getCanDoActionMessages(),
Line 439: parameters.getAuditLogable())) {
Line 440: return true;
same here. true is when the flow is correct
Line 441: }
Line 442: newUsedGlobalStorageSize.put(quotaId, sum
Line 443: +
quota.getGlobalQuotaStorage().getStorageSizeGBUsage());
Line 444: }
Line 582: storageUsagePercentage + storageRequestPercentage,
Line 583: storageRequestPercentage);
Line 584: } else {
Line 585: quotaManagerAuditLogger.auditLogStorage(auditLogableBase,
Line 586:
AuditLogType.USER_EXCEEDED_QUOTA_STORAGE_GRACE_LIMIT,
quotaManagerAuditLogger.auditLogStorage(...)
this will cause a write to DB? inside of a lock?
Line 587: quota.getQuotaName(),
Line 588: storageUsagePercentage,
Line 589: storageRequestPercentage);
Line 590: if
(QuotaEnforcementTypeEnum.HARD_ENFORCEMENT.equals(quotaEnforcementTypeEnum)) {
Line 826:
Line 827: public boolean
validateAndSetClusterQuota(QuotaConsumptionParametersWrapper parameters) {
Line 828: List<QuotaVdsGroupConsumptionParameter>
vdsGroupConsumptionParameters =
Line 829:
filterQuotaVdsGroupConsumptionParameter(parameters.getParameters());
Line 830: synchronized
(storagePoolQuotaMap.get(parameters.getAuditLogable().getStoragePool().getId()))
{
same here..
Line 831: for (QuotaVdsGroupConsumptionParameter parameter :
vdsGroupConsumptionParameters) {
Line 832: Quota quota = parameter.getQuota();
Line 833: QuotaVdsGroup quotaVdsGroup = null;
Line 834:
Line 1038: * @return - true if the request was validated and set
Line 1039: */
Line 1040: public boolean consume(QuotaConsumptionParametersWrapper
parameters) {
Line 1041: if (!validateAndCompleteParameters(parameters)) {
Line 1042: throw new InvalidQuotaParametersException();
validateParameters() is enough
Line 1043: }
Line 1044:
Line 1045: return internalConsumeAndReleaseHandler(parameters);
Line 1046: }
Line 1127:
Line 1128: // for each parameter - check and complete
Line 1129: for (QuotaConsumptionParameter param :
parameters.getParameters()) {
Line 1130: // check that quota id is valid and fetch the quota from
db (or cache). add the quota to the param
Line 1131: boolean validQuotaId = checkAndFetchQuota(parameters,
param);
what is the need for cache if we fetch the quota all the time?
Line 1132:
Line 1133: // In case this param is a QuotaVdsConsumptionParameter
- check that it has a valid
Line 1134: // vds group id which is handled by this quota
Line 1135: boolean validVdsGroup = true;
Line 1181: return true;
Line 1182: }
Line 1183:
Line 1184: // In case this param is a QuotaVdsConsumptionParameter - check
that it has a valid
Line 1185: // vds group id which is handled by this quota
please use a single terminology: Cluster, VdsGroup or VDS :)
Line 1186: private boolean
checkVdsGroupMatchQuota(QuotaConsumptionParametersWrapper parameters,
QuotaConsumptionParameter param) {
Line 1187: Quota quota = param.getQuota();
Line 1188: QuotaVdsGroupConsumptionParameter paramVds =
(QuotaVdsGroupConsumptionParameter) param;
Line 1189:
Line 1265: lock.readLock().lock();
Line 1266: try {
Line 1267: directQuotaMap.put(quotaId, quota);
Line 1268: // cache in storage-pool->quota map
Line 1269:
storagePoolQuotaMap.putIfAbsent(quota.getStoragePoolId(), new HashMap<Guid,
Quota>());
fetchQuotaFromDB should acquire locks and update cache?
Line 1270:
storagePoolQuotaMap.get(quota.getStoragePoolId()).put(quotaId, quota);
Line 1271: } finally {
Line 1272: lock.readLock().unlock();
Line 1273: }
--
To view, visit http://gerrit.ovirt.org/8776
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibb100467a55b26e4219d1a2562da86b81ffdc032
Gerrit-PatchSet: 15
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: ofri masad <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Doron Fediuck <[email protected]>
Gerrit-Reviewer: Gilad Chaplik <[email protected]>
Gerrit-Reviewer: Laszlo Hornyak <[email protected]>
Gerrit-Reviewer: Michael Kublin <[email protected]>
Gerrit-Reviewer: Sharad Mishra <[email protected]>
Gerrit-Reviewer: Yair Zaslavsky <[email protected]>
Gerrit-Reviewer: ofri masad <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
