-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Add a Close method to storage extension's Client interface, and use it #3506
Add a Close method to storage extension's Client interface, and use it #3506
Conversation
for _, client := range lfs.clients { | ||
client.close() | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is not clear how we can do garbage collection after this change. As TODO says we were planning to delete everything that does not have a client. Now that we no longer track clients how do we do that? TODO now seems irrelevant/impossible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the future, I think a mechanism very similar to the previous design should work for cleanup. Each time GetClient
is called, we can capture the filename associated with the client that is created. When the extension shuts down, we can use the same logic that we had intended.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Every component that gets a client should close it when they are shutdown, so not sure if this extension should do the cleanup.
Please resolve the conflict. |
for _, client := range lfs.clients { | ||
client.close() | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Every component that gets a client should close it when they are shutdown, so not sure if this extension should do the cleanup.
Based on the current experience, they are not necessary. Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>
Addresses issues identified in review of potential port to collector core.