Skip to content

Commit

Permalink
Remove resourceQuotaPerResourceDesc and memoryQuotaPerResourceDesc fr…
Browse files Browse the repository at this point in the history 
…om GroupResourcePlacementService.

- Both fields are used only internally
- currently we clone description for every provisioning, so the value is always 1 in the map.
- during PUT only resourceQuotaPerResourceDesc is preserved, so memoryQuotaPerResourceDesc is
random.

Change-Id: Iccfb55da297f1a7809c94f88dcf3b90deb6776f9
Reviewed-on: http://bellevue-ci.eng.vmware.com:8080/8933
Upgrade-Verified: jenkins <[email protected]>
Closures-Verified: jenkins <[email protected]>
Compute-Verified: jenkins <[email protected]>
CS-Verified: jenkins <[email protected]>
Bellevue-Verified: jenkins <[email protected]>
Reviewed-by: Rostislav Georgiev <[email protected]>
  • Loading branch information
Rostislav Georgiev committed Apr 13, 2017
1 parent bffdf64 commit 1d5069d
Show file tree
Hide file tree
Showing 7 changed files with 54 additions and 175 deletions.
92 changes: 22 additions & 70 deletions ...ute/src/main/java/com/vmware/admiral/compute/container/GroupResourcePlacementService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import com.vmware.admiral.compute.container.ContainerService.ContainerState;
import com.vmware.admiral.service.common.MultiTenantDocument;
import com.vmware.photon.controller.model.resources.ComputeDescriptionService.ComputeDescription;
import com.vmware.photon.controller.model.resources.ComputeDescriptionService.ComputeDescription.ComputeType;
import com.vmware.photon.controller.model.resources.ComputeService.ComputeState;
import com.vmware.photon.controller.model.resources.ResourcePoolService;
import com.vmware.photon.controller.model.resources.ResourcePoolService.ResourcePoolState;
Expand Down Expand Up @@ -122,8 +123,9 @@ public static class GroupResourcePlacementState extends MultiTenantDocument {
public String resourcePoolLink;

/** The priority with which the group resource placements will be applied */
@Documentation(description = "The priority with which the group resource placements will be applied." +
" Lower number means higher priority.")
@Documentation(description = "The priority with which the group resource placements will be applied."
+
" Lower number means higher priority.")
public int priority;

@Documentation(description = "The resource type for which the group resource quotas will be applied.")
Expand Down Expand Up @@ -187,16 +189,19 @@ public static class GroupResourcePlacementState extends MultiTenantDocument {
public long availableMemory;

/** Set by Task. The number of used instances linked to their Resource descriptions. */
@Documentation(description = "The number of used instances linked to their Resource descriptions.")
@Documentation(description = "Deprecated, not in use.The number of used instances linked to their Resource descriptions.")
@UsageOption(option = PropertyUsageOption.SERVICE_USE)
@PropertyOptions(indexing = { PropertyIndexingOption.STORE_ONLY })
@Deprecated
public Map<String, Long> resourceQuotaPerResourceDesc;

/** Set by Task. Memory quota per resource desc. */
@Documentation(description = "Memory quota per resource desc.")
@Documentation(description = "Deprecated, not in use.Memory quota per resource desc.")
@UsageOption(option = PropertyUsageOption.SERVICE_USE)
@PropertyOptions(indexing = { PropertyIndexingOption.STORE_ONLY })
@Deprecated
public Map<String, Long> memoryQuotaPerResourceDesc;

}

/**
Expand Down Expand Up @@ -231,8 +236,6 @@ public static GroupResourcePlacementPoolState create(
poolState.maxNumberInstances = groupResourcePlacementState.maxNumberInstances;
poolState.availableInstancesCount = groupResourcePlacementState.availableInstancesCount;
poolState.allocatedInstancesCount = groupResourcePlacementState.allocatedInstancesCount;
poolState.resourceQuotaPerResourceDesc =
groupResourcePlacementState.resourceQuotaPerResourceDesc;
poolState.customProperties = groupResourcePlacementState.customProperties;
poolState.cpuShares = groupResourcePlacementState.cpuShares;
poolState.memoryLimit = groupResourcePlacementState.memoryLimit;
Expand Down Expand Up @@ -291,6 +294,8 @@ public void handleCreate(Operation start) {
logFine("Initial name is %s", state.name);

validateStateOnStart(state, start, (o) -> {
state.availableInstancesCount = state.maxNumberInstances;
state.allocatedInstancesCount = 0;
start.complete();
});
}
Expand All @@ -306,9 +311,6 @@ public void handlePut(Operation put) {
GroupResourcePlacementState currentState = getState(put);
GroupResourcePlacementState putBody = put.getBody(GroupResourcePlacementState.class);

// make sure that the active placements are not overridden before validation
putBody.resourceQuotaPerResourceDesc = currentState.resourceQuotaPerResourceDesc;

validateStateOnStart(putBody, put, (a) -> {
// make sure the current placements are not overridden
currentState.name = putBody.name;
Expand Down Expand Up @@ -359,8 +361,6 @@ public void handlePut(Operation put) {
currentState.memoryLimit = putBody.memoryLimit;
currentState.availableMemory = currentState.memoryLimit - reservedMemory;

currentState.tenantLinks = putBody.tenantLinks;

setState(put, currentState);
put.setBody(currentState).complete();
});
Expand Down Expand Up @@ -413,36 +413,6 @@ public void handlePatch(Operation patch) {
return;
}

if (state.resourceQuotaPerResourceDesc == null) {
state.resourceQuotaPerResourceDesc = new HashMap<>();
}

Long countPerDesc = state.resourceQuotaPerResourceDesc
.get(request.resourceDescriptionLink);
if (countPerDesc == null) {
if (request.resourceCount < 0) {
patch.fail(new LocalizableValidationException(
"Releasing placement do not exist for requested resourceDescriptionLink: "
+ request.resourceDescriptionLink,
"compute.placements.resource.not.exists",
request.resourceDescriptionLink));
return;
}
state.resourceQuotaPerResourceDesc.put(request.resourceDescriptionLink,
request.resourceCount);
} else {
long currentCountPerDesc = countPerDesc + request.resourceCount;
if (currentCountPerDesc < 0) {
patch.fail(new LocalizableValidationException(
"Releasing placement is more than previously requested for the "
+ "resourceDescriptionLink: " + request.resourceDescriptionLink,
"compute.placements.release.too.much", request.resourceDescriptionLink));
return;
}
state.resourceQuotaPerResourceDesc.put(request.resourceDescriptionLink,
currentCountPerDesc);
}

state.availableInstancesCount = currentCount;
state.allocatedInstancesCount += request.resourceCount;

Expand Down Expand Up @@ -513,19 +483,6 @@ private boolean reserveMemory(Operation patch,
state.availableMemory = currentMemory;
}

if (state.memoryQuotaPerResourceDesc == null) {
state.memoryQuotaPerResourceDesc = new HashMap<>();
}

Long allMemory = state.memoryQuotaPerResourceDesc.get(request.resourceDescriptionLink);

if (allMemory == null) {
state.memoryQuotaPerResourceDesc.put(request.resourceDescriptionLink, requestedMemory);
} else {
state.memoryQuotaPerResourceDesc
.put(request.resourceDescriptionLink, requestedMemory + allMemory);
}

return true;
}

Expand All @@ -545,7 +502,7 @@ public void handleDelete(Operation delete) {

if (state.allocatedInstancesCount != count) {
logWarning("Reservation mismatch detected for placement %s!! "
+ "allocatedInstancesCount=%d actual=%d",
+ "allocatedInstancesCount=%d actual=%d",
state.documentSelfLink, state.allocatedInstancesCount, count);
}

Expand Down Expand Up @@ -617,13 +574,7 @@ private void validateStateOnStart(GroupResourcePlacementState state, Operation o

validatePlacementSize(state, operation, (o) -> {

if (state.resourceQuotaPerResourceDesc == null
|| state.resourceQuotaPerResourceDesc.isEmpty()) {
state.availableInstancesCount = state.maxNumberInstances;
state.allocatedInstancesCount = 0;
state.availableMemory = state.memoryLimit;
}

state.availableMemory = state.memoryLimit;
callbackFunction.accept(null);
});
}
Expand All @@ -650,7 +601,7 @@ private void validatePlacementSize(GroupResourcePlacementState state, Operation
"Not enough memory in this placement zone. "
+ "Total memory in placement zone: %s, "
+ "requested: %s",
totalMemory, state.memoryLimit);
totalMemory, state.memoryLimit);
operation.fail(new LocalizableValidationException(errorMesg,
"compute.placements.not.enough.memory.in.zone",
totalMemory, state.memoryLimit));
Expand Down Expand Up @@ -691,7 +642,7 @@ private void getOtherPlacementsInResourcePoolAndValidate(GroupResourcePlacementS
long availableMemory = totalMemory - allPlacementMemory;
if (availableMemory > 0 && availableMemory < state.memoryLimit) {
String errorMsg = String.format("Memory already reserved by other placements. "
+ "Available memory: %s, requested: %s",
+ "Available memory: %s, requested: %s",
availableMemory, state.memoryLimit);
operation.fail(new LocalizableValidationException(errorMsg,
"compute.placement.memory.unavailable", availableMemory,
Expand All @@ -709,13 +660,13 @@ public ServiceDocument getDocumentTemplate() {
GroupResourcePlacementState template =
(GroupResourcePlacementState) super.getDocumentTemplate();
com.vmware.photon.controller.model.ServiceUtils.setRetentionLimit(template);
template.resourceQuotaPerResourceDesc = new HashMap<>();
template.memoryQuotaPerResourceDesc = new HashMap<>();
template.customProperties = new HashMap<String, String>(1);
template.customProperties.put("propKey string", "customPropertyValue string");
// Having multiple resource descriptions hit the default limit. 1MB should be enough for
// ~13 760 containers
template.documentDescription.serializedStateSizeLimit = 1024 * 1024; // 1MB
template.resourceQuotaPerResourceDesc = new HashMap<>();
template.memoryQuotaPerResourceDesc = new HashMap<>();

return template;
}
Expand All @@ -735,9 +686,9 @@ private boolean isReservationServiceTaskAuthorizedRequest(Operation patch) {
}

@SuppressWarnings("unchecked")
private <T extends ServiceDocument> void countResourcesForPlacement(GroupResourcePlacementState state,
Consumer<ServiceDocumentQueryElementResult<T>>
completionHandler) {
private <T extends ServiceDocument> void countResourcesForPlacement(
GroupResourcePlacementState state,
Consumer<ServiceDocumentQueryElementResult<T>> completionHandler) {
QueryTask queryTask;
Class<T> resourceClass;

Expand All @@ -750,7 +701,8 @@ private <T extends ServiceDocument> void countResourcesForPlacement(GroupResourc
resourceClass = (Class<T>) ComputeState.class;
queryTask = QueryUtil.buildPropertyQuery(resourceClass,
ComputeState.FIELD_NAME_RESOURCE_POOL_LINK,
state.resourcePoolLink);
state.resourcePoolLink, ComputeState.FIELD_NAME_TYPE,
ComputeType.VM_GUEST.name());
} else {
throw new LocalizableValidationException("Unsupported placement resourceType "
+ state.resourceType,
Expand Down
Loading

0 comments on commit 1d5069d

Please sign in to comment.