feat(spanner): implement transaction routing logic based on database metadata isolation levels and lock modes

[shobhitsg]

No description provided.

[gemini-code-assist]

Code Review

This pull request introduces dynamic database-level metadata detection (dialect, default transaction isolation level, and default read lock mode) queried from INFORMATION_SCHEMA.DATABASE_OPTIONS to replace client-side hardcoded defaults. The feedback identifies several critical issues: a correctness bug in canEnableLRYW that breaks Leader-Routed Read-Your-Writes for repeatable read transactions with unspecified lock modes, a race condition in metadataSupplier when retrieving the session reference, and a concurrency race condition in AbstractLazyInitializer between reset() and get(). Additionally, it is recommended to add a unit test to prevent regressions for repeatable reads with null lock modes.

[gemini-code-assist]

There is a correctness bug in canEnableLRYW.

If readLockMode is unspecified and isolationLevel is REPEATABLE_READ, resolveEffectiveMode keeps readLockMode as null.

However, canEnableLRYW checks mode.readLockMode == ReadLockMode.READ_LOCK_MODE_UNSPECIFIED. Since null is not equal to ReadLockMode.READ_LOCK_MODE_UNSPECIFIED, this check evaluates to false, causing routeToLeader to be true. This completely breaks Leader-Routed Read-Your-Writes (routing to replicas) for repeatable read transactions with unspecified lock modes.

We should use the helper method isUnspecified(mode.readLockMode) instead.

Suggested change

	private static boolean canEnableLRYW(TransactionMode mode) {
	return mode.readLockMode == ReadLockMode.OPTIMISTIC
	|| (mode.isolationLevel == IsolationLevel.REPEATABLE_READ
	&& mode.readLockMode == ReadLockMode.READ_LOCK_MODE_UNSPECIFIED);
	}
	private static boolean canEnableLRYW(TransactionMode mode) {
	return mode.readLockMode == ReadLockMode.OPTIMISTIC
	|| (mode.isolationLevel == IsolationLevel.REPEATABLE_READ
	&& isUnspecified(mode.readLockMode));
	}

[gemini-code-assist]

There is an issue in the metadataSupplier initialization logic:

Race Condition: multiplexedSessionReference.get() is retrieved after the query executes. If the maintainer replaces the session concurrently, multiplexedSessionReference will point to a new, non-done future. This causes sessionRefFuture.isDone() to evaluate to false, silently skipping setting the metadata on the new session reference, while the old session reference also misses it. We should retrieve the SessionReference before running the query to ensure it is attached to the correct session.

Note: We should avoid adding defensive null checks (like checking resultSet.isNull) for values that are guaranteed to be non-null by design, as this can hide invariant breaks.

  private final AbstractLazyInitializer<DatabaseMetadata> metadataSupplier = new AbstractLazyInitializer<DatabaseMetadata>() {
    @Override
    protected DatabaseMetadata initialize() {
      Dialect dialect = Dialect.GOOGLE_STANDARD_SQL;
      IsolationLevel isolationLevel = IsolationLevel.SERIALIZABLE;
      ReadLockMode readLockMode = ReadLockMode.PESSIMISTIC;

      SessionReference sessionRef = null;
      try {
        Future<SessionReference> sessionRefFuture = multiplexedSessionReference.get();
        if (sessionRefFuture != null) {
          sessionRef = sessionRefFuture.get();
        }
      } catch (Exception exception) {
        logger.log(
            Level.WARNING,
            "Failed to retrieve session reference for database metadata initialization",
            exception);
      }

      try (ResultSet resultSet = singleUse().executeQuery(DETERMINE_METADATA_STATEMENT)) {
        while (resultSet.next()) {
          String name = resultSet.getString(0);
          String value = resultSet.getString(1);
          if ("database_dialect".equalsIgnoreCase(name)) {
            dialect = Dialect.fromName(value);
          } else if ("default_transaction_isolation".equalsIgnoreCase(name)) {
            isolationLevel = parseIsolationLevel(value);
          } else if ("default_read_lock_mode".equalsIgnoreCase(name)) {
            readLockMode = parseReadLockMode(value);
          }
        }
      } catch (Exception exception) {
        logger.log(
            Level.WARNING,
            "Failed to detect database metadata, falling back to defaults",
            exception);
      }
      DatabaseMetadata metadata = new DatabaseMetadata(dialect, isolationLevel, readLockMode);
      if (sessionRef != null) {
        sessionRef.setDatabaseMetadata(metadata);
      }
      return metadata;
    }
  };

References

1. Avoid adding defensive null checks for values that are guaranteed to be non-null by design, as this can hide invariant breaks.

[gemini-code-assist]

There is a potential concurrency race condition when reset() is called concurrently with get().

If get() has already passed the if (!initialized) check (because initialized was true), but has not yet returned object, a concurrent call to reset() can set object = null and initialized = false. This will cause get() to return null, which can lead to unexpected NullPointerExceptions in callers.

To prevent this, we can capture object in a local variable and, if it is null (indicating a concurrent reset), safely re-initialize by calling get() recursively. Additionally, in performance-sensitive code, we should prefer using explicit locks over the 'synchronized' keyword.

Suggested change

	return object;
	}
	T result = object;
	if (result == null && error == null) {
	lock.lock();
	try {
	if (!initialized && object == null && error == null) {
	return get();
	}
	result = object;
	} finally {
	lock.unlock();
	}
	}
	return result;

References

1. In performance-sensitive code, prefer using explicit locks over the 'synchronized' keyword to protect shared state while ensuring thread safety and visibility.

[gemini-code-assist]

To prevent regressions and ensure that Leader-Routed Read-Your-Writes (LRYW) is correctly enabled when readLockMode is null (unspecified), we should add a unit test case covering this scenario.

Suggested change

	TransactionContextImpl context = createContext(callSiteOptions);
	assertTrue(context.isRouteToLeader());
	}
	}
	TransactionContextImpl context = createContext(callSiteOptions);
	assertTrue(context.isRouteToLeader());
	}
	@Test
	public void testRepeatableReadWithNullLockModeCanEnableLRYW() {
	when(sessionReference.getDatabaseMetadata()).thenReturn(null);
	Options callSiteOptions = Options.fromTransactionOptions(
	Options.isolationLevel(IsolationLevel.REPEATABLE_READ));
	TransactionContextImpl context = createContext(callSiteOptions);
	assertFalse(context.isRouteToLeader());
	}
	}