Skip to content
This repository was archived by the owner on Mar 4, 2026. It is now read-only.

fix: pool.ts: add more detailed logging for client garbage collection#2420

Merged
dconeybe merged 4 commits intomainfrom
dconeybe/MoreClientPoolLogging
Sep 22, 2025
Merged

fix: pool.ts: add more detailed logging for client garbage collection#2420
dconeybe merged 4 commits intomainfrom
dconeybe/MoreClientPoolLogging

Conversation

@dconeybe
Copy link
Contributor

@dconeybe dconeybe commented Sep 19, 2025 •
edited
Loading

The motivation for this PR is to help investigate b/437011084, where ClientPool.acquire() apparently is returning previously-garbage-collected clients, resulting in "GoogleError: The client has already been closed". This is a follow-up PR to #2416, adding even more logging towards the investigation.

The expanded logging looks like this:

Creating a new client [cli84iJ0] (requiresGrpc: false)
Creating a new client [clipLmMe] (requiresGrpc: true)
Re-using existing client [clipLmMe] with 9 remaining operations
Releasing client [cli84iJ0] (gc={name=PoolIsTransitioningToGrpc, shouldGarbageCollectClient=true, clientActiveRequestCount=0, poolGrpcEnabled=true, clientGrpcEnabled=false})
Garbage collecting client [cli84iJ0] (2 active clients: {cli84iJ0, clipLmMe}, 0 failed clients: {})
Releasing client [clipLmMe] (gc={name=ClientHasActiveRequests, shouldGarbageCollectClient=false, clientActiveRequestCount=1})
Releasing client [clipLmMe] (gc={name=IdleCapacity, shouldGarbageCollectClient=false, clientActiveRequestCount=0, idleCapacityCount=10, maxIdleCapacityCount=10, maxIdleClients=1, concurrentOperationLimit=10})
Garbage collected client [cli84iJ0] (1 active clients: {clipLmMe}, 0 failed clients: {})

The delta from #2416 is:

  • Every call to ClientPool.release is logged (previously only calls that resulted in a garbage collection were logged)
  • Every call to ClientPool.release logs details about the decision to gc or not to gc.

@dconeybe dconeybe self-assigned this Sep 19, 2025
@dconeybe dconeybe requested review from a team as code owners September 19, 2025 20:06
@dconeybe dconeybe added the api: firestore Issues related to the googleapis/nodejs-firestore API. label Sep 19, 2025
@product-auto-label product-auto-label bot added the size: m Pull request size is medium. label Sep 19, 2025
@dconeybe dconeybe mentioned this pull request Sep 19, 2025
Merged
@dconeybe dconeybe changed the title change: pool.ts: add more detailed logging for client garbage collection fix: pool.ts: add more detailed logging for client garbage collection Sep 19, 2025
@dconeybe
Copy link
Contributor Author

dconeybe commented Sep 22, 2025

You can ignore the "ci / docs (pull_request)" github action failures. That action is painfully flaky with false positives.

@dconeybe dconeybe merged commit 1bbca46 into main Sep 22, 2025
43 of 49 checks passed
@dconeybe dconeybe deleted the dconeybe/MoreClientPoolLogging branch September 22, 2025 18:37
@release-please release-please bot mentioned this pull request Sep 22, 2025
Merged
@dconeybe dconeybe mentioned this pull request Sep 25, 2025
Merged
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@wu-hui wu-hui wu-hui approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@dconeybe dconeybe

Labels

api: firestore Issues related to the googleapis/nodejs-firestore API. size: m Pull request size is medium.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@dconeybe @wu-hui