diff options
author | jonmv <venstad@gmail.com> | 2022-11-14 11:47:15 +0100 |
---|---|---|
committer | jonmv <venstad@gmail.com> | 2022-11-14 11:47:15 +0100 |
commit | 2a8b661f439195d6f9fcd055f026a44c2ba1aaec (patch) | |
tree | bcfacb35c121251be4ad92d6b1c3bd5cb56e0baf /zkfacade | |
parent | 315636db2c7bccd7ae48ce698105c38f1b7d0acc (diff) |
Improve exception message grammar, javadoc
Diffstat (limited to 'zkfacade')
3 files changed, 30 insertions, 29 deletions
diff --git a/zkfacade/src/main/java/com/yahoo/vespa/curator/CuratorWrapper.java b/zkfacade/src/main/java/com/yahoo/vespa/curator/CuratorWrapper.java index 27d969c0c09..273c0be044b 100644 --- a/zkfacade/src/main/java/com/yahoo/vespa/curator/CuratorWrapper.java +++ b/zkfacade/src/main/java/com/yahoo/vespa/curator/CuratorWrapper.java @@ -110,7 +110,7 @@ public class CuratorWrapper extends AbstractComponent implements VespaCurator { @Override public void register(SingletonWorker singleton, Duration timeout) { try { - await(singletons.register(singleton.id(), singleton), timeout, "register " + singleton); + await(singletons.register(singleton.id(), singleton), timeout, "registering " + singleton); } catch (RuntimeException e) { try { @@ -125,7 +125,7 @@ public class CuratorWrapper extends AbstractComponent implements VespaCurator { @Override public void unregister(SingletonWorker singleton, Duration timeout) { - await(singletons.unregister(singleton), timeout, "unregister " + singleton); + await(singletons.unregister(singleton), timeout, "unregistering " + singleton); } private void await(Future<?> future, Duration timeout, String action) { @@ -141,7 +141,7 @@ public class CuratorWrapper extends AbstractComponent implements VespaCurator { throw new UncheckedTimeoutException("timed out while " + action, e); } catch (ExecutionException e) { - throw new RuntimeException("failed to " + action, e.getCause()); + throw new RuntimeException("failed " + action, e.getCause()); } } diff --git a/zkfacade/src/main/java/com/yahoo/vespa/curator/api/VespaCurator.java b/zkfacade/src/main/java/com/yahoo/vespa/curator/api/VespaCurator.java index f2bc38a4644..f7bfc63e30a 100644 --- a/zkfacade/src/main/java/com/yahoo/vespa/curator/api/VespaCurator.java +++ b/zkfacade/src/main/java/com/yahoo/vespa/curator/api/VespaCurator.java @@ -98,6 +98,29 @@ public interface VespaCurator { * containers in the cluster, and across all component generations. * <p> * <br> + * Notes to implementors: + * <ul> + * <li>{@link #activate()} is called by the system on a singleton whenever it is the newest registered + * singleton in this container, and this container has the lease for the ID with which the singleton + * was registered. See {@link #id}, {@link #register} and {@link #isActive}.</li> + * <li>{@link #deactivate()} is called by the system on a singleton which is currently active whenever + * the above no longer holds. See {@link #unregister}.</li> + * <li>Callbacks for the same ID are always invoked by the same thread, in serial; <strong>the callbacks must + * return in a timely manner</strong>, but are encouraged to throw exceptions when something's wrong.</li> + * <li>Activation and deactivation may be triggered by: + * <ol> + * <li>the container acquiring or losing the activation lease; or</li> + * <li>registration of unregistration of a new or obsolete singleton.</li> + * </ol> + * Events triggered by the latter happen synchronously, and errors are propagated to the caller for cleanup. + * Events triggered by the former may happen in the background, and because the system tries to always have + * one activated singleton, exceptions during activation will cause the container to abandon its lease, so + * another container may obtain it instead; exceptions during deactivation are only logged. + * </li> + * <li>A container without any registered singletons will not attempt to hold the activation lease.</li> + * </ul> + * <p> + * <br> * Sample usage: * <pre> * public class SingletonHolder extends AbstractComponent { @@ -129,7 +152,8 @@ public interface VespaCurator { * * @Override * public void activate() { - * resource.open(); // Verify resource works here, and propagate any errors out. + * try { resource.open(5, TimeUnit.SECONDS); } // Verify resource works here, and propagate any errors out. + * catch {Exception e) { resource.close(); throw new RuntimeException("failed opening " + resource, e); } * running.set(true); * future = executor.submit(this::doWork); * } @@ -137,7 +161,7 @@ public interface VespaCurator { * @Override * public void deactivate() { * running.set(false); - * try { future.get(10, TimeUnit.SECONDS); } + * try { future.get(5, TimeUnit.SECONDS); } * catch (Exception e) { ... } * finally { resource.close(); } * } @@ -152,29 +176,6 @@ public interface VespaCurator { * * } * </pre> - * <p> - * <br> - * Notes to implementors: - * <ul> - * <li>{@link #activate()} is called by the system on a singleton whenever it is the newest registered - * singleton in this container, and this container has the lease for the ID with which the singleton - * was registered. See {@link #id}, {@link #register} and {@link #isActive}.</li> - * <li>{@link #deactivate()} is called by the system on a singleton which is currently active whenever - * the above no longer holds. See {@link #unregister}.</li> - * <li>Callbacks for the same ID are always invoked by the same thread, in serial; the callbacks must - * return in a timely manner, but are encouraged to throw exceptions when something's wrong</li> - * <li>Activation and deactivation may be triggered by: - * <ol> - * <li>the container acquiring or losing the activation lease; or</li> - * <li>registration of unregistration of a new or obsolete singleton.</li> - * </ol> - * Events triggered by the latter happen synchronously, and errors are propagated to the caller for cleanup. - * Events triggered by the former may happen in the background, and because the system tries to always have - * one activated singleton, exceptions during activation will cause the container to abandon its lease, so - * another container may obtain it instead; exceptions during deactivation are only logged. - * </li> - * <li>A container without any registered singletons will not attempt to hold the activation lease.</li> - * </ul> */ interface SingletonWorker { diff --git a/zkfacade/src/test/java/com/yahoo/vespa/curator/CuratorWrapperTest.java b/zkfacade/src/test/java/com/yahoo/vespa/curator/CuratorWrapperTest.java index 227cc562211..a788111faff 100644 --- a/zkfacade/src/test/java/com/yahoo/vespa/curator/CuratorWrapperTest.java +++ b/zkfacade/src/test/java/com/yahoo/vespa/curator/CuratorWrapperTest.java @@ -216,7 +216,7 @@ public class CuratorWrapperTest { stunning.arriveAndAwaitAdvance(); // Failing component is done being deactivated. stunning.arriveAndAwaitAdvance(); // Failing component is done cleaning up after itself. assertTrue(newSingleton.isActive); - assertEquals("failed to register failing singleton", thrownMessage.get()); + assertEquals("failed registering failing singleton", thrownMessage.get()); verifyMetrics(Map.of("activation.count", 6.0, "activation.millis", 0.0, "activation.failure.count", 1.0, |