feat(bigtable): add config manager in bidi package

[sushanb]

No description provided.

[gemini-code-assist]

Code Review

This pull request introduces a ConfigManager to handle fetching and periodically polling client configurations from the Bigtable server, including logic for probabilistic session routing. Feedback identifies a potential goroutine leak in StartPolling due to overwriting the cancel function, suggests refactoring ShouldUseSession to use internal state for idiomatic consistency, and recommends adding a timeout to the GetClientConfiguration RPC call to prevent indefinite blocking.

[gemini-code-assist]

Calling StartPolling multiple times will overwrite the m.cancel function, leading to a leak of previous polling goroutines because they can no longer be stopped via Close(). Additionally, the goroutine should ensure m.cancel is cleared upon exit so that polling can be restarted if needed. The suggested fix uses a lock to prevent concurrent starts and ensures the field is cleared upon exit, while avoiding performing any slow operations while holding the exclusive lock.

func (m *ConfigManager) StartPolling(ctx context.Context, instanceName, appProfileId string) {
	m.mu.Lock()
	if m.cancel != nil {
		m.mu.Unlock()
		return
	}
	pollCtx, cancel := context.WithCancel(ctx)
	m.cancel = cancel
	m.mu.Unlock()

	go func() {
		defer func() {
			cancel()
			m.mu.Lock()
			m.cancel = nil
			m.mu.Unlock()
		}()

References

1. Avoid performing slow operations, such as gRPC channel creation, while holding an exclusive lock, as this can block other operations and slow down the entire client.

[gemini-code-assist]

ShouldUseSession is defined as a method on ConfigManager but it doesn't use the manager's internal state, requiring the caller to pass the config manually. It would be more idiomatic for the manager to provide a parameterless ShouldUseSession() method that uses m.currentConfig. Avoid converting this to a standalone helper function to maintain API consistency and follow repository patterns.

References

1. Avoid redundant top-level functions if a similar method exists on a struct, especially if it leads to API inconsistency or deviates from parity with other SDKs.

[gemini-code-assist]

The RPC call to GetClientConfiguration lacks a timeout. If the network hangs, the polling goroutine could be blocked indefinitely. Ensure the RPC's context is tied to a lifecycle with a timeout to prevent resource leaks or long delays.

References

1. Propagate the RPC's context to factory calls that create new gRPC connections to ensure that connection attempts are tied to the RPC's lifecycle and can be cancelled or timed out, preventing resource leaks or long delays.