Skip to content

Commit

Permalink
fix(mtx): sidecar scenario due to usage of wrong credentials (#732)
Browse files Browse the repository at this point in the history
We currently have a memory leak in `@cap-js/hana` that doesn't occur
with `@sap/cds-hana`:

```sh
# In cap/dev:
cf login --sso
cf create-service service-manager container bookshop-sm
cds init bookshop --add multitenancy
cd bookshop/mtx/sidecar
cds bind -2 bookshop-sm --for hana
cds run --profile hana --resolve-bindings
BOOM 💣 → memory leak
```

To get heap snapshots run this instead of plain `cds run`:
```js
NODE_OPTIONS='--heapsnapshot-near-heap-limit=3 --max-old-space-size=100' cds run --profile hana --resolve-bindings
```

Turns out the memory is full of type errors from `hdb`:
<img width="1079" alt="Screenshot 2024-07-08 at 12 29 00"
src="https://github.com/cap-js/cds-dbs/assets/24377039/f6dff878-8678-4959-abe4-195f9aca919b">

From the error I could tell it tried to open the `hdb` connection with
Service Manger credentials instead of the actual credentials fetched
from it.

The problem is that in a sidecar scenario we don't explicitly set
`cds.requires.multitenancy` (might change, see also internally
cap/cds-mtxs/pull/1006), so `@cap-js/hana` takes a wrong turn.

I also found out the
[`create`](https://github.com/cap-js/cds-dbs/blob/0b9108c11a61b18704e36f93fbd654e0942bf40a/hana/lib/HANAService.js#L55)
function here is called extremely often in succession in the error case,
which explains why the memory is filled up so quickly.

This PR fixes the issue for our sidecar setup and older MTX versions (if
we also decide to go with cap/cds-mtxs/pull/1006) but we should still
investigate in a follow-up how the behavior can be improved so the
`create` isn't called so often without debouncing and filling up the
memory in the error case.

---------

Co-authored-by: Patrice Bender <patrice.bender@sap.com>
  • Loading branch information
swaldmann and patricebender authored Jul 8, 2024
1 parent 77b7978 commit 0b5c91f
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion hana/lib/HANAService.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ class HANAService extends SQLService {
// REVISIT: Add multi tenant factory when clarified
get factory() {
const driver = drivers[this.options.driver || this.options.credentials?.driver]?.driver || drivers.default.driver
const isMultitenant = 'multiTenant' in this.options ? this.options.multiTenant : cds.env.requires.multitenancy
const service = this
const isMultitenant = !!service.options.credentials.sm_url || ('multiTenant' in this.options ? this.options.multiTenant : cds.env.requires.multitenancy)
const acquireTimeoutMillis = this.options.pool?.acquireTimeoutMillis || cds.env.profiles.includes('production') ? 1000 : 10000
return {
options: {
Expand Down

0 comments on commit 0b5c91f

Please sign in to comment.