bamaer commented on PR #8282:
URL: https://github.com/apache/hop/pull/8282#issuecomment-5584741879
One regression, and one pre-existing bug worth fixing while this code is
being touched.
**1. Nothing indexes `:ExecutionMetric`**
`updateNeo4jExecutionState` merges on `:ExecutionMetric {id, name, copyNr,
metricKey}` and then
matches the same node for the `HAS_METRIC` relationship. No index or
constraint is created on that
label anywhere — the DDL emitted by this class indexes `EL_EXECUTION` and
omits `metricKey`:
```java
// NeoExecutionInfoLocation:288 and :374
addIndex(cypher, "idx_execution_metric_id", EL_EXECUTION, EP_ID, EP_NAME,
EP_COPY_NR);
```
Both statements therefore plan as a `NodeByLabelScan`. To be clear, that
predates this PR — the
relationship `withMatch` already scanned — and CREATE→MERGE is a net
improvement, since CREATE
appended a duplicate node per metric per tick and grew the label without
bound. But MERGE takes the
scan count from one to two per metric per tick, and the DDL is advisory: if
the label isn't
corrected, nobody gets the index. Since this PR already edits that list
(`idx_execution_start_date`),
suggest `CL_EXECUTION_METRIC` plus `CP_METRIC_KEY` in both places.
**2. `writeHierarchies` logs and throws, leaking a session and driver**
In both `PipelineLoggingExtensionPoint` and `WorkflowLoggingExtensionPoint`,
the session and driver
are closed after `logHierarchy(...)` inside the finished-listener lambda,
with no `finally` — and the
outer `catch (Exception e)` covers only the registration, not the lambda
body. So the added `throw`
means `Driver.close()` never runs on the error path, leaking a connection
pool and its threads for
the life of the JVM.
It also escapes the listener loop:
`Workflow.fireExecutionFinishedListeners()` has no try/catch,
and `Pipeline.fireExecutionFinishedListeners()` collects `HopException`
only, which
`HopRuntimeException` is not.
Dropping the `throw` keeps the added `logError` and restores the previous
behaviour; a `finally`
around the closes in both extension points works too.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
