Repository navigation
Conversation
Member
Author
|
TODO: possible write-optimization when using Trade-off: A read penalty if the same row is read immediately after. |
[tidb ci] Signed-off-by: Alexander Schwartz <alexander.schwartz@ibm.com>
TiDB accepts FOR UPDATE SKIP LOCKED but ignores the SKIP LOCKED part, so the session lock check reported rows as locked that were not, which led to optimistic lock failures under concurrent logins. On TiDB, use a plain FOR UPDATE that waits for the lock, the version comparison afterward detects a changed row. See pingcap/tidb#18207 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Alexander Schwartz <alexander.schwartz@ibm.com>
TiDB ignores SKIP LOCKED, and plain FOR UPDATE can deadlock when concurrent requests lock session rows in different orders. Use NOWAIT instead and treat a row locked by another transaction as not locked, so the caller retries in a separate transaction. Add a test that a failed lock leaves the transaction usable. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Alexander Schwartz <alexander.schwartz@ibm.com>
Explain why NOWAIT works on the MySQL dialect only, as it depends on how Hibernate marks transactions for rollback after a failed lock. Throw an exception if the fallback is reached on another dialect, and check the dialect instead of the product name in the test. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Alexander Schwartz <alexander.schwartz@ibm.com>
Compute the TiDB flag when the EntityManagerFactory is initialized instead of lazily on first use, so the shared properties map is no longer written concurrently. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Alexander Schwartz <alexander.schwartz@ibm.com>
ahus1
force-pushed
the
is-54724-spike-tidb
branch
from
November 12, 2026 09:25
d13a849 to
ef54011
Compare
Both settings roughly halve the latency of writes. They are enabled by default on TiDB clusters, but can be off on upgraded clusters. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Alexander Schwartz <alexander.schwartz@ibm.com>
Member
Author
|
Those settings are already "on" by default when running a cluster. Keycloak will warn if they are not. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #54724
Spike to find out if Keycloak works with TiDB when it is treated as a plain MySQL database (vendor
mysql, MySQL dialect and driver). The tests run against TiDB v8.5.8 viaKC_TEST_DATABASE=tidb, and the CI job runs nightly, manually, or when the commit message contains[tidb ci].Finding: TiDB accepts
FOR UPDATE SKIP LOCKEDbut silently ignores theSKIP LOCKEDpart (pingcap/tidb#18207). A row locked by another transaction is returned instead of being skipped.JpaUserSessionPersisterProvider.lockUserSessionandlockClientSessionuse it to check that the row is locked by the current transaction, so on TiDB they reported rows as locked that were not. The versioned update then matched 0 rows, which failedConcurrentLoginTestwithOptimisticLockExceptionand an HTTP 500 "Database operation failed".Quick fix in this PR:
JpaUtils.isSkipLockedSupporteddetects TiDB via the JDBC product version (8.0.11-TiDB-v8.5.8), as the dialect and product name areMySQL. On TiDB, both lock methods useFOR UPDATE NOWAITinstead ofSKIP LOCKED. If the row is locked by another transaction, TiDB fails immediately (error 3572), and the persister treats this as "not locked", so the existing retry path in a separate transaction applies. PlainFOR UPDATEwas tried first, but it deadlocked inConcurrentLoginTest.concurrentLoginSingleUserSingleClienton CI when requests locked rows in a different order. WithNOWAIT,StatelessTestSuitepasses on TiDB v8.5.8 (150 tests, 0 failures). This relies on how Hibernate handles the failed lock: with a lock timeout and the MySQL dialect, it doesn't mark the transaction as rollback-only, while other dialects such as PostgreSQL do. Therefore the fallback is intended for TiDB only and throws anUnsupportedOperationExceptionon other dialects.UserSessionPersisterLockTest(MySQL dialect only) verifies that a failed lock returnsfalseand leaves the transaction usable, and will fail if a Hibernate upgrade changes this.Other places using
SKIP LOCKED(event store expiry, outbox drain and poll) were reviewed but not changed. On TiDB they should only wait for locks instead of skipping rows, and not produce wrong results.