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

Implement SessionResetter interface #7

Merged
merged 2 commits into from
Mar 29, 2021
Merged

Implement SessionResetter interface #7

merged 2 commits into from
Mar 29, 2021

Conversation

khous
Copy link
Contributor

@khous khous commented Mar 29, 2021

This interface allows the driver to tell database/sql when a connection needs to be discarded from the pool.
These methods in this hierarchy end up calling db.conn which requests a connection from the pool and asks the driver if a reset is needed. thus flushing the connection pool.

Copy link

@mphillips-infoblox mphillips-infoblox left a comment

Choose a reason for hiding this comment

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

Can we make resyncPeriod configurable in filewatcher.go?

@khous
Copy link
Contributor Author

khous commented Mar 29, 2021

Can we make resyncPeriod configurable in filewatcher.go?

Yeah, but why? Do you think the filesystem will need to be checked more often than two seconds? Not sure if this is a good thing to open up to users who are likely to misinterpret its usage.

@khous khous merged commit 647c00f into main Mar 29, 2021
@mphillips-infoblox
Copy link

That is true, but we may want to make it longer depending on how the secret updating strategy is configured.
https://kubernetes.io/docs/concepts/configuration/secret/#mounted-secrets-are-updated-automatically

@khous
Copy link
Contributor Author

khous commented Mar 29, 2021

That is true, but we may want to make it longer depending on how the secret updating strategy is configured.
https://kubernetes.io/docs/concepts/configuration/secret/#mounted-secrets-are-updated-automatically

fsnotify is getting file updates immediately. The resync is just a catchall in case something fails afaict.

@khous khous deleted the khous/reset-session branch March 29, 2021 19:02
@mphillips-infoblox
Copy link

ok, lets leave it for now

@@ -95,6 +96,14 @@ func (c *managedConn) IsValid() bool {
return s.IsValid()
}

func (c *managedConn) ResetSession(ctx context.Context) error {
if c.reset {
Copy link
Contributor

Choose a reason for hiding this comment

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

This variable is inspected but it is set when a mutex is set; this is a race condition.

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.

3 participants