Filesystem lease based credentials cache #1
Conversation
| ) | ||
|
|
||
| // Helper functions for min/max (Go < 1.21 compatibility) | ||
| func min(a, b time.Duration) time.Duration { |
There was a problem hiding this comment.
Go version here is not the same as gosnowflake, so this was added for compat
|
|
||
| // In-memory cache to avoid file reads on every token access | ||
| mu sync.RWMutex | ||
| memCache map[string]*oauth2.Token // hostname -> token |
There was a problem hiding this comment.
Separate cache to solve the following problem(s):
- Lease contention in high concurrency scenarios
- Slow connection creation time due to high contention
Rather than making the credentials cache a file system based read/write lock and dealing with all the complexities there:
- Multiple processes and subsequent runs share via the file system cache
- Multiple threads share in memory
| return nil, errors.New("unhandled cloud type: " + cloud.String()) | ||
| } | ||
|
|
||
| // Use default port if not specified |
There was a problem hiding this comment.
Port specification to come later when we need to handle app redirect. tbd!
| cacheDir string | ||
| leaseHandler *LeaseHandler | ||
|
|
||
| // In-memory cache to avoid file reads on every token access |
There was a problem hiding this comment.
| // In-memory cache to avoid file reads on every token access | |
| // In-memory cache to allow relaxed reads (allowed to be stale relative to persisted values) |
| if port == 0 { | ||
| port = defaultPort | ||
| } |
There was a problem hiding this comment.
Can you handle this at the callers of NewAuthenticator?
| } | ||
|
|
||
| authenticator, err := u2m.NewAuthenticator(os.Getenv("DATABRICKS_HOST"), 1*time.Minute) | ||
| authenticator, err := u2m.NewAuthenticator(os.Getenv("DATABRICKS_HOST"), 1*time.Minute, 0) |
| Transport http.RoundTripper | ||
| UseLz4Compression bool | ||
| EnableMetricViewMetadata bool | ||
| OAuthRedirectPort int // Port for OAuth U2M redirect callback (default: 8030) |
There was a problem hiding this comment.
You should set it to the defaultPort in WithDefaults
| } | ||
|
|
||
| // readToken reads a cached token, first from memory, then from disk | ||
| func (tc *tokenCache) readToken(hostname string) (*oauth2.Token, error) { |
There was a problem hiding this comment.
That's not a good API. Multi-threaded programming is challenging because encapsulation of concerns becomes almost impossible. You want the caller to be aware that it's doing a relaxed read. You can't just return a stale value.
Call it readTokenRelaxed(hostname).
| } | ||
|
|
||
| // readTokenFromDisk reads a token from the disk cache file | ||
| func (tc *tokenCache) readTokenFromDisk(hostname string) (*oauth2.Token, error) { |
There was a problem hiding this comment.
Is this a private function? I'm forgetting what makes functions private/public in Go.
| } | ||
|
|
||
| // writeToken writes a token to the cache file and in-memory cache | ||
| func (tc *tokenCache) writeToken(hostname string, token *oauth2.Token) error { |
There was a problem hiding this comment.
This function should take a lease. You can never mutate without a lease, lest you write bad data into the cache.
There was a problem hiding this comment.
You can have a tryWriteToken(optionalLease, ...) defined as:
cached = relaxed_read();
if cached == token:
return
if optionalLease == nil:
return error // caller must acquire a lease and try again
write to the file
Pulls in the lease implementation that we use in Snowflake with a slight twist.
Issues resolved: