Merge pull request #9728 from atmire/w2p-116609_improve-running-process-observability-main

Improve observability of running processes + Fail them during startup

dspace-api/src/main/java/org/dspace/scripts/ProcessServiceImpl.java

@@ -45,14 +45,15 @@ import org.dspace.core.Context;
 import org.dspace.core.LogHelper;
 import org.dspace.eperson.EPerson;
 import org.dspace.eperson.Group;
-import org.dspace.eperson.service.EPersonService;
 import org.dspace.scripts.service.ProcessService;
+import org.dspace.services.ConfigurationService;
+import org.springframework.beans.factory.InitializingBean;
 import org.springframework.beans.factory.annotation.Autowired;
 
 /**
  * The implementation for the {@link ProcessService} class
  */
-public class ProcessServiceImpl implements ProcessService {
+public class ProcessServiceImpl implements ProcessService, InitializingBean {
 
     private static final Logger log = org.apache.logging.log4j.LogManager.getLogger(ProcessService.class);
 
@@ -72,7 +73,34 @@ public class ProcessServiceImpl implements ProcessService {
     private MetadataFieldService metadataFieldService;
 
     @Autowired
-    private EPersonService ePersonService;
+    private ConfigurationService configurationService;
+
+    @Override
+    public void afterPropertiesSet() throws Exception {
+        try {
+            Context context = new Context();
+
+            // Processes that were running or scheduled when tomcat crashed, should be cleaned up during startup.
+            List<Process> processesToBeFailed = findByStatusAndCreationTimeOlderThan(
+                context, List.of(ProcessStatus.RUNNING, ProcessStatus.SCHEDULED), new Date());
+            for (Process process : processesToBeFailed) {
+                context.setCurrentUser(process.getEPerson());
+                // Fail the process.
+                log.info("Process with ID {} did not complete before tomcat shutdown, failing it now.",
+                         process.getID());
+                fail(context, process);
+                // But still attach its log to the process.
+                appendLog(process.getID(), process.getName(),
+                          "Process did not complete before tomcat shutdown.",
+                          ProcessLogLevel.ERROR);
+                createLogBitstream(context, process);
+            }
+
+            context.complete();
+        } catch (Exception e) {
+            log.error("Unable to clean up Processes: ", e);
+        }
+    }
 
     @Override
     public Process create(Context context, EPerson ePerson, String scriptName,
@@ -293,8 +321,8 @@ public class ProcessServiceImpl implements ProcessService {
     @Override
     public void appendLog(int processId, String scriptName, String output, ProcessLogLevel processLogLevel)
             throws IOException {
-        File tmpDir = FileUtils.getTempDirectory();
+        File logsDir = getLogsDirectory();
-        File tempFile = new File(tmpDir, scriptName + processId + ".log");
+        File tempFile = new File(logsDir, processId + "-" + scriptName + ".log");
         FileWriter out = new FileWriter(tempFile, true);
         try {
             try (BufferedWriter writer = new BufferedWriter(out)) {
@@ -309,13 +337,16 @@ public class ProcessServiceImpl implements ProcessService {
     @Override
     public void createLogBitstream(Context context, Process process)
             throws IOException, SQLException, AuthorizeException {
-        File tmpDir = FileUtils.getTempDirectory();
+        File logsDir = getLogsDirectory();
-        File tempFile = new File(tmpDir, process.getName() + process.getID() + ".log");
+        File tempFile = new File(logsDir, process.getID() + "-" + process.getName() + ".log");
+        if (tempFile.exists()) {
             FileInputStream inputStream = FileUtils.openInputStream(tempFile);
-        appendFile(context, process, inputStream, Process.OUTPUT_TYPE, process.getName() + process.getID() + ".log");
+            appendFile(context, process, inputStream, Process.OUTPUT_TYPE,
+                       process.getID() + "-" + process.getName() + ".log");
             inputStream.close();
             tempFile.delete();
         }
+    }
 
     @Override
     public List<Process> findByStatusAndCreationTimeOlderThan(Context context, List<ProcessStatus> statuses,
@@ -343,4 +374,15 @@ public class ProcessServiceImpl implements ProcessService {
         return  sb.toString();
     }
 
+    private File getLogsDirectory() {
+        String pathStr = configurationService.getProperty("dspace.dir")
+            + File.separator + "log" + File.separator + "processes";
+        File logsDir = new File(pathStr);
+        if (!logsDir.exists()) {
+            if (!logsDir.mkdirs()) {
+                throw new RuntimeException("Couldn't create [dspace.dir]/log/processes/ directory.");
+            }
+        }
+        return logsDir;
+    }
 }

dspace-server-webapp/src/main/java/org/dspace/app/rest/scripts/handler/impl/RestDSpaceRunnableHandler.java

@@ -130,7 +130,7 @@ public class RestDSpaceRunnableHandler implements DSpaceRunnableHandler {
 
     @Override
     public void handleException(Exception e) {
-        handleException(null, e);
+        handleException(e.getMessage(), e);
     }
 
     @Override

dspace-server-webapp/src/test/java/org/dspace/app/rest/ProcessRestRepositoryIT.java

@@ -929,10 +929,10 @@ public class ProcessRestRepositoryIT extends AbstractControllerIntegrationTest {
         getClient(token).perform(get("/api/system/processes/" + process1.getID() + "/output"))
                         .andExpect(status().isOk())
                         .andExpect(jsonPath("$.name",
-                                            is(process1.getName() + process1.getID() + ".log")))
+                                            is(process1.getID() + "-" + process1.getName() + ".log")))
                         .andExpect(jsonPath("$.type", is("bitstream")))
                         .andExpect(jsonPath("$.metadata['dc.title'][0].value",
-                                            is(process1.getName() + process1.getID() + ".log")))
+                                            is(process1.getID() + "-" + process1.getName() + ".log")))
                         .andExpect(jsonPath("$.metadata['dspace.process.filetype'][0].value",
                                             is("script_output")));
 
@@ -942,10 +942,10 @@ public class ProcessRestRepositoryIT extends AbstractControllerIntegrationTest {
                         .perform(get("/api/system/processes/" + process1.getID() + "/output"))
                         .andExpect(status().isOk())
                         .andExpect(jsonPath("$.name",
-                                            is(process1.getName() + process1.getID() + ".log")))
+                                            is(process1.getID() + "-" + process1.getName() + ".log")))
                         .andExpect(jsonPath("$.type", is("bitstream")))
                         .andExpect(jsonPath("$.metadata['dc.title'][0].value",
-                                            is(process1.getName() + process1.getID() + ".log")))
+                                            is(process1.getID() + "-" + process1.getName() + ".log")))
                         .andExpect(jsonPath("$.metadata['dspace.process.filetype'][0].value",
                                             is("script_output")));