Skip to content

feat(uploads): add maxFiles configuration policy to jvmId uploaded recordings #1333

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

Merged
merged 8 commits into from
Jan 17, 2023
Merged
Show file tree
Hide file tree
Changes from 7 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
1 change: 1 addition & 0 deletions run.sh
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,7 @@ podman run \
-e CRYOSTAT_DISCOVERY_PING_PERIOD="$CRYOSTAT_DISCOVERY_PING_PERIOD" \
-e CRYOSTAT_ACTIVE_REPORTS_CACHE_EXPIRY_SECONDS="$CRYOSTAT_ACTIVE_REPORTS_CACHE_EXPIRY_SECONDS" \
-e CRYOSTAT_ACTIVE_REPORTS_CACHE_REFRESH_SECONDS="$CRYOSTAT_ACTIVE_REPORTS_CACHE_REFRESH_SECONDS" \
-e CRYOSTAT_PUSH_MAX_FILES="$CRYOSTAT_PUSH_MAX_FILES" \
-e GRAFANA_DATASOURCE_URL="$GRAFANA_DATASOURCE_URL" \
-e GRAFANA_DASHBOARD_URL="$GRAFANA_DASHBOARD_URL" \
-e KEYSTORE_PATH="$KEYSTORE_PATH" \
Expand Down
3 changes: 3 additions & 0 deletions src/main/java/io/cryostat/configuration/Variables.java
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,9 @@ private Variables() {}
public static final String ACTIVE_REPORTS_CACHE_REFRESH_ENV =
"CRYOSTAT_ACTIVE_REPORTS_CACHE_REFRESH_SECONDS";

// agent configuration
public static final String PUSH_MAX_FILES_ENV = "CRYOSTAT_PUSH_MAX_FILES";

// SSL configuration
public static final String DISABLE_SSL = "CRYOSTAT_DISABLE_SSL";
public static final String KEYSTORE_PATH_ENV = "KEYSTORE_PATH";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@
import io.cryostat.platform.discovery.EnvironmentNode;
import io.cryostat.platform.discovery.NodeType;
import io.cryostat.platform.discovery.TargetNode;
import io.cryostat.recordings.JvmIdHelper;
import io.cryostat.util.PluggableTypeAdapter;

import com.google.gson.JsonSyntaxException;
Expand All @@ -66,17 +65,12 @@
public class AbstractNodeTypeAdapter extends PluggableTypeAdapter<AbstractNode> {

private final Lazy<Set<PluggableTypeAdapter<?>>> adapters;
private final Lazy<JvmIdHelper> jvmIdHelper;
private final Logger logger;

public AbstractNodeTypeAdapter(
Class<AbstractNode> klazz,
Lazy<Set<PluggableTypeAdapter<?>>> adapters,
Lazy<JvmIdHelper> jvmIdHelper,
Logger logger) {
Class<AbstractNode> klazz, Lazy<Set<PluggableTypeAdapter<?>>> adapters, Logger logger) {
super(klazz);
this.adapters = adapters;
this.jvmIdHelper = jvmIdHelper;
this.logger = logger;
}

Expand Down
6 changes: 2 additions & 4 deletions src/main/java/io/cryostat/discovery/DiscoveryModule.java
Original file line number Diff line number Diff line change
Expand Up @@ -128,9 +128,7 @@ static BuiltInDiscovery provideBuiltInDiscovery(
@Provides
@IntoSet
static PluggableTypeAdapter<?> provideBaseNodeTypeAdapter(
Lazy<Set<PluggableTypeAdapter<?>>> adapters,
Lazy<JvmIdHelper> jvmIdHelper,
Logger logger) {
return new AbstractNodeTypeAdapter(AbstractNode.class, adapters, jvmIdHelper, logger);
Lazy<Set<PluggableTypeAdapter<?>>> adapters, Logger logger) {
return new AbstractNodeTypeAdapter(AbstractNode.class, adapters, logger);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@

import io.cryostat.MainModule;
import io.cryostat.configuration.CredentialsManager;
import io.cryostat.configuration.Variables;
import io.cryostat.core.log.Logger;
import io.cryostat.core.sys.FileSystem;
import io.cryostat.messaging.notifications.NotificationFactory;
Expand Down Expand Up @@ -94,6 +95,7 @@ public class RecordingsFromIdPostHandler extends AbstractAuthenticatedRequestHan
private final RecordingArchiveHelper recordingArchiveHelper;
private final RecordingMetadataManager recordingMetadataManager;
private final Path savedRecordingsPath;
private final int globalMaxFiles;
private final Provider<WebServer> webServer;

private static final String NOTIFICATION_CATEGORY = "ArchivedRecordingCreated";
Expand All @@ -109,6 +111,7 @@ public class RecordingsFromIdPostHandler extends AbstractAuthenticatedRequestHan
RecordingArchiveHelper recordingArchiveHelper,
RecordingMetadataManager recordingMetadataManager,
@Named(MainModule.RECORDINGS_PATH) Path savedRecordingsPath,
@Named(Variables.PUSH_MAX_FILES_ENV) int globalMaxFiles,
Provider<WebServer> webServer,
Logger logger) {
super(auth, credentialsManager, logger);
Expand All @@ -118,6 +121,7 @@ public class RecordingsFromIdPostHandler extends AbstractAuthenticatedRequestHan
this.recordingArchiveHelper = recordingArchiveHelper;
this.recordingMetadataManager = recordingMetadataManager;
this.savedRecordingsPath = savedRecordingsPath;
this.globalMaxFiles = globalMaxFiles;
this.webServer = webServer;
this.gson = gson;
this.logger = logger;
Expand All @@ -135,7 +139,11 @@ public HttpMethod httpMethod() {

@Override
public Set<ResourceAction> resourceActions() {
return EnumSet.of(ResourceAction.CREATE_RECORDING);
return EnumSet.of(
ResourceAction.CREATE_RECORDING,
ResourceAction.READ_RECORDING,
ResourceAction.DELETE_RECORDING,
ResourceAction.DELETE_REPORT);
}

@Override
Expand Down Expand Up @@ -170,6 +178,17 @@ public void handleAuthenticated(RoutingContext ctx) throws Exception {
throw new ApiException(503, "Recording saving not available");
}

String maxFilesParam = ctx.request().getParam("maxFiles", String.valueOf(globalMaxFiles));
int maxFiles;
try {
maxFiles = Integer.parseInt(maxFilesParam);
if (maxFiles <= 0) {
throw new ApiException(400, "maxFiles must be a positive integer");
}
} catch (NumberFormatException e) {
throw new ApiException(400, "maxFiles must be a positive integer");
}

FileUpload upload =
webServer
.get()
Expand All @@ -195,7 +214,7 @@ public void handleAuthenticated(RoutingContext ctx) throws Exception {
}
} catch (JvmIdDoesNotExistException e) {
recordingArchiveHelper.deleteTempFileUpload(upload);
throw new ApiException(400, String.format("jvmId [%s] must be valid ", e.getMessage()));
throw new ApiException(400, String.format("jvmId must be valid: %s", e.getMessage()));
}
String subdirectoryName = idHelper.jvmIdToSubdirectoryName(jvmId);

Expand Down Expand Up @@ -258,14 +277,16 @@ public void handleAuthenticated(RoutingContext ctx) throws Exception {
}

String fsName = res2.result();

try {
recordingArchiveHelper.pruneTargetUploads(
subdirectoryName, maxFiles);
if (hasLabels) {
recordingMetadataManager
.setRecordingMetadataFromPath(
subdirectoryName, fsName, metadata)
.get();
}

} catch (InterruptedException
| ExecutionException
| IOException e) {
Expand Down
63 changes: 54 additions & 9 deletions src/main/java/io/cryostat/recordings/RecordingArchiveHelper.java
Original file line number Diff line number Diff line change
Expand Up @@ -571,7 +571,6 @@ private void validateSavePath(String recordingName, Path path) throws IOExceptio
public boolean deleteReports(String subdirectoryName, String recordingName) {
try {
logger.info("Invalidating archived report cache for {}", recordingName);
boolean deleted = true;
for (String reportName :
fs.listDirectoryChildren(
archivedRecordingsReportPath.resolve(subdirectoryName))) {
Expand All @@ -581,17 +580,14 @@ public boolean deleteReports(String subdirectoryName, String recordingName) {
.resolve(subdirectoryName)
.resolve(reportName)
.toAbsolutePath();
if (!fs.deleteIfExists(reportPath)) {
logger.warn("Failed to delete report {}", reportPath);
deleted = false;
} else {
logger.trace("Deleted report {}", reportName);
if (fs.exists(reportPath)) {
fs.deleteIfExists(reportPath);
}
}
}
return deleted;
return true;
} catch (IOException e) {
logger.warn(e);
logger.warn("Failed to delete report cache for {}", recordingName);
return false;
}
}
Expand Down Expand Up @@ -1042,6 +1038,16 @@ private long getArchivedTime(String recordingName) {
}
}

private long getLastModifiedTime(Path path) {
try {
FileTime fileTime = (FileTime) Files.getAttribute(path, "lastModifiedTime");
return fileTime.toMillis();
} catch (IOException e) {
logger.error("Invalid path: {}", path);
return 0;
}
}

// Timestamp must be in form of 20191219T213834Z (YYYYMMDDTHHMMSSZ)
// Used on the third regex matcher group of a Cryostat archived recording name
public long getArchivedTimeFromTimestamp(String timestamp) {
Expand All @@ -1063,6 +1069,46 @@ public long getArchivedTimeFromTimestamp(String timestamp) {
}
}

// Preconditions:
// 1. The uploaded recording(s) were saved to the fs before calling this method
public boolean pruneTargetUploads(String subdirectoryName, int maxUploads) throws IOException {
Path subdirectoryPath = this.archivedRecordingsPath.resolve(subdirectoryName);
if (!fs.exists(subdirectoryPath)) {
throw new IllegalArgumentException("Invalid path: " + subdirectoryPath);
}
List<String> recordings = fs.listDirectoryChildren(subdirectoryPath);
recordings.remove(CONNECT_URL);
if (recordings.size() <= maxUploads) {
return false;
}

List<Future<ArchivedRecordingInfo>> toDelete =
recordings.stream()
.map(subdirectoryPath::resolve)
.filter(fs::isRegularFile)
.filter(n -> n.getFileName().toString().endsWith(".jfr"))
.sorted(
(Path p1, Path p2) ->
this.getLastModifiedTime(p1) > this.getLastModifiedTime(p2)
? -1
: 1)
.map(Path::getFileName)
.map(Path::toString)
.skip(maxUploads)
.map((String r) -> this.deleteRecordingFromPath(subdirectoryName, r))
.toList();
if (toDelete.isEmpty()) {
return false;
}
try {
CompletableFuture.allOf(toDelete.toArray(new CompletableFuture[toDelete.size()])).get();
} catch (InterruptedException | ExecutionException e) {
logger.error("Failed to delete recordings: {}", e.getMessage());
return false;
}
return true;
}

public void saveUploadedRecording(
String subdirectoryName,
String basename,
Expand Down Expand Up @@ -1131,7 +1177,6 @@ public void saveUploadedRecording(
makeFailedAsyncResult(res2.cause()));
return;
}

handler.handle(makeAsyncResult(filename));
});
});
Expand Down
7 changes: 7 additions & 0 deletions src/main/java/io/cryostat/recordings/RecordingsModule.java
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,13 @@ static long provideJmxConnectionTimeoutSeconds(Environment env) {
return Math.max(1, Long.parseLong(env.getEnv(Variables.JMX_CONNECTION_TIMEOUT, "3")));
}

@Provides
@Named(Variables.PUSH_MAX_FILES_ENV)
static int providePushMaxFiles(Environment env) {
return Integer.parseInt(
env.getEnv(Variables.PUSH_MAX_FILES_ENV, String.valueOf(Integer.MAX_VALUE)));
}

@Provides
@Singleton
static RecordingTargetHelper provideRecordingTargetHelper(
Expand Down
Loading