socket.io icon indicating copy to clipboard operation
socket.io copied to clipboard

[adapter]: Add option to fetchSockets that returns results even if some nodes didn't respond

Open timsauvageot opened this issue 1 year ago • 1 comments

Is your feature request related to a problem? Please describe. Hey there, the current implementation of fetchSockets in ClusterAdapterWithHeartbeat only resolves if a response was received from all cluster nodes. In some situations this isn't needed and an optimistic response would suffice (i.e. return all responses of nodes that are alive). Nodes that go down (based on logic inside of cleanupTimer) are also not removed from the missingUids list of any pending requests (customRequests). So even when it's detected that a node is down and therefore can't return a response, an error is still thrown.

Describe the solution you'd like

  • Add optional flag to fetchSockets which causes the function to resolve with the values of all nodes that answered.
  • remove id of nodes that were detected as not alive in cleanUp timer from missingUids of customRequests

Describe alternatives you've considered An optional flag could also be set on ClusterAdapterOptions however the config would then need to be added as union type for existing adapters (e.g. change createAdapter(pool: Pool, opts: Partial<PostgresAdapterOptions> = {}) to createAdapter(pool: Pool, opts: Partial<PostgresAdapterOptions & ClusterAdapterOptions> = {}) otherwise the value cannot be set.

Additional context I already opened an issue in the adapter repository but didn't get a response there. I'd be happy to implement the changes myself if there aren't any concerns.

timsauvageot avatar Apr 17 '24 16:04 timsauvageot

Hi!

With emitWithAck(), we attach the incomplete response array to the error, do you think it would make sense to do the same with fetchSockets()?

https://github.com/socketio/socket.io/blob/239a2a82d0371867d37537f4a5df831aeb18653c/lib/broadcast-operator.ts#L307-L317

remove id of nodes that were detected as not alive in cleanUp timer from missingUids of customRequests

I think this was implemented in https://github.com/socketio/socket.io-adapter/commit/0e23ff0cc671e3186510f7cfb8a4c1147457296f, included in [email protected]. Is that what you had in mind?

darrachequesne avatar Apr 26 '24 13:04 darrachequesne