PDGGK opened a new pull request, #11750:
URL: https://github.com/apache/seatunnel/pull/11750
## Purpose
Close the Neo4j session and driver even when `close()` fails part way
through.
Fixes #11749
## Brief Change Log
**`Neo4jSinkWriter.close()`**
```java
flushWriteBuffer();
session.close();
driver.close();
```
`flushWriteBuffer()` → `writeByQuery()` deliberately converts a
`Neo4jException` into a `Neo4jConnectorException` and rethrows it (`:129-132`).
So whenever the final batch cannot be written — the database going away during
teardown is the ordinary case — neither close runs and the whole `Driver` is
leaked, along with its connection pool and Netty event-loop group. Wrapped in
try/finally, with the driver close nested so a failing session close cannot
skip it either.
**`Neo4jSourceReader.close()`** had the same shape without the flush; a
session that fails to close skipped the driver. Same treatment, three lines.
I included the source reader deliberately rather than leaving it for a
follow-up: this PR's whole argument is that these have to be released on the
exception path, and the identical bug forty lines away in the same connector
would be odd to leave behind.
## Testing
New `Neo4jCloseTest`, plain Mockito, no container:
- `sinkWriterClosesSessionAndDriverWhenTheFinalFlushFails` — one buffered
row, `session.writeTransaction` stubbed to throw `ServiceUnavailableException`,
then `assertThrows(Neo4jConnectorException.class, writer::close)` and verify
both closes.
- `sourceReaderClosesDriverWhenTheSessionFailsToClose` — `session.close()`
stubbed to throw, verify `driver.close()` still runs.
On `dev` both fail with exactly the leak:
```
Neo4jCloseTest.sinkWriterClosesSessionAndDriverWhenTheFinalFlushFails:80
Wanted but not invoked: session.close();
Neo4jCloseTest.sourceReaderClosesDriverWhenTheSessionFailsToClose:103
Wanted but not invoked: driver.close();
```
With the change: 2/2, and the `connector-neo4j` module is 4/4 including the
existing `Neo4jSourceReaderTest`. `spotless:check` on the module passes.
Note for anyone running this locally: the test JVM has to be JDK 17 or lower
— on JDK 22 Byte Buddy cannot mock `org.neo4j.driver.Driver` and the failure
looks unrelated to the patch.
## Severity
Honest read: this is low-to-medium. It needs a failure during teardown to
trigger, and the driver's pool is small. But it is a whole driver plus an
event-loop group per occurrence, it is silent, and the fix is a try/finally.
## Does this pull request potentially affect one of the following parts?
- [x] Connectors (source or sink)
--
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]
