From d76b238f05f2b6860ab5685bfbea9f5b433ccdc1 Mon Sep 17 00:00:00 2001 From: georgweiss Date: Thu, 29 Oct 2026 15:15:59 +0100 Subject: [PATCH 1/2] Add preference to choose snapshot restore policy: sync or async --- .../saveandrestore/util/Preferences.java | 6 ++ .../saveandrestore/util/RestorePolicy.java | 16 +++ .../saveandrestore/util/SnapshotUtil.java | 98 +++++++++++-------- ...ve_and_restore_util_preferences.properties | 6 ++ 4 files changed, 87 insertions(+), 39 deletions(-) create mode 100644 app/save-and-restore/util/src/main/java/org/phoebus/saveandrestore/util/RestorePolicy.java diff --git a/app/save-and-restore/util/src/main/java/org/phoebus/saveandrestore/util/Preferences.java b/app/save-and-restore/util/src/main/java/org/phoebus/saveandrestore/util/Preferences.java index 2e0e7e6bed..4d25e04f38 100644 --- a/app/save-and-restore/util/src/main/java/org/phoebus/saveandrestore/util/Preferences.java +++ b/app/save-and-restore/util/src/main/java/org/phoebus/saveandrestore/util/Preferences.java @@ -39,6 +39,12 @@ public class Preferences { @Preference public static int writeTimeout; + /** + * Restore policy: see + */ + @Preference + public static RestorePolicy restorePolicy; + static { AnnotatedPreferences.initialize(Preferences.class, "/save_and_restore_util_preferences.properties"); } diff --git a/app/save-and-restore/util/src/main/java/org/phoebus/saveandrestore/util/RestorePolicy.java b/app/save-and-restore/util/src/main/java/org/phoebus/saveandrestore/util/RestorePolicy.java new file mode 100644 index 0000000000..54ff1b57cf --- /dev/null +++ b/app/save-and-restore/util/src/main/java/org/phoebus/saveandrestore/util/RestorePolicy.java @@ -0,0 +1,16 @@ +/* + * Copyright (C) 2026 European Spallation Source ERIC. + * + */ + +package org.phoebus.saveandrestore.util; + +/** + * Determines if a restore operation should be performed in a threaded/asynchronous manner, + * or one PV at a time. With channel access (ca) asynchronous writes may trigger a deadlock in + * JCA, wich can be worked around with synchronous write operations. + */ +public enum RestorePolicy { + ASYNCHRONOUS, + SYNCHRONOUS, +} diff --git a/app/save-and-restore/util/src/main/java/org/phoebus/saveandrestore/util/SnapshotUtil.java b/app/save-and-restore/util/src/main/java/org/phoebus/saveandrestore/util/SnapshotUtil.java index f46179de0e..e6e01c8d53 100644 --- a/app/save-and-restore/util/src/main/java/org/phoebus/saveandrestore/util/SnapshotUtil.java +++ b/app/save-and-restore/util/src/main/java/org/phoebus/saveandrestore/util/SnapshotUtil.java @@ -89,20 +89,31 @@ public synchronized List restore(List snapshotItems List cleanedSnapshotItems = cleanSnapshotItems(snapshotItems); List restoreResultList = new ArrayList<>(); - List callables = new ArrayList<>(); - for (SnapshotItem si : cleanedSnapshotItems) { - RestoreCallable restoreCallable = new RestoreCallable(si, restoreResultList, connectionTimeout); - callables.add(restoreCallable); - } + if(Preferences.restorePolicy.equals(RestorePolicy.ASYNCHRONOUS)) { + List callables = new ArrayList<>(); + for (SnapshotItem si : cleanedSnapshotItems) { + RestoreCallable restoreCallable = new RestoreCallable(si, restoreResultList, connectionTimeout); + callables.add(restoreCallable); + } - try { - executorService.invokeAll(callables); - } catch (InterruptedException e) { - LOG.log(Level.WARNING, "Got exception waiting for all tasks to finish", e); - // Return empty list here? - return Collections.emptyList(); - } finally { - callables.forEach(RestoreCallable::release); + try { + executorService.invokeAll(callables); + } catch (InterruptedException e) { + LOG.log(Level.WARNING, "Got exception waiting for all tasks to finish", e); + // Return empty list here? + return Collections.emptyList(); + } + } + else if(Preferences.restorePolicy.equals(RestorePolicy.SYNCHRONOUS)) { + for (SnapshotItem si : cleanedSnapshotItems) { + Optional restoreResultOptional = writePV(si, connectionTimeout); + if(restoreResultOptional.isPresent()) { + restoreResultList.add(restoreResultOptional.get()); + } + } + } + else{ + LOG.log(Level.WARNING, "Unsupported restore policy: " + Preferences.restorePolicy); } return restoreResultList; @@ -273,7 +284,6 @@ private List cleanSnapshotItems(List snapshotItems) private class RestoreCallable implements Callable { private final List restoreResultList; - private PV pv; private final SnapshotItem snapshotItem; private final long connectionTimeout; @@ -285,40 +295,50 @@ public RestoreCallable(SnapshotItem snapshotItem, List restoreRes @Override public Void call() { - CountDownLatch countDownLatch = new CountDownLatch(1); - try { - pv = PVPool.getPV(snapshotItem.getConfigPv().getPvName()); - pv.onValueEvent().subscribe(value -> { - if (!PV.isDisconnected(value)) { - countDownLatch.countDown(); - } - }); - if (!countDownLatch.await(connectionTimeout, TimeUnit.MILLISECONDS)) { - LOG.log(Level.WARNING, "Connection to PV '" + snapshotItem.getConfigPv().getPvName() + - "' timed out after " + connectionTimeout + "ms."); - RestoreResult restoreResult = new RestoreResult(); - restoreResult.setSnapshotItem(snapshotItem); - restoreResult.setErrorMsg("No monitor event from PV '" + snapshotItem.getConfigPv().getPvName() + "'"); - restoreResultList.add(restoreResult); - } else { - pv.write(VTypeHelper.toObject(snapshotItem.getValue())); + Optional restoreResultOptional = writePV(snapshotItem, connectionTimeout); + if(restoreResultOptional.isPresent()) { + restoreResultList.add(restoreResultOptional.get()); + } + return null; + } + } + + private Optional writePV(SnapshotItem snapshotItem, long connectionTimeout) { + CountDownLatch countDownLatch = new CountDownLatch(1); + Optional restoreResultOptional = Optional.empty(); + PV pv = null; + try { + pv = PVPool.getPV(snapshotItem.getConfigPv().getPvName()); + pv.onValueEvent().subscribe(value -> { + if (!PV.isDisconnected(value)) { + countDownLatch.countDown(); } - } catch (Exception e) { - LOG.log(Level.WARNING, "Failed to write to PV '" + snapshotItem.getConfigPv().getPvName() + "'", e); + }); + if (!countDownLatch.await(connectionTimeout, TimeUnit.MILLISECONDS)) { + LOG.log(Level.WARNING, "Connection to PV '" + snapshotItem.getConfigPv().getPvName() + + "' timed out after " + connectionTimeout + "ms."); RestoreResult restoreResult = new RestoreResult(); restoreResult.setSnapshotItem(snapshotItem); - restoreResult.setErrorMsg("Failed to write to PV '" + snapshotItem.getConfigPv().getPvName() + "', cause: " + e.getMessage()); - restoreResultList.add(restoreResult); - countDownLatch.countDown(); + restoreResult.setErrorMsg("No monitor event from PV '" + snapshotItem.getConfigPv().getPvName() + "'"); + restoreResultOptional = Optional.of(restoreResult); + } else { + pv.write(VTypeHelper.toObject(snapshotItem.getValue())); } - return null; + } catch (Exception e) { + LOG.log(Level.WARNING, "Failed to write to PV '" + snapshotItem.getConfigPv().getPvName() + "'", e); + RestoreResult restoreResult = new RestoreResult(); + restoreResult.setSnapshotItem(snapshotItem); + restoreResult.setErrorMsg("Failed to write to PV '" + snapshotItem.getConfigPv().getPvName() + "', cause: " + e.getMessage()); + restoreResultOptional = Optional.of(restoreResult); + countDownLatch.countDown(); } - - public void release() { + finally { if (pv != null) { PVPool.releasePV(pv); } } + + return restoreResultOptional; } /** diff --git a/app/save-and-restore/util/src/main/resources/save_and_restore_util_preferences.properties b/app/save-and-restore/util/src/main/resources/save_and_restore_util_preferences.properties index 195e24cf11..6185c3776c 100644 --- a/app/save-and-restore/util/src/main/resources/save_and_restore_util_preferences.properties +++ b/app/save-and-restore/util/src/main/resources/save_and_restore_util_preferences.properties @@ -7,3 +7,9 @@ connectionTimeout=5000 # Write timeout, in milliseconds, when restoring snapshot. writeTimeout=5000 + +# Restore policy: ASYNCHRONOUS (parallel PV writes) or SYNCHRONOUS +# (write one PV at a time). With channel access (ca), parallel writes +# may trigger deadlocks in the JCA library. SYNCHRONOUS policy +# is then the workaround to consider. +restorePolicy=SYNCHRONOUS From f6aa218c27d5aafd52a12e7be793c8f099bb0be2 Mon Sep 17 00:00:00 2001 From: georgweiss Date: Thu, 29 Oct 2026 15:29:37 +0100 Subject: [PATCH 2/2] Update doc to explain restore policy setting --- app/save-and-restore/app/doc/index.rst | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/app/save-and-restore/app/doc/index.rst b/app/save-and-restore/app/doc/index.rst index 09c6674841..6af024597a 100644 --- a/app/save-and-restore/app/doc/index.rst +++ b/app/save-and-restore/app/doc/index.rst @@ -344,8 +344,10 @@ To restore the values in the snapshot user should click the Restore button. Duri disabled. In case a write operation fails on a PV, the process is *not* aborted, and user will be presented with a list of failed PVs when restore operation completes. -**NOTE:** During the restore operation PVs are written in parallel, i.e. in an asynchronous manner. The order of -write operations is hence undefined. +**NOTE:** During the restore operation PVs are by default written in parallel, i.e. in an asynchronous manner. The order of +write operations is hence undefined. If synchronous write operations of PVs are needed (e.g. when using channel access), +the client or server or both should set the preference ``org.phoebus.saveandrestore.util/restorePolicy`` should be set +to ``SYNCHRONOUS``. Prior to restore user has the option to: