Change of threading policy, configurable timeout

Author: georgweiss

app/save-and-restore/util/src/main/java/org/phoebus/saveandrestore/util/SnapshotUtil.java

@@ -41,8 +41,6 @@ public class SnapshotUtil {
 
     private final Logger LOG = Logger.getLogger(SnapshotUtil.class.getName());
 
-    private final int connectionTimeout = Preferences.connectionTimeout;
-
     private final ExecutorService executorService = Executors.newFixedThreadPool(10);
 
     public SnapshotUtil() {
@@ -70,13 +68,30 @@ public SnapshotUtil() {
      * @param snapshotItems {@link SnapshotItem}
      */
     public synchronized List<RestoreResult> restore(List<SnapshotItem> snapshotItems) {
+        return restore(snapshotItems, Preferences.connectionTimeout);
+    }
+
+    /**
+     * Restore PV values from a list of snapshot items
+     *
+     * <p>
+     * Writes concurrently the pv value to the non-null set PVs in
+     * the snapshot items.
+     * Uses synchronized to ensure only one frontend can write at a time.
+     * Returns a list of the snapshot items you have set, with an error message if
+     * an error occurred.
+     *
+     * @param snapshotItems {@link SnapshotItem}
+     * @param connectionTimeout The timeout in ms to use for EPICS connection.
+     */
+    public synchronized List<RestoreResult> restore(List<SnapshotItem> snapshotItems, long connectionTimeout) {
         // First clean the list of SnapshotItems from read-only elements.
         List<SnapshotItem> cleanedSnapshotItems = cleanSnapshotItems(snapshotItems);
         List<RestoreResult> restoreResultList = new ArrayList<>();
 
         List<RestoreCallable> callables = new ArrayList<>();
         for (SnapshotItem si : cleanedSnapshotItems) {
-            RestoreCallable restoreCallable = new RestoreCallable(si, restoreResultList);
+            RestoreCallable restoreCallable = new RestoreCallable(si, restoreResultList, connectionTimeout);
             callables.add(restoreCallable);
         }
 
@@ -98,25 +113,53 @@ public synchronized List<RestoreResult> restore(List<SnapshotItem> snapshotItems
      * {@link ConfigPv} item in {@link ConfigurationData} a {@link SnapshotItem} is created holding the
      * values read.
      * Read operations are concurrent using a thread pool. Failed connections/reads will cause a wait of at most
-     * {@link #connectionTimeout} ms on each thread.
+     * {@link Preferences#connectionTimeout} ms on each thread.
      *
      * @param configurationData Identifies which {@link Configuration} user selected to create a snapshot.
      * @return A list of {@link SnapshotItem}s holding the values read from IOCs.
      */
     public List<SnapshotItem> takeSnapshot(ConfigurationData configurationData) {
-        return takeSnapshot(configurationData.getPvList());
+        return takeSnapshot(configurationData.getPvList(), Preferences.connectionTimeout);
+    }
+
+    /**
+     * Reads all PVs and read-back PVs as defined in the {@link ConfigurationData} argument. For each
+     * {@link ConfigPv} item in {@link ConfigurationData} a {@link SnapshotItem} is created holding the
+     * values read.
+     * Read operations are concurrent using a thread pool. Failed connections/reads will cause a wait of at most
+     * <code>connectionTimeout</code> ms on each thread.
+     *
+     * @param configurationData Identifies which {@link Configuration} user selected to create a snapshot.
+     * @return A list of {@link SnapshotItem}s holding the values read from IOCs.
+     */
+    public List<SnapshotItem> takeSnapshot(ConfigurationData configurationData, long connectionTimeout) {
+        return takeSnapshot(configurationData.getPvList(), connectionTimeout);
     }
 
     /**
      * Reads all PVs and read-back PVs as defined in the {@link ConfigurationData} argument. For each
      * {@link ConfigPv} item in {@link ConfigurationData} a {@link SnapshotItem} is created.
      * Read operations are concurrent using a thread pool. Failed connections/reads will cause a wait of at most
-     * {@link #connectionTimeout} ms on each thread.
+     * {@link Preferences#connectionTimeout} ms on each thread.
      *
      * @param configPvs List of {@link ConfigPv}s defining a {@link Configuration}.
      * @return A list of {@link SnapshotItem}s holding the values read from IOCs.
      */
     public List<SnapshotItem> takeSnapshot(final List<ConfigPv> configPvs) {
+        return takeSnapshot(configPvs, Preferences.connectionTimeout);
+    }
+
+    /**
+     * Reads all PVs and read-back PVs as defined in the {@link ConfigurationData} argument. For each
+     * {@link ConfigPv} item in {@link ConfigurationData} a {@link SnapshotItem} is created.
+     * Read operations are concurrent using a thread pool. Failed connections/reads will cause a wait of at most
+     * <code>connectionTimeout</code> ms on each thread.
+     *
+     * @param configPvs List of {@link ConfigPv}s defining a {@link Configuration}.
+     * @param connectionTimeout The timeout in ms to use for EPICS connection.
+     * @return A list of {@link SnapshotItem}s holding the values read from IOCs.
+     */
+    public List<SnapshotItem> takeSnapshot(final List<ConfigPv> configPvs, long connectionTimeout) {
         List<SnapshotItem> snapshotItems = new ArrayList<>();
         List<Callable<Void>> callables = new ArrayList<>();
         Map<String, VType> pvValues = Collections.synchronizedMap(new HashMap<>());
@@ -232,10 +275,12 @@ private class RestoreCallable implements Callable<Void> {
         private final List<RestoreResult> restoreResultList;
         private PV pv;
         private final SnapshotItem snapshotItem;
+        private final long connectionTimeout;
 
-        public RestoreCallable(SnapshotItem snapshotItem, List<RestoreResult> restoreResultList) {
+        public RestoreCallable(SnapshotItem snapshotItem, List<RestoreResult> restoreResultList, long connectionTimeout) {
             this.snapshotItem = snapshotItem;
             this.restoreResultList = restoreResultList;
+            this.connectionTimeout = connectionTimeout;
         }
 
         @Override

services/save-and-restore/src/main/java/org/phoebus/service/saveandrestore/web/config/WebConfiguration.java

@@ -21,8 +21,10 @@
 import org.phoebus.service.saveandrestore.persistence.dao.NodeDAO;
 import org.phoebus.service.saveandrestore.persistence.dao.impl.elasticsearch.ElasticsearchDAO;
 import org.phoebus.service.saveandrestore.websocket.WebSocket;
+import org.springframework.beans.factory.annotation.Value;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
+import org.springframework.context.annotation.PropertySource;
 import org.springframework.context.annotation.Scope;
 
 import java.util.List;
@@ -34,8 +36,18 @@
  * {@link Configuration} class setting up beans for {@link org.springframework.stereotype.Controller} classes.
  */
 @Configuration
+@PropertySource("classpath:application.properties")
 public class WebConfiguration {
 
+    @Value("${connection.timeout:5000}")
+    public long connectionTimeout;
+
+    @SuppressWarnings("unused")
+    @Bean
+    public long getConnectionTimeout(){
+        return connectionTimeout;
+    }
+
     /**
      *
      * @return A {@link NodeDAO} instance.
@@ -63,11 +75,13 @@ public SnapshotUtil snapshotRestorer(){
         return new SnapshotUtil();
     }
 
+    @SuppressWarnings("unused")
     @Bean
     public ExecutorService executorService(){
         return Executors.newCachedThreadPool();
     }
 
+    @SuppressWarnings("unused")
     @Bean(name = "sockets")
     @Scope("singleton")
     public List<WebSocket> getSockets() {

services/save-and-restore/src/main/java/org/phoebus/service/saveandrestore/web/controllers/BaseController.java

@@ -33,6 +33,7 @@
 /**
  * Base controller that should be extended to make sure exceptions are handled
  * properly, i.e. make the service return suitable HTTP status codes.
+ *
  * @author georgweiss
  * Created 23 Nov 2018
  */
@@ -44,6 +45,9 @@ public abstract class BaseController {
 
     private final Logger logger = Logger.getLogger(BaseController.class.getName());
 
+    @Autowired
+    public long connectionTimeout;
+
     /**
      * Identity of the admin role
      */
@@ -59,7 +63,8 @@ public abstract class BaseController {
 
     /**
      * Intercepts {@link SnapshotNotFoundException} and triggers a {@link HttpStatus#NOT_FOUND}.
-     * @param req The servlet request
+     *
+     * @param req       The servlet request
      * @param exception The exception to intercept
      * @return A {@link ResponseEntity} carrying the underlying exception message.
      */
@@ -72,7 +77,8 @@ public ResponseEntity<String> handleSnapshotNotFoundException(HttpServletRequest
 
     /**
      * Intercepts {@link IllegalArgumentException} and triggers a {@link HttpStatus#BAD_REQUEST}.
-     * @param req The servlet request
+     *
+     * @param req       The servlet request
      * @param exception The exception to intercept
      * @return A {@link ResponseEntity} carrying the underlying exception message.
      */
@@ -85,7 +91,8 @@ public ResponseEntity<String> handleIllegalArgumentException(HttpServletRequest
 
     /**
      * Intercepts {@link NodeNotFoundException} and triggers a {@link HttpStatus#NOT_FOUND}.
-     * @param req The {@link HttpServlet} request
+     *
+     * @param req       The {@link HttpServlet} request
      * @param exception The exception to intercept
      * @return A {@link ResponseEntity} carrying the underlying exception message.
      */

services/save-and-restore/src/main/java/org/phoebus/service/saveandrestore/web/controllers/SnapshotRestoreController.java

@@ -23,6 +23,8 @@
 import org.phoebus.saveandrestore.util.SnapshotUtil;
 import org.phoebus.service.saveandrestore.persistence.dao.NodeDAO;
 import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.beans.factory.annotation.Value;
+import org.springframework.context.annotation.PropertySource;
 import org.springframework.web.bind.annotation.PostMapping;
 import org.springframework.web.bind.annotation.RequestBody;
 import org.springframework.web.bind.annotation.RequestParam;
@@ -50,7 +52,7 @@ public class SnapshotRestoreController extends BaseController {
     @PostMapping(value = "/restore/items", produces = JSON)
     public List<RestoreResult> restoreFromSnapshotItems(
             @RequestBody List<SnapshotItem> snapshotItems) {
-        return snapshotUtil.restore(snapshotItems);
+        return snapshotUtil.restore(snapshotItems, connectionTimeout);
     }
 
     @PostMapping(value = "/restore/node", produces = JSON)
@@ -59,7 +61,7 @@ public List<RestoreResult> restoreFromSnapshotNode(
         Node snapshotNode = nodeDAO.getNode(nodeId);
         LOG.log(Level.INFO, "Restore requested for snapshot '" + snapshotNode.getName() + "'");
         var snapshot = nodeDAO.getSnapshotData(nodeId);
-        return snapshotUtil.restore(snapshot.getSnapshotItems());
+        return snapshotUtil.restore(snapshot.getSnapshotItems(), connectionTimeout);
     }
 }
 

services/save-and-restore/src/main/java/org/phoebus/service/saveandrestore/web/controllers/TakeSnapshotController.java

@@ -54,7 +54,7 @@ public List<SnapshotItem> takeSnapshot(@PathVariable String configNodeId) {
         ConfigurationData configurationData = nodeDAO.getConfigurationData(configNodeId);
         List<SnapshotItem> snapshotItems;
         try {
-            snapshotItems = snapshotUtil.takeSnapshot(configurationData);
+            snapshotItems = snapshotUtil.takeSnapshot(configurationData, connectionTimeout);
         } catch (Exception e) {
             throw new RuntimeException(e);
         }
@@ -78,7 +78,7 @@ public List<SnapshotItem> takeSnapshot(@PathVariable String configNodeId) {
     public Snapshot takeSnapshotAndSave(@PathVariable String configNodeId,
                                         @RequestParam(name = "name", required = false) String snapshotName,
                                         @RequestParam(name = "comment", required = false) String comment) {
-       if (snapshotName != null) {
+        if (snapshotName != null) {
             String _snapshotName = snapshotName;
             List<Node> childNodes = nodeDAO.getChildNodes(configNodeId);
             if (childNodes.stream().anyMatch(n -> n.getName().equals(_snapshotName) &&