Skip to content

Conversation

@agourlay
Copy link
Member

While running performance benchmarks I get the following warning.

(node:262939) MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 error listeners added to [ClientHttp2Session]. MaxListeners is 10. Use emitter.setMaxListeners() to increase limit
    at genericNodeError (node:internal/errors:983:15)
    at wrappedFn (node:internal/errors:537:14)
    at _addListener (node:events:593:17)
    at ClientHttp2Session.addListener (node:events:611:10)
    at file:///home/agourlay/Workspace/qdrant-rest-grpc-benchmark/node_modules/@bufbuild/connect-node/dist/esm/node-universal-client.js:160:24
    at process.processTicksAndRejections (node:internal/process/task_queues:105:5)

This memory leak has been fixed in connect-node 0.12.0

Overriding the dependency locally for my benchmarks removes the warning.

For this PR I decided to bump to 0.13.0 as it is the last version before the package migration.

@agourlay agourlay changed the title Bump connect 0.13.o to fix memory leak Bump connect 0.13.0 to fix memory leak Oct 24, 2025
@agourlay
Copy link
Member Author

Dear @IvanPleshkov, could you please help me with CI here, I have no idea what I am doing 🙃

@Apidcloud
Copy link

Apidcloud commented Oct 24, 2025

I think to fix the CI you can just install those new versions so that the corresponding .lock dependencies file gets updated.

e.g., npm install --save @bufbuild/[email protected] @bufbuild/[email protected]

and then for the dev dependency:
npm install -D @bufbuild/[email protected]

@IvanPleshkov
Copy link
Contributor

Dear @IvanPleshkov, could you please help me with CI here, I have no idea what I am doing 🙃

It seems you forgot to update .lock files. Call pnpm install to update it

@IvanPleshkov
Copy link
Contributor

Dear @IvanPleshkov, could you please help me with CI here, I have no idea what I am doing 🙃

Fixed. I have a question about these changes. v0.13 is obsolete too. Why did you choose this version, and how can I reproduce the problem?

packages/js-client-grpc                  |  WARN  deprecated @bufbuild/[email protected]
packages/js-client-grpc                  |  WARN  deprecated @bufbuild/[email protected]
packages/js-client-grpc                  |  WARN  deprecated @bufbuild/[email protected]

@IvanPleshkov
Copy link
Contributor

Dear @IvanPleshkov, could you please help me with CI here, I have no idea what I am doing 🙃

Fixed. I have a question about these changes. v0.13 is obsolete too. Why did you choose this version, and how can I reproduce the problem?

packages/js-client-grpc                  |  WARN  deprecated @bufbuild/[email protected]
packages/js-client-grpc                  |  WARN  deprecated @bufbuild/[email protected]
packages/js-client-grpc                  |  WARN  deprecated @bufbuild/[email protected]

It seems I got it. We cannot use v1 and later because of backward compatibility

@agourlay
Copy link
Member Author

@IvanPleshkov Thanks for fixing the branch 🙏

Migrating to [email protected] requires more work and was not necessary for the fix 👍

If you are still interested in the repro, the work was done using @Apidcloud repository from qdrant/qdrant#7366

 "overrides": {
    "@qdrant/js-client-grpc": {
      "@bufbuild/connect": "0.13.0",
      "@bufbuild/connect-node": "0.13.0"
    }
  }

Overriding the transitive dependencies locally worked out fine for me.

@agourlay agourlay merged commit 1f146b6 into master Oct 24, 2025
6 checks passed
@agourlay agourlay deleted the bump-connect-0.13-memory-leak branch October 24, 2025 14:50
@Apidcloud
Copy link

Apidcloud commented Oct 24, 2025

While I wait for the release of a new version or until I have a bit of time to try overriding the versions directly, I'm just curious on whether it solved the allocation issues you showed in the original issue and whether it lowered the memory usage or not.

On a separate note, just realised 0.13.0 is over 2 years old, with many versions in between. From some of the release notes of subsequent versions, I would say there's a decent chance the decoding/deserialization performance gets better. How much effort do you think it would take to update it (and protobuf) to the latest, @IvanPleshkov? And how can I help?

@agourlay
Copy link
Member Author

agourlay commented Nov 11, 2025

I'm just curious on whether it solved the allocation issues you showed in the original issue and whether it lowered the memory usage or not.

Bumping to 0.13.0 did not provide significant memory gain.
It only removed the aforementioned warning.

On a separate note, just realized 0.13.0 is over 2 years old, with many versions in between.

We got started internally on that update, please check #112 out where your JS expertise will be appreciated! 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants