[sakthivelmanii]

Problem

When AbstractLazyInitializer.get() is invoked and initialize() throws an exception, the failure was previously permanently cached in this.error with this.initialized = true regardless of whether the error was transient or retryable. In MultiplexedSessionDatabaseClient.getDialect() and DatabaseClientImpl.getStatementFactory(), an initial transient failure during session creation or query execution (DEADLINE_EXCEEDED, RESOURCE_EXHAUSTED, or UNAVAILABLE) caused all subsequent calls to immediately rethrow the cached exception without retrying, even after multiplexed session creation succeeded on retry.

Solution

Update AbstractLazyInitializer to introduce a protected boolean shouldCacheError(Exception) method that returns true by default, while allowing subclasses and client components to override it to avoid caching transient or retryable errors.

• In AbstractLazyInitializer, define protected boolean shouldCacheError(Exception) returning true by default so retryable errors can be selectively retried on subsequent get() calls without permanent caching.
• In MultiplexedSessionDatabaseClient (dialectSupplier), override shouldCacheError(Exception) to not cache retryable SpannerException errors matching RETRYABLE_ERROR_CODES (DEADLINE_EXCEEDED, RESOURCE_EXHAUSTED, UNAVAILABLE).
• In DatabaseClientImpl, simplify statementFactorySupplier to call new StatementFactory(getDialect()) directly and override shouldCacheError(Exception) to return false (delegating error caching and retry lifecycle entirely to getDialect()), and remove unused getDialectAsync() and unused imports.
• Add unit tests in LazySpannerInitializerTest and DatabaseClientImplTest covering retryable error recovery, non-retryable error caching, and concurrent retries.

[gemini-code-assist]

Code Review

This pull request updates AbstractLazyInitializer to allow retrying initialization when transient, retryable errors occur, rather than caching them indefinitely. It also refactors exception handling in DatabaseClientImpl when retrieving the database dialect and exposes RETRYABLE_ERROR_CODES from MultiplexedSessionDatabaseClient. Comprehensive tests have been added to verify recovery behavior. The review feedback suggests simplifying the shouldCacheError method in AbstractLazyInitializer by removing redundant checks for specific exception subclasses, as they are already covered by the more general SpannerException check.

[olavloite]

This class is a generic abstract class that in theory could be used for any kind of lazy initialization. It should not contain such specific code for the MultiplexedSessionDatabaseClient. You should rather design this by adding a method shouldCacheError(Exception) that by default returns false, and then extend this class in MultiplexedSessionClient and override the method there, and then add this logic in that method implementation.

[sakthivelmanii]

Made shouldCacheError(Exception) a protected method on AbstractLazyInitializer (returning true by default to preserve existing behavior for other initializers) and overridden it in MultiplexedSessionDatabaseClient and DatabaseClientImpl.

[olavloite]

nit: remove 'for backward compatibility.' That sentence only makes sense in the context of the current PR, but in one year from now, it will not make sense to a reader anymore.

[sakthivelmanii]

Removed 'for backward compatibility' from the Javadoc.

[olavloite]

I think that we can simplify this implementation to this:

  private final AbstractLazyInitializer<StatementFactory> statementFactorySupplier =
      new AbstractLazyInitializer<StatementFactory>() {
        @Override
        protected StatementFactory initialize() {
          return new StatementFactory(getDialect());
        }

        @Override
        protected boolean shouldCacheError(Exception e) {
          // getDialect() already decides whether to cache or retry dialect errors.
          // Never cache errors here so that statementFactorySupplier always delegates
          // the retry/caching lifecycle entirely to getDialect().
          return false;
        }
      };

[sakthivelmanii]

Simplified statementFactorySupplier to call getDialect() directly and return false in shouldCacheError(Exception).