Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LocalAppDeployer scaling operations causes too many created empty, unnecessary & unused dirs #233

Open
Hassen-BENNOUR opened this issue Mar 24, 2023 · 0 comments
Labels
status/need-triage Team needs to triage and take a first look

Comments

@Hassen-BENNOUR
Copy link

Hassen-BENNOUR commented Mar 24, 2023

On call to Scale API, every call (in my case with spring-cloud-skipper through Spring Cloud Data Flow & alertmanager) causes a new directories created under the workingDirectoriesRoot

The problem is below, the createWorkingDir is called before checking if deltaCount == 0

So even if there is no scaling needed the directories are created.
With a huge amount of alerts this causes too many empty directories which saturates the files system
Even if i use the tmp dir, the time that the cleaning is done, it creates too many directories

LocalAppDeployer

`

@Override
public void scale(AppScaleRequest appScaleRequest) {
	validateStatus(appScaleRequest.getDeploymentId(), DeploymentState.deployed);
	AppInstancesHolder holder = running.get(appScaleRequest.getDeploymentId());
	List<AppInstance> instances = holder != null ? holder.instances : null;
	if (instances == null) {
		throw new IllegalStateException(
				"Can't find existing instances for deploymentId " + appScaleRequest.getDeploymentId());
	}
	AppDeploymentRequest request = holder.request;

	String group = request.getDeploymentProperties().get(GROUP_PROPERTY_KEY);
	String deploymentId = String.format("%s.%s", group, request.getDefinition().getName());

	try {
		//here is the problem
		Path workDir = createWorkingDir(request.getDeploymentProperties(), deploymentId);
		int deltaCount = appScaleRequest.getCount() - instances.size();
		int targetCount = instances.size() + deltaCount;

		//here we check if scaling is necessary, workDir is created
		if (deltaCount > 0) {
			for (int index = instances.size(); index < targetCount; index++) {
				instances.add(deployApp(request, workDir, group, deploymentId, index, request.getDeploymentProperties()));
			}
		}
		else if (deltaCount < 0) {
		...

`

I use 2.7.4 but this is the case now

Used with Autoscaling with Prometheus, Alertmanager and SCDF Scale API

I propose

`

public void scale(AppScaleRequest appScaleRequest) {
    this.validateStatus(appScaleRequest.getDeploymentId(), DeploymentState.deployed);
    LocalAppDeployer.AppInstancesHolder holder = (LocalAppDeployer.AppInstancesHolder)this.running.get(appScaleRequest.getDeploymentId());
    List<LocalAppDeployer.AppInstance> instances = holder != null ? holder.instances : null;
    if (instances == null) {
        throw new IllegalStateException("Can't find existing instances for deploymentId " + appScaleRequest.getDeploymentId());
    } else {
        int deltaCount = appScaleRequest.getCount() - instances.size();
        if (deltaCount == 0) {
            return;
        }
        AppDeploymentRequest request = holder.request;
        String group = (String)request.getDeploymentProperties().get("spring.cloud.deployer.group");
        String deploymentId = String.format("%s.%s", group, request.getDefinition().getName());

        try {
            Path workDir = this.createWorkingDir(request.getDeploymentProperties(), deploymentId);
            int targetCount = instances.size() + deltaCount;
            if (deltaCount > 0) {
                for(int index = instances.size(); index < targetCount; ++index) {
                    instances.add(this.deployApp(request, workDir, group, deploymentId, index, request.getDeploymentProperties()));
                }
            } else if (deltaCount < 0) {
                List<LocalAppDeployer.AppInstance> processes = new ArrayList();

                for(int index = instances.size() - 1; index >= targetCount; --index) {
                    processes.add(instances.remove(index));
                }

                Iterator var15 = processes.iterator();

                while(var15.hasNext()) {
                    LocalAppDeployer.AppInstance instance = (LocalAppDeployer.AppInstance)var15.next();
                    if (this.isAlive(instance.getProcess())) {
                        logger.info("Un-deploying app with deploymentId {} instance {}.", deploymentId, instance.getInstanceNumber());
                        this.shutdownAndWait(instance);
                    }
                }
            }

        } catch (IOException var13) {
            throw new RuntimeException("Exception trying to deploy " + request, var13);
        }
    }
}
`
@github-actions github-actions bot added the status/need-triage Team needs to triage and take a first look label Mar 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status/need-triage Team needs to triage and take a first look
Projects
None yet
Development

No branches or pull requests

1 participant