From aa3f32f3da3e7959bcd75cd38edaa360d8629f1b Mon Sep 17 00:00:00 2001 From: LSmyrnaios Date: Wed, 30 Aug 2023 14:02:54 +0300 Subject: [PATCH] - Make sure the given number of threads, given by the user is above zero. - Adjust the number and size of log files. - Update Spring Boot. - Code polishing. --- build.gradle | 2 +- .../controllers/BulkImportController.java | 3 +++ .../controllers/ShutdownController.java | 2 +- .../urls_controller/controllers/UrlsController.java | 11 +++++++---- .../urls_controller/services/UrlsServiceImpl.java | 3 +++ .../eu/openaire/urls_controller/util/FileUtils.java | 4 ++-- src/main/resources/logback-spring.xml | 4 ++-- 7 files changed, 19 insertions(+), 10 deletions(-) diff --git a/build.gradle b/build.gradle index 34362c5..2859a5e 100644 --- a/build.gradle +++ b/build.gradle @@ -1,5 +1,5 @@ plugins { - id 'org.springframework.boot' version '2.7.14' + id 'org.springframework.boot' version '2.7.15' id 'io.spring.dependency-management' version '1.1.3' id 'java' } diff --git a/src/main/java/eu/openaire/urls_controller/controllers/BulkImportController.java b/src/main/java/eu/openaire/urls_controller/controllers/BulkImportController.java index 26075dd..3c25370 100644 --- a/src/main/java/eu/openaire/urls_controller/controllers/BulkImportController.java +++ b/src/main/java/eu/openaire/urls_controller/controllers/BulkImportController.java @@ -66,6 +66,9 @@ public class BulkImportController { this.bulkImportService = bulkImportService; numOfThreadsForBulkImportProcedures = bulkImport.getNumOfThreadsForBulkImportProcedures(); + if ( numOfThreadsForBulkImportProcedures <= 0 ) + throw new RuntimeException("The given \"numOfThreadsForBulkImportProcedures\" is not a positive number: " + numOfThreadsForBulkImportProcedures); + logger.info("Will use " + numOfThreadsForBulkImportProcedures + " threads for the bulk-import procedures."); bulkImportExecutor = Executors.newFixedThreadPool(numOfThreadsForBulkImportProcedures); // At most < numOfThreadsForBulkImportProcedures > threads will be used per bulk-import procedure.. } diff --git a/src/main/java/eu/openaire/urls_controller/controllers/ShutdownController.java b/src/main/java/eu/openaire/urls_controller/controllers/ShutdownController.java index b8be1c8..b3ee208 100644 --- a/src/main/java/eu/openaire/urls_controller/controllers/ShutdownController.java +++ b/src/main/java/eu/openaire/urls_controller/controllers/ShutdownController.java @@ -112,7 +112,7 @@ public class ShutdownController { workerInfo.setHasShutdown(true); // This will update the map. - // Return "HTTP-OK" to this worker and wait for the scheduler to check and shutdown the service. + // Return "HTTP-OK" to this worker. If this was part of a shutdown-service request, then wait for the scheduler to check and shutdown the service. return ResponseEntity.ok().build(); } diff --git a/src/main/java/eu/openaire/urls_controller/controllers/UrlsController.java b/src/main/java/eu/openaire/urls_controller/controllers/UrlsController.java index ff724fe..5c05baf 100644 --- a/src/main/java/eu/openaire/urls_controller/controllers/UrlsController.java +++ b/src/main/java/eu/openaire/urls_controller/controllers/UrlsController.java @@ -66,6 +66,9 @@ public class UrlsController { this.workerReportsDirPath = workerReportsDirPath; // This dir will be created later. + if ( numOfBackgroundThreads <= 0 ) + throw new RuntimeException("The given \"numOfBackgroundThreads\" is not a positive number: " + numOfBackgroundThreads); + logger.info("Will use " + numOfBackgroundThreads + " threads for background tasks, such as processing worker-reports or bulk-import procedures."); backgroundExecutor = Executors.newFixedThreadPool(numOfBackgroundThreads); // At most < numOfBackgroundThreads > tasks will be running in parallel. } @@ -155,16 +158,16 @@ public class UrlsController { return ResponseEntity.status(HttpStatus.FORBIDDEN).body(errorMsg); } - int sizeOUrlReports = 0; + int sizeOfUrlReports = 0; List urlReports = workerReport.getUrlReports(); - if ( (urlReports == null) || ((sizeOUrlReports = urlReports.size()) == 0) ) { + if ( (urlReports == null) || ((sizeOfUrlReports = urlReports.size()) == 0) ) { String errorMsg = "The given \"WorkerReport\" from worker with ID \"" + curWorkerId + "\" was empty (without any UrlReports)!"; logger.error(errorMsg); return ResponseEntity.status(HttpStatus.BAD_REQUEST).body(errorMsg); } long curReportAssignmentsCounter = workerReport.getAssignmentRequestCounter(); - logger.info("Received the WorkerReport for batch-assignments_" + curReportAssignmentsCounter + ", from the worker with id: " + curWorkerId + ". It contains " + sizeOUrlReports + " urlReports. Going to request the fullTexts from the Worker and insert the UrlReports into the database."); + logger.info("Received the WorkerReport for batch-assignments_" + curReportAssignmentsCounter + ", from the worker with id: " + curWorkerId + ". It contains " + sizeOfUrlReports + " urlReports. Going to request the fullTexts from the Worker and insert the UrlReports into the database."); // Make sure this worker's report directory is created. Path currentWorkerReportLocationDir = Paths.get(this.workerReportsDirPath, curWorkerId); @@ -183,7 +186,7 @@ public class UrlsController { fileUtils.writeToFile(workerReportFile, workerReport.getJsonReport(), false); // Only one thread is writing to this specific file. // The above method will overwrite a possibly existing file. So in case of a crash, it's better to back up the reports before starting the Controller again (as the assignments-counter will start over, from 0). - int finalSizeOUrlReports = sizeOUrlReports; + int finalSizeOUrlReports = sizeOfUrlReports; UrlsController.backgroundCallableTasks.add(() -> urlsService.addWorkerReport(curWorkerId, curReportAssignmentsCounter, urlReports, finalSizeOUrlReports) ); diff --git a/src/main/java/eu/openaire/urls_controller/services/UrlsServiceImpl.java b/src/main/java/eu/openaire/urls_controller/services/UrlsServiceImpl.java index 6071ffc..1631841 100644 --- a/src/main/java/eu/openaire/urls_controller/services/UrlsServiceImpl.java +++ b/src/main/java/eu/openaire/urls_controller/services/UrlsServiceImpl.java @@ -12,11 +12,13 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Value; +import org.springframework.boot.web.client.RestTemplateBuilder; import org.springframework.dao.EmptyResultDataAccessException; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; import org.springframework.jdbc.core.JdbcTemplate; import org.springframework.stereotype.Service; +import org.springframework.web.client.HttpClientErrorException; import org.springframework.web.client.HttpServerErrorException; import org.springframework.web.client.RestTemplate; @@ -27,6 +29,7 @@ import java.sql.Connection; import java.sql.PreparedStatement; import java.sql.SQLException; import java.sql.Timestamp; +import java.time.Duration; import java.util.ArrayList; import java.util.HashMap; import java.util.List; diff --git a/src/main/java/eu/openaire/urls_controller/util/FileUtils.java b/src/main/java/eu/openaire/urls_controller/util/FileUtils.java index 7d72f38..1ec2f50 100644 --- a/src/main/java/eu/openaire/urls_controller/util/FileUtils.java +++ b/src/main/java/eu/openaire/urls_controller/util/FileUtils.java @@ -59,7 +59,7 @@ public class FileUtils { public static final String workingDir = System.getProperty("user.dir") + File.separator; - private boolean isTestEnvironment; + private final boolean isTestEnvironment; public FileUtils (@Value("${services.pdfaggregation.controller.baseFilesLocation}") String baseFilesLocation, @Value("${services.pdfaggregation.controller.isTestEnvironment}") boolean isTestEnvironment) { @@ -466,7 +466,7 @@ public class FileUtils { //int numUploadedFiles = 0; for( String fileName : fileNames ) { - if ( fileName.contains(".tar") ) // Exclude the tar-files from uploading. + if ( fileName.contains(".tar") ) // Exclude the tar-files from uploading (".tar" and ".tar.zstd"). continue; // Check if this stored file is related to one or more Payloads from the Set. Defend against malicious file injection. It does not add more overhead, since we already need the "fileRelatedPayloads". diff --git a/src/main/resources/logback-spring.xml b/src/main/resources/logback-spring.xml index 0769f58..c8827f2 100644 --- a/src/main/resources/logback-spring.xml +++ b/src/main/resources/logback-spring.xml @@ -6,11 +6,11 @@ logs/UrlsController.%i.log.zip 1 - 20 + 10 - 50MB + 100MB