apache / apache/iotdb-client-nodejs

Session pool follow-ups: minPoolSize: 0, pending waiters on close(), and a failed idle-session close

Ouverte
#22 0 commentaires 0 réactions 0 personnes assignées Voir sur GitHub
Langage dominant
TypeScript
Étoiles
3
Forks
2
Merge moyen
7 min
PR mergées (30 j)
2

Description

## Summary

Three non-blocking observations about `BaseSessionPool` that came out of the reviews of #19 and #20. None of them is a crash or a leak on the normal path, so I did not fold them into those PRs; @CritasWang suggested collecting them in a single tracking issue.

Line references are against `develop` at a8ca4d2.

## 1. `minPoolSize: 0` is silently coerced to 1

`BaseSessionPool.ts:129` (and the cleanup floor at `:373`) read the setting as:

```ts
const minSize = this.config.minPoolSize || 1;
```

`0 || 1` is `1`, so a caller who explicitly asks for `minPoolSize: 0` gets one eagerly created session at `init()`, and `cleanupIdleSessions` then keeps one session alive forever rather than letting the pool drain to empty. In most pool implementations `0` means "create nothing up front, and allow the pool to go back to empty when idle", which is a reasonable ask for short-lived or cost-sensitive processes.

If `0` is intended to be legal, `??` instead of `||` would express it. If it is not intended to be legal, rejecting it in config validation would be clearer than silently changing it.

## 2. `close()` clears the wait queue without settling the waiters

`close()` at `:505` ends with `this.waitQueue.clear()` (`:525`). The queued entries are the waiter callbacks themselves, so clearing the queue drops them without ever resolving or rejecting their promises.

A caller that is inside `await pool.getSession()` when another part of the application closes the pool therefore does not fail fast. It stays pending until the acquire timeout fires (`waitTimeout`, default 60000ms at `:290`) and then rejects with `Timeout waiting for available session` (`:320`) — which points at pool exhaustion rather than at the real cause. Rejecting each queued waiter during `close()` with a "pool is closed" error would make both the latency and the message correct.

## 3. A failed `session.close()` during idle cleanup orphans the connection

`cleanupIdleSessions` (`:391` onward) deliberately removes a session from `pool` and `idleSessions` *before* awaiting `close()`, which is correct — it prevents a concurrent `getSession()` from handing out a session that is about to be destroyed. But if `close()` rejects, the error is logged (`:409`) and nothing else happens: the session is already out of every pool structure, so its underlying connection is no longer referenced, never retried, and never counted anywhere.

That is not a leak in the common case — the RPC usually fails because the connection is already gone — but a transport-level failure would leave a real socket open with no owner. Keeping the session in a small "failed to close" set for a later retry, or at least counting it in a metric, would make the failure visible.

## Not included

The `getSession()` create-branch race that also came up in the #19 review is already fixed on `develop` (`:266-276` now locates the session by identity instead of `shift()`ing the front of the idle queue), so there is nothing left to track there.

I am happy to send a PR for any of these — please let me know which are worth changing and whether you would prefer them separately.

Guide de contribution

Ouvrir le guide de contribution

Piste de recherche

Commencez dans BaseSessionPool.ts, au niveau de la gestion de minPoolSize autour des lignes 129 et 373, de close() autour de la ligne 505 et de cleanupIdleSessions autour de la ligne 391. Examinez le flux existant de acquire et de cleanup, puis ajoutez une couverture ciblée pour les pools de taille nulle, les waiters en attente pendant close() et les fermetures échouées de idle sessions. Le travail est terminé lorsque chaque comportement a un résultat explicite et qu’aucun waiter ni aucune fermeture échouée n’est abandonné silencieusement.

Rédigé par le modèle d'indexation à partir du texte de l'issue.

Évaluation

Stack technique
nodejs, typescript
Domaine
backend, databases
Type d'issue
Bug
Difficulté
4/5
Temps estimé
3-5 jours
Activité
Calme
Clarté
Plutôt claire
Accessibilité débutants
48/100

Recevez les nouvelles issues par e-mail

Un résumé court des issues GitHub adaptées aux débutants.