Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CBG-2983 Close cbgt agents on database close #6265

Merged
merged 2 commits into from
May 26, 2023
Merged

Conversation

adamcfraser
Copy link
Collaborator

@adamcfraser adamcfraser commented May 25, 2023

Manager.Stop() doesn’t close cbgt’s gocb agents - partly because the fts use case is to have one manager that exists for the lifetime of the process. For SG’s usage, where manager lifecycle is bound to a database, we need to shut down these agents when we close the database/importListener.

CBG-2983

Integration Tests

Manager.Stop() doesn’t close cbgt’s gocb agents - partly because the fts use case is to have one manager that exists for the lifetime of the process.  For SG’s usage, where manager lifecycle is bound to a database, we need to shut down these agents when we close the database/importListener.
Copy link
Collaborator

@torcolvin torcolvin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything is just naming improvements you can take or leave, but I think there's a problem.

If CloseStatsClients takes bucketName and bucketUUID, this will break if there a multiple databases on the same bucket, as is now allowed in 3.1

base/dcp_sharded.go Show resolved Hide resolved
base/dcp_sharded.go Show resolved Hide resolved
base/dcp_sharded.go Show resolved Hide resolved
Copy link
Collaborator Author

@adamcfraser adamcfraser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR feedback responses.

@adamcfraser
Copy link
Collaborator Author

Added a comment to indicate that cbgt does refcounting on CloseStatsClients, so it handles the case of multiple databases targeting the same bucket.

@adamcfraser adamcfraser merged commit 7093dda into master May 26, 2023
@adamcfraser adamcfraser deleted the CBG-2983-update branch May 26, 2023 13:58
torcolvin pushed a commit that referenced this pull request May 30, 2023
* CBG-2983 Close cbgt agents on database close

Manager.Stop() doesn’t close cbgt’s gocb agents - partly because the fts use case is to have one manager that exists for the lifetime of the process.  For SG’s usage, where manager lifecycle is bound to a database, we need to shut down these agents when we close the database/importListener.

* Improve inline documentation for CloseStatsClients call
bbrks pushed a commit that referenced this pull request May 31, 2023
Co-authored-by: Ben Brooks <[email protected]>
Co-authored-by: Adam Fraser <[email protected]>
Close cbgt agents on database close (#6265)
Close (#6269)
bbrks pushed a commit that referenced this pull request Mar 28, 2024
* CBG-2983 Close cbgt agents on database close

Manager.Stop() doesn’t close cbgt’s gocb agents - partly because the fts use case is to have one manager that exists for the lifetime of the process.  For SG’s usage, where manager lifecycle is bound to a database, we need to shut down these agents when we close the database/importListener.

* Improve inline documentation for CloseStatsClients call
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.

2 participants