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

Add cleaner for orphaned files #1406

Merged
merged 1 commit into from
Nov 29, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ public final class Messages {
public static final String FILE_WITH_ID_AND_SPACE_DOES_NOT_EXIST = "File with ID \"{0}\" and space \"{1}\" does not exist.";
public static final String ERROR_DELETING_FILE_WITH_ID = "Error deleting file with ID \"{0}\"";
public static final String ERROR_GETTING_FILES_WITH_SPACE_AND_NAMESPACE = "Error getting files with space {0} and namespace {1}";
public static final String ERROR_GETTING_FILES_CREATED_AFTER_0 = "Error getting files created after {0}";
public static final String ERROR_GETTING_FILES_WITH_SPACE_NAMESPACE_AND_NAME = "Error getting files with space {0} namespace {1} and file name {2}";
public static final String ERROR_GETTING_ALL_FILES = "Error getting all files";
public static final String ERROR_DELETING_PROCESS_LOGS_WITH_NAMESPACE = "Error deleting process logs with namespace \"{0}\"";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ public interface OperationQuery extends Query<Operation, OperationQuery> {

OperationQuery startedBefore(LocalDateTime startedBefore);

OperationQuery startedAfter(LocalDateTime timestamp);

OperationQuery endedBefore(LocalDateTime endedBefore);

OperationQuery endedAfter(LocalDateTime endedAfter);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,16 @@ public OperationQuery startedBefore(LocalDateTime startedBefore) {
return this;
}

@Override
public OperationQuery startedAfter(LocalDateTime timestamp) {
queryCriteria.addRestriction(ImmutableQueryAttributeRestriction.<LocalDateTime> builder()
.attribute(AttributeNames.STARTED_AT)
.condition(getCriteriaBuilder()::greaterThanOrEqualTo)
.value(timestamp)
.build());
return this;
}

@Override
public OperationQuery endedBefore(LocalDateTime endedBefore) {
queryCriteria.addRestriction(ImmutableQueryAttributeRestriction.<LocalDateTime> builder()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ public abstract class SqlFileQueryProvider {
private static final String UPDATE_FILE_DIGEST = "UPDATE %s SET DIGEST = ? WHERE FILE_ID = ?";
private static final String INSERT_FILE_ATTRIBUTES = "INSERT INTO %s (FILE_ID, SPACE, FILE_NAME, NAMESPACE, FILE_SIZE, DIGEST, DIGEST_ALGORITHM, MODIFIED) VALUES (?, ?, ?, ?, ?, ?, ?, ?)";
private static final String SELECT_ALL_FILES = "SELECT FILE_ID, SPACE, DIGEST, DIGEST_ALGORITHM, MODIFIED, FILE_NAME, NAMESPACE, FILE_SIZE FROM %s";
private static final String SELECT_FILES_CREATED_AFTER = "SELECT FILE_ID, SPACE, DIGEST, DIGEST_ALGORITHM, MODIFIED, FILE_NAME, NAMESPACE, FILE_SIZE FROM %s WHERE MODIFIED > ?";
private static final String SELECT_FILES_BY_NAMESPACE_AND_SPACE = "SELECT FILE_ID, SPACE, DIGEST, DIGEST_ALGORITHM, MODIFIED, FILE_NAME, NAMESPACE, FILE_SIZE FROM %s WHERE NAMESPACE=? AND SPACE=?";
private static final String SELECT_FILES_BY_NAMESPACE_SPACE_AND_NAME = "SELECT FILE_ID, SPACE, DIGEST, DIGEST_ALGORITHM, MODIFIED, FILE_NAME, NAMESPACE, FILE_SIZE FROM %s WHERE NAMESPACE=? AND SPACE=? AND FILE_NAME=? ORDER BY MODIFIED ASC";
private static final String SELECT_FILES_BY_SPACE_WITH_NO_NAMESPACE = "SELECT FILE_ID, SPACE, DIGEST, DIGEST_ALGORITHM, MODIFIED, FILE_NAME, NAMESPACE, FILE_SIZE FROM %s WHERE SPACE=? AND NAMESPACE IS NULL";
Expand Down Expand Up @@ -197,6 +198,27 @@ public SqlQuery<List<FileEntry>> getListAllFilesQuery() {
};
}

public SqlQuery<List<FileEntry>> getListFilesCreatedAfterQuery(LocalDateTime timestamp) {
return (Connection connection) -> {
PreparedStatement statement = null;
ResultSet resultSet = null;
try {
List<FileEntry> files = new ArrayList<>();
statement = connection.prepareStatement(getQuery(SELECT_FILES_CREATED_AFTER));
statement.setTimestamp(1, Timestamp.from(timestamp.atZone(ZoneId.systemDefault())
.toInstant()));
resultSet = statement.executeQuery();
while (resultSet.next()) {
files.add(getFileEntry(resultSet));
}
return files;
} finally {
JdbcUtil.closeQuietly(resultSet);
JdbcUtil.closeQuietly(statement);
}
};
}

public SqlQuery<FileEntry> getRetrieveFileQuery(String space, String id) {
return (Connection connection) -> {
PreparedStatement statement = null;
Expand Down Expand Up @@ -290,9 +312,8 @@ public SqlQuery<Integer> getDeleteModifiedBeforeQuery(LocalDateTime modification
PreparedStatement statement = null;
try {
statement = connection.prepareStatement(getQuery(DELETE_FILES_MODIFIED_BEFORE));
statement.setTimestamp(1, new Timestamp(modificationTime.atZone(ZoneId.systemDefault())
.toInstant()
.toEpochMilli()));
statement.setTimestamp(1, Timestamp.from(modificationTime.atZone(ZoneId.systemDefault())
.toInstant()));
int deletedFiles = statement.executeUpdate();
logger.debug(MessageFormat.format(Messages.DELETED_0_FILES_MODIFIED_BEFORE_1, deletedFiles, modificationTime));
return deletedFiles;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,15 @@ public List<FileEntry> listFiles(String space, String namespace) throws FileStor
}
}

public List<FileEntry> listFilesCreatedAfter(LocalDateTime timestamp) throws FileStorageException {
try {
return getSqlQueryExecutor().execute(getSqlFileQueryProvider().getListFilesCreatedAfterQuery(timestamp));
} catch (SQLException e) {
throw new FileStorageException(MessageFormat.format(Messages.ERROR_GETTING_FILES_CREATED_AFTER_0, timestamp),
e);
}
}

public FileEntry getFile(String space, String id) throws FileStorageException {
try {
return getSqlQueryExecutor().execute(getSqlFileQueryProvider().getRetrieveFileQuery(space, id));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ public class Messages {
public static final String VERSION_RULE_DOES_NOT_ALLOW_DEPLOYMENT_TYPE = "Version rule \"{0}\" does not allow deployment type \"{1}\".";
public static final String UNKNOWN_UPLOAD_STATUS = "Unknown upload status: {0}";
public static final String COULD_NOT_DELETE_FILES_MODIFIED_BEFORE_0 = "Could not delete files modified before {0}";
public static final String COULD_NOT_DELETE_ORPHANED_FILES_MODIFIED_AFTER_0 = "Could not delete orphaned files modified after {0}";
public static final String COULD_NOT_DELETE_PROCESS_LOGS_MODIFIED_BEFORE_0 = "Could not delete process logs modified before {0}";
public static final String MODULES_0_SPECIFIED_FOR_DEPLOYMENT_ARE_NOT_PART_OF_DEPLOYMENT_DESCRIPTOR_MODULES = "Modules {0}, specified for deployment, are not part of deployment descriptor modules";
public static final String SERVICE_BROKER_0_DOES_NOT_EXIST = "Service broker \"{0}\" does not exist";
Expand Down Expand Up @@ -666,6 +667,11 @@ public class Messages {
public static final String PROCESS_WAS_DELETED_0 = "Process was deleted: {0}";
public static final String MODULE_0_WAS_NOT_FOUND = "Module \"{0}\" was not found";
public static final String DETECTING_LIVE_APPLICATION_ENV = "Detecting live application env...";
public static final String GETTING_FILES_CREATED_AFTER_0 = "Getting files created after {0} to check for deletion";
public static final String GETTING_OPERATIONS_STARTED_AFTER_0 = "Getting operations started after {0} to check for orphaned files";
public static final String GETTING_HISTORIC_VARIABLES_FOR_OPERATIONS_STARTED_AFTER_0 = "Getting historic variables for operations started after {0}";
public static final String NO_ORPHANED_FILES_TO_DELETE = "No orphaned files to delete.";
public static final String DELETING_ORPHANED_FILES_0 = "Deleting {0} orphaned files: {1}";

protected Messages() {
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
package org.cloudfoundry.multiapps.controller.process.jobs;

import org.cloudfoundry.multiapps.common.SLException;
import org.cloudfoundry.multiapps.controller.core.util.ApplicationConfiguration;
import org.cloudfoundry.multiapps.controller.persistence.model.FileEntry;
import org.cloudfoundry.multiapps.controller.persistence.services.FileService;
import org.cloudfoundry.multiapps.controller.persistence.services.FileStorageException;
import org.cloudfoundry.multiapps.controller.persistence.services.OperationService;
import org.cloudfoundry.multiapps.controller.process.Messages;
import org.cloudfoundry.multiapps.controller.process.flowable.FlowableFacade;
import org.cloudfoundry.multiapps.controller.process.variables.Variables;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.scheduling.annotation.Scheduled;

import javax.inject.Inject;
import javax.inject.Named;
import java.text.MessageFormat;
import java.time.LocalDateTime;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.concurrent.TimeUnit;
import java.util.stream.Collectors;

@Named
public class OrphanedFilesCleaner {
IvanBorislavovDimitrov marked this conversation as resolved.
Show resolved Hide resolved

private static final Logger LOGGER = LoggerFactory.getLogger(OrphanedFilesCleaner.class);
private static final int SELECTED_INSTANCE_FOR_CLEANUP = 1;

private final FileService fileService;
private final OperationService operationService;
private final FlowableFacade flowableFacade;
private final ApplicationConfiguration configuration;

@Inject
public OrphanedFilesCleaner(FileService fileService, OperationService operationService,
FlowableFacade flowableFacade, ApplicationConfiguration config) {
this.fileService = fileService;
this.operationService = operationService;
this.flowableFacade = flowableFacade;
this.configuration = config;
}

//this is quite inefficient because the Operation table does not contain any info on which archive
//it is started with
@Scheduled(fixedRate = 30, timeUnit = TimeUnit.MINUTES)
IvanBorislavovDimitrov marked this conversation as resolved.
Show resolved Hide resolved
public void run() {
IvanBorislavovDimitrov marked this conversation as resolved.
Show resolved Hide resolved
if (configuration.getApplicationInstanceIndex() != SELECTED_INSTANCE_FOR_CLEANUP) {
return;
}

var timestamp = LocalDateTime.now();
var oneHourAgo = timestamp.minusHours(1);
try {
LOGGER.debug(MessageFormat.format(Messages.GETTING_FILES_CREATED_AFTER_0, oneHourAgo));
var files = fileService.listFilesCreatedAfter(oneHourAgo);
var fileIdsToFiles = files.stream()
.collect(Collectors.toMap(FileEntry::getId, entry -> entry));

var historicVariables = getHistoricAppArchiveIDs(oneHourAgo);

filterFilesWithStartedOperations(fileIdsToFiles, historicVariables);

if (fileIdsToFiles.isEmpty()) {
LOGGER.info(Messages.NO_ORPHANED_FILES_TO_DELETE);
return;
}
LOGGER.debug(MessageFormat.format(Messages.DELETING_ORPHANED_FILES_0, fileIdsToFiles.size(), fileIdsToFiles.keySet()));
for (var orphanedFile : fileIdsToFiles.values()) {
fileService.deleteFile(orphanedFile.getSpace(), orphanedFile.getId());
}
} catch (FileStorageException e) {
throw new SLException(e, Messages.COULD_NOT_DELETE_ORPHANED_FILES_MODIFIED_AFTER_0, oneHourAgo);
}
}

private List<String> getHistoricAppArchiveIDs(LocalDateTime startedAfter) {
LOGGER.debug(MessageFormat.format(Messages.GETTING_OPERATIONS_STARTED_AFTER_0, startedAfter));
var operations = operationService.createQuery()
.startedAfter(startedAfter)
.list();

LOGGER.debug(MessageFormat.format(Messages.GETTING_HISTORIC_VARIABLES_FOR_OPERATIONS_STARTED_AFTER_0, startedAfter));
List<String> result = new LinkedList<>();
for (var operation : operations) {
var historicVariable = flowableFacade.getHistoricVariableInstance(operation.getProcessId(),
Variables.APP_ARCHIVE_ID.getName());
result.add(String.valueOf(historicVariable.getValue()));
}
return result;
}

private void filterFilesWithStartedOperations(Map<String, FileEntry> files, List<String> historicVars) {
var fileIDs = files.keySet();
for (var appArchiveId : historicVars) {
if (fileIDs.contains(appArchiveId)) {
files.remove(appArchiveId);
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
package org.cloudfoundry.multiapps.controller.process.jobs;

import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import static org.mockito.Mockito.any;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.eq;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;

import org.cloudfoundry.multiapps.controller.api.model.ImmutableOperation;
import org.cloudfoundry.multiapps.controller.api.model.Operation;
import org.cloudfoundry.multiapps.controller.core.util.ApplicationConfiguration;
import org.cloudfoundry.multiapps.controller.persistence.model.FileEntry;
import org.cloudfoundry.multiapps.controller.persistence.model.ImmutableFileEntry;
import org.cloudfoundry.multiapps.controller.persistence.query.OperationQuery;
import org.cloudfoundry.multiapps.controller.persistence.services.FileService;
import org.cloudfoundry.multiapps.controller.persistence.services.FileStorageException;
import org.cloudfoundry.multiapps.controller.persistence.services.OperationService;
import org.cloudfoundry.multiapps.controller.process.flowable.FlowableFacade;
import org.flowable.variable.api.history.HistoricVariableInstance;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.mockito.Answers;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;

import java.util.List;

class OrphanedFilesCleanerTest {

@Mock
private FileService fileService;
@Mock
private ApplicationConfiguration configuration;
@Mock
private FlowableFacade flowableFacade;
@Mock
private OperationService operationService;
@Mock(answer = Answers.RETURNS_SELF)
private OperationQuery query;
@InjectMocks
private OrphanedFilesCleaner cleaner;

@BeforeEach
void initMocks() throws Exception {
MockitoAnnotations.openMocks(this)
.close();
}

@Test
void testWithOrphanedFiles() throws FileStorageException {
when(configuration.getApplicationInstanceIndex()).thenReturn(1);
when(query.list()).thenReturn(List.of());
when(operationService.createQuery()).thenReturn(query);
when(fileService.listFilesCreatedAfter(any())).thenReturn(List.of(createFileEntry("id-1", "space-1"),
createFileEntry("id-2", "space-2")));

cleaner.run();

verify(fileService, times(2)).deleteFile(any(), any());
}

@Test
void testWithoutOrphanedFiles() throws FileStorageException {
when(configuration.getApplicationInstanceIndex()).thenReturn(1);
when(query.list()).thenReturn(List.of(createOperation("process-1"), createOperation("process-2")));
when(operationService.createQuery()).thenReturn(query);
when(fileService.listFilesCreatedAfter(any())).thenReturn(List.of(createFileEntry("id-1", "space-1"),
createFileEntry("id-2", "space-2")));
var histVar1 = mock(HistoricVariableInstance.class);
when(histVar1.getValue()).thenReturn("id-1");
var histVar2 = mock(HistoricVariableInstance.class);
when(histVar2.getValue()).thenReturn("id-2");
when(flowableFacade.getHistoricVariableInstance(eq("process-1"), any())).thenReturn(histVar1);
when(flowableFacade.getHistoricVariableInstance(eq("process-2"), any())).thenReturn(histVar2);

cleaner.run();

verify(fileService, never()).deleteFile(any(), any());
}

private Operation createOperation(String processId) {
return ImmutableOperation.builder()
.processId(processId)
.build();
}

private FileEntry createFileEntry(String id, String space) {
return ImmutableFileEntry.builder()
.id(id)
.space(space)
.build();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,16 +28,15 @@ public void afterPropertiesSet() {
}

private ObjectStoreFileStorage createObjectStoreFileStorage() {
BlobStoreContext context = getBlobStoreContext();
return context == null ? null : new ObjectStoreFileStorage(context.getBlobStore(), getServiceInfo().getContainer());
}

private BlobStoreContext getBlobStoreContext() {
BlobStoreContext blobStoreContext;
ObjectStoreServiceInfo serviceInfo = getServiceInfo();
if (serviceInfo == null) {
return null;
}
BlobStoreContext context = getBlobStoreContext(serviceInfo);
return context == null ? null : new ObjectStoreFileStorage(context.getBlobStore(), serviceInfo.getContainer());
}

private BlobStoreContext getBlobStoreContext(ObjectStoreServiceInfo serviceInfo) {
ContextBuilder contextBuilder = ContextBuilder.newBuilder(serviceInfo.getProvider());
if (serviceInfo.getCredentialsSupplier() != null) {
contextBuilder.credentialsSupplier(serviceInfo.getCredentialsSupplier());
Expand All @@ -49,8 +48,7 @@ private BlobStoreContext getBlobStoreContext() {
if (serviceInfo.getEndpoint() != null) {
contextBuilder.endpoint(serviceInfo.getEndpoint());
}
blobStoreContext = contextBuilder.buildView(BlobStoreContext.class);
return blobStoreContext;
return contextBuilder.buildView(BlobStoreContext.class);
}

private ObjectStoreServiceInfo getServiceInfo() {
Expand Down
Loading