Skip to content

EMS: close the GOPACS REST client, populate the unwritten redispatch bid attributes #97

Description

@Miggets7

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. Two redispatch attributes are declared and displayed but never written

The Redispatch — Bid panel in asset-types.json shows three attributes. Two of them can never hold a value:

Attribute Declared Written Result
redispatchSuggestedPower EmsGOPACSAsset.java:153, READ_ONLY nowhere always null
redispatchSuggestedVolume EmsGOPACSAsset.java:163, READ_ONLY nowhere always null
redispatchBidPrice EmsGOPACSAsset.java:168 nowhere operator must enter it blind

Both suggested values are marked READ_ONLY, so the operator cannot fill them either. The panel renders two permanently empty fields.

The section comment above them says // --- Redispatch bid attributes (auto-calculated, operator-overridable) ---. The auto-calculation was never implemented. redispatchBidPrice is read at GOPACSRedispatchHandler.java:570 when placing a bid, so the intent appears to have been that the handler pre-fills a suggested price and power from the announcement, and the operator overrides if they want.

redispatchSuggestedPower also carries STORE_DATA_POINTS, HAS_PREDICTED_DATA_POINTS and DATA_POINTS_MAX_AGE_DAYS=7, so it is set up for charting that will never have data.

Decide which way to go and do it:

  • implement the calculation and write all three on each announcement, next to the existing writes in processAnnouncement (GOPACSRedispatchHandler.java:424-456), or
  • drop the two suggested attributes and remove them from the panel.

Leaving them declared and displayed but unpopulated is the one option that should not stay.

3. General improvements

Audit every declared attribute for a writer. The two above were found by diffing the AttributeDescriptor constants in EmsGOPACSAsset against the sendAttributeEvent call sites. Worth doing the same sweep across the other EMS asset types, and worth a test that fails when a READ_ONLY attribute has no writer, since that combination is always a bug.

EmsDistroEnergyAsset has no observability at all. It declares only portfolio, and its panel config shows only that. After #96 the handler decides per run how many market days to submit and where the forecast horizon ends, and none of that is visible on the asset. redispatchLastPoll is the precedent worth copying:

  • 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.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

BugError or issue in the system

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions