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.

Sign up for GitHub

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

Development: Improve build stats performance #10108

Draft
wants to merge 3 commits into
base: develop
Choose a base branch
from
+188 −138
Draft

Development: Improve build stats performance #10108

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5502cb2
use less database queries
krusche Jan 5, 2025
6d81724
fix query (check if works with postgres)
BBesrour Jan 6, 2025
249c32d
fix issues in query
krusche Jan 6, 2025
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
4 changes: 2 additions & 2 deletions src/main/java/de/tum/cit/aet/artemis/assessment/repository/LongFeedbackTextRepository.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,15 +45,15 @@ public interface LongFeedbackTextRepository extends ArtemisJpaRepository<LongFee
Optional<LongFeedbackText> findWithFeedbackAndResultAndParticipationByFeedbackId(@Param("feedbackId") final Long feedbackId);

@Modifying
@Transactional
@Transactional // ok because of delete
@Query("""
DELETE FROM LongFeedbackText longFeedback
WHERE longFeedback.feedback.id IN :feedbackIds
""")
void deleteByFeedbackIds(@Param("feedbackIds") List<Long> feedbackIds);

@Modifying
@Transactional
@Transactional // ok because of delete
void deleteByFeedbackId(final Long feedbackId);

default LongFeedbackText findByFeedbackIdWithFeedbackAndResultAndParticipationElseThrow(final Long feedbackId) {
Expand Down
2 changes: 1 addition & 1 deletion .../cit/aet/artemis/assessment/repository/cleanup/PlagiarismComparisonCleanupRepository.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ SELECT COUNT(s)
int countPlagiarismSubmissionsByComparisonIdsIn(@Param("ids") List<Long> ids);

@Modifying
@Transactional // ok because of modifying
@Transactional // ok because of modifying query
@Query("""
UPDATE PlagiarismComparison pc
SET pc.submissionA = NULL, pc.submissionB = NULL
Expand Down
39 changes: 21 additions & 18 deletions src/main/java/de/tum/cit/aet/artemis/buildagent/service/BuildAgentDockerService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,11 @@ public void cleanUpContainers() {
.filter(container -> container.getNames()[0].startsWith("/" + buildContainerPrefix)).toList();
}
catch (Exception ex) {
if (DockerUtil.isDockerNotAvailable(ex)) {
log.error("Cannot connect to Docker Host. Make sure Docker is running and configured properly! Error while listing containers for cleanup: {}",
ex.getMessage());
return;
}
log.error("Make sure Docker is running and configured properly! Error while listing containers for cleanup: {}", ex.getMessage(), ex);
return;
}
Expand Down Expand Up @@ -170,15 +175,6 @@ public void cleanUpContainers() {
*/
public static class MyPullImageResultCallback extends PullImageResultCallback {

private final String buildJobId;

private final BuildLogsMap buildLogsMap;

MyPullImageResultCallback(String buildJobId, BuildLogsMap buildLogsMap) {
this.buildJobId = buildJobId;
this.buildLogsMap = buildLogsMap;
}

@Override
public void onNext(PullResponseItem item) {
String msg = "~~~~~~~~~~~~~~~~~~~~ Pull image progress: " + item.getStatus() + " ~~~~~~~~~~~~~~~~~~~~";
Expand Down Expand Up @@ -217,12 +213,12 @@ public void onComplete() {
public void pullDockerImage(BuildJobQueueItem buildJob, BuildLogsMap buildLogsMap) {
DockerClient dockerClient = buildAgentConfiguration.getDockerClient();
final String imageName = buildJob.buildConfig().dockerImage();
try {
try (var inspectImageCommand = dockerClient.inspectImageCmd(imageName)) {
// First check if the image is already available
String msg = "~~~~~~~~~~~~~~~~~~~~ Inspecting docker image " + imageName + " ~~~~~~~~~~~~~~~~~~~~";
log.info(msg);
buildLogsMap.appendBuildLogEntry(buildJob.id(), msg);
var inspectImageResponse = dockerClient.inspectImageCmd(imageName).exec();
var inspectImageResponse = inspectImageCommand.exec();
checkImageArchitecture(imageName, inspectImageResponse, buildJob, buildLogsMap);
}
catch (NotFoundException | BadRequestException e) {
Expand All @@ -247,7 +243,7 @@ public void pullDockerImage(BuildJobQueueItem buildJob, BuildLogsMap buildLogsMa
try {
// Only pull the image if the inspect command failed
var command = dockerClient.pullImageCmd(imageName).withPlatform(imageArchitecture);
var exec = command.exec(new MyPullImageResultCallback(buildJob.id(), buildLogsMap));
var exec = command.exec(new MyPullImageResultCallback());
exec.awaitCompletion();

// Check if the image is compatible with the current architecture
Expand All @@ -264,6 +260,13 @@ public void pullDockerImage(BuildJobQueueItem buildJob, BuildLogsMap buildLogsMa
log.info(msg2);
buildLogsMap.appendBuildLogEntry(buildJob.id(), msg2);
}
catch (Exception ex) {
if (DockerUtil.isDockerNotAvailable(ex)) {
log.error("Cannot connect to Docker Host. Make sure Docker is running and configured properly! Error while inspecting image: {}", ex.getMessage());
}
throw new LocalCIException("Cannot connect to Docker Host. Make sure Docker is running and configured properly!", ex);
// Do not proceed if Docker is not running
}
finally {
lock.unlock();
}
Expand Down Expand Up @@ -320,12 +323,12 @@ public void deleteOldDockerImages() {
for (String dockerImage : dockerImageCleanupInfo.keySet()) {
if (imageNames.contains(dockerImage)) {
if (dockerImageCleanupInfo.get(dockerImage).isBefore(ZonedDateTime.now().minusDays(imageExpiryDays))) {
log.info("Deleting docker image {}", dockerImage);
try {
buildAgentConfiguration.getDockerClient().removeImageCmd(dockerImage).exec();
log.info("Remove docker image {} because it was not used for at least {} days", dockerImage, imageExpiryDays);
try (final var removeCommand = buildAgentConfiguration.getDockerClient().removeImageCmd(dockerImage)) {
removeCommand.exec();
}
catch (NotFoundException e) {
log.warn("Docker image {} not found during cleanup", dockerImage);
log.warn("Docker image {} not found during cleaning up old docker images", dockerImage);
}
}
}
Expand Down Expand Up @@ -379,14 +382,14 @@ public void checkUsableDiskSpaceThenCleanUp() {
Map.Entry<String, ZonedDateTime> oldestImage = mutableSortedImagesByLastBuildDate.getFirst();
while (oldestImage != null && usableSpace < threshold && deleteAttempts > 0 && totalAttempts > 0) {
if (unusedImages.contains(oldestImage.getKey())) {
log.info("Deleting docker image {}", oldestImage.getKey());
log.info("Remove oldest docker image {} to cleanup disk space to avoid filling up the hard disk", oldestImage.getKey());
try {
dockerClient.removeImageCmd(oldestImage.getKey()).exec();
usableSpace = dockerRootDirectory.getUsableSpace();
deleteAttempts--;
}
catch (NotFoundException e) {
log.warn("Docker image {} not found during cleanup", oldestImage.getKey());
log.warn("Docker image {} not found during disk cleanup", oldestImage.getKey());
}
}
mutableSortedImagesByLastBuildDate.remove(oldestImage);
Expand Down
Loading
Loading