Follow-up to #96. Three unrelated items in the EMS extension, grouped because they are all small and touch the same handlers.
1. GOPACSHandler never closes its REST client
GOPACSHandler stores a ResteasyClient as a field (GOPACSHandler.java:126, assigned at :219) and undeploy() (:326-332) cancels the scheduled futures and undeploys the web service, but never closes it.
EmsOptimisationService.processAssetChange does stop-then-start on every UPDATE of an EmsGOPACSAsset, so each edit of the asset leaves the previous client and its connection pool allocated. The client has already made requests by then, so it holds real sockets.
This is the same defect fixed for DistroEnergyHandler in #96 (f878f18). The fix is the same shape:
public void undeploy() {
for (ScheduledFuture<?> scheduledFuture : scheduledFutureList) {
scheduledFuture.cancel(true);
}
scheduledFutureList.clear();
client.close();
webService.undeploy(getDeploymentName(contractedEAN));
}
Closing is safe with the shared executor. WebTargetBuilder.createClient(ExecutorService) goes through ResteasyClientBuilder.executorService(ExecutorService), which sets cleanupExecutor to false, so Container.EXECUTOR is untouched. GOPACSRedispatchHandler.stopPolling() already does this.
Worth checking the same constructor window #96 closed: anything between createClient and the end of the constructor that can throw leaks a client, because no reference escapes a constructor that threw and undeploy() can never run. In GOPACSHandler the client.target(...) calls at :222-224 are the candidates.
2. General improvements
lastSubmission (TIMESTAMP, READ_ONLY) so a stalled handler is visible without reading logs
daysSubmitted (NUMBER, READ_ONLY) which is the forecast horizon in days, currently only a FINE log line
Constructor-publishes-this pattern. #96 fixed this in DistroEnergyHandler by moving scheduling into a deploy() method called after construction (afc6d9f). GOPACSHandler schedules from its constructor too and is worth the same treatment, which also makes it constructible in a test without an executor.
GOPACSHandler has no Level.WARNING budget discipline. Not urgent, but the DST-collapse warning added in #96 fires per position, so on the fall-back day it can emit several hundred lines per portfolio. Root cause is openremote/openremote#3292 upstream; a rate limit or a once-per-day summary would make it readable in the meantime.
Follow-up to #96. Three unrelated items in the EMS extension, grouped because they are all small and touch the same handlers.
1.
GOPACSHandlernever closes its REST clientGOPACSHandlerstores aResteasyClientas a field (GOPACSHandler.java:126, assigned at:219) andundeploy()(:326-332) cancels the scheduled futures and undeploys the web service, but never closes it.EmsOptimisationService.processAssetChangedoes stop-then-start on everyUPDATEof anEmsGOPACSAsset, so each edit of the asset leaves the previous client and its connection pool allocated. The client has already made requests by then, so it holds real sockets.This is the same defect fixed for
DistroEnergyHandlerin #96 (f878f18). The fix is the same shape:Closing is safe with the shared executor.
WebTargetBuilder.createClient(ExecutorService)goes throughResteasyClientBuilder.executorService(ExecutorService), which setscleanupExecutorto false, soContainer.EXECUTORis untouched.GOPACSRedispatchHandler.stopPolling()already does this.Worth checking the same constructor window #96 closed: anything between
createClientand the end of the constructor that can throw leaks a client, because no reference escapes a constructor that threw andundeploy()can never run. InGOPACSHandlertheclient.target(...)calls at:222-224are the candidates.2. General improvements
lastSubmission(TIMESTAMP,READ_ONLY) so a stalled handler is visible without reading logsdaysSubmitted(NUMBER,READ_ONLY) which is the forecast horizon in days, currently only a FINE log lineConstructor-publishes-
thispattern. #96 fixed this inDistroEnergyHandlerby moving scheduling into adeploy()method called after construction (afc6d9f).GOPACSHandlerschedules from its constructor too and is worth the same treatment, which also makes it constructible in a test without an executor.GOPACSHandlerhas noLevel.WARNINGbudget discipline. Not urgent, but the DST-collapse warning added in #96 fires per position, so on the fall-back day it can emit several hundred lines per portfolio. Root cause is openremote/openremote#3292 upstream; a rate limit or a once-per-day summary would make it readable in the meantime.