SAP / node-hdb

SAP HANA Database Client for Node
Apache License 2.0
317 stars 97 forks source link

Fix discarded enqueued queries #216

Closed BobdenOs closed 1 year ago

BobdenOs commented 1 year ago

Description

Currently it is possible to have Tasks which are queued while the database connection is in the process of being disconnected from the database. As the running Task is being rejected by the onend event it will trigger the next Task. This task is removed from the queue and send to the connection. Resulting in the connection becoming dormant and the started Task hanging. Additionally all queued Tasks are completely discarded when onclose is called. Which makes all the enqueued Tasks hang.

Visual examples

The current behavior:

sequenceDiagram
participant Connection
participant Queue
participant HANA

Connection->>Queue: Task 1
Connection->>Queue: Task 2
Queue->>Connection: run 1
Connection->>HANA: Task 1
HANA-->>Connection: Disconnect
Connection->>Queue: Task 1 (Error)
Queue->>Queue: next
Queue->>Connection: run 2
Connection->>HANA: Task 2
Connection->>Connection: Update Socket status

The behavior of the PR:

sequenceDiagram
participant Connection
participant Queue
participant HANA

Connection->>Queue: Task 1
Connection->>Queue: Task 2
Queue->>Connection: run 1
Connection->>HANA: Task 1
HANA-->>Connection: Disconnect
Connection->>Queue: Abort (Error)
Queue->>Queue: Task 2 (Error)
Connection->>Queue: Task 1 (Error)
Connection->>Connection: Update Socket Status

Alternative solution would be to call run in the next of the queue with setImmediate, but this might come with an performance impact for positive connection scenarios.

sequenceDiagram
participant Connection
participant Queue
participant HANA

Connection->>Queue: Task 1
Connection->>Queue: Task 2
Queue->>Connection: run 1
Connection->>HANA: Task 1
HANA-->>Connection: Disconnect
Connection->>Queue: Task 1 (Error)
Queue->>Queue: next (setImmediate)
Connection->>Connection: Update Socket Status
Queue->>Connection: run 2
Connection->>Queue: Task 2 (Error)
cla-assistant[bot] commented 1 year ago

CLA assistant check
All committers have signed the CLA.