goaymode created IGNITE-25794:
---------------------------------
Summary: Optimize lockMultiple() in IgniteTxManager for Large
Number of Entries
Key: IGNITE-25794
URL: https://issues.apache.org/jira/browse/IGNITE-25794
Project: Ignite
Issue Type: Improvement
Components: cache
Affects Versions: 2.16, 2.10
Reporter: goaymode
In
[IgniteTxManager.lockMultiple(),|https://github.com/apache/ignite/blob/cdf8e3a146189610c3e23cf8055510c460e7ccba/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/transactions/IgniteTxManager.java#L1885]
when an attempt to lock a cache entry fails all previously acquired locks are
released with
[IgniteTxManager.txUnlock()|https://github.com/apache/ignite/blob/cdf8e3a146189610c3e23cf8055510c460e7ccba/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/transactions/IgniteTxManager.java#L1929]
to maintain consistency. However, the [unlocking
loop|https://github.com/apache/ignite/blob/cdf8e3a146189610c3e23cf8055510c460e7ccba/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/transactions/IgniteTxManager.java#L1925C25-L1925C65]
scans from the beginning of *entries list* up to the point of failure each
time a lock attempt fails. I believe this approach is inefficient, especially
when dealing with a large number of entries or if there are repeated failures
as it redundantly iterates over entries that have already been already unlocked
in previous attempts. This is also happening under [checkpointReadLock()
|https://github.com/apache/ignite/blob/cdf8e3a146189610c3e23cf8055510c460e7ccba/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/transactions/IgniteTxManager.java#L1906]increasing
risk of contention. And furthermore,
[IgniteTxAdapter.topologyVersion|https://github.com/apache/ignite/blob/cdf8e3a146189610c3e23cf8055510c460e7ccba/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/transactions/IgniteTxAdapter.java#L547]
which isn’t expensive is called within retry paths inside
[txUnlock()|https://github.com/apache/ignite/blob/cdf8e3a146189610c3e23cf8055510c460e7ccba/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/transactions/IgniteTxManager.java#L1992]
and
[lockMultiple()|https://github.com/apache/ignite/blob/cdf8e3a146189610c3e23cf8055510c460e7ccba/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/transactions/IgniteTxManager.java#L1943].
When retries occur frequently, the repeated lookup and iteration over a
potentially large number of cache contexts (topologyVersion() →
[IgniteTxManager.lockedTopologyVersion()|https://github.com/apache/ignite/blob/cdf8e3a146189610c3e23cf8055510c460e7ccba/modules/core/src/main/java/org/apache/ignite/internal/processors/cache/transactions/IgniteTxManager.java#L1006]
) can further contribute to performance degradation.
I think one optimization for lockMultiple() would be to keep track of only the
entries that have been successfully locked so far. When a lock attempt fails,
only those tracked entries would be unlocked, avoiding unnecessary *full scans*
of the entire entries list on each failure. See Below
{code:java}
List<IgniteTxEntry> lockedEntries = new ArrayList<>();
for (IgniteTxEntry txEntry1 : entries) {
if (!txEntry1.markPrepared() || txEntry1.explicitVersion() != null)
continue;
while (true) {
cctx.database().checkpointReadLock();
try {
GridCacheEntryEx entry1 = txEntry1.cached();
....
if (!entry1.tmLock(tx, timeout, serOrder, serReadVer, read)) {
// Unlock only the entries we actually locked
for (IgniteTxEntry locked : lockedEntries) {
txUnlock(tx, locked);
}
return false;
}
// Record successfully locked entry
lockedEntries.add(txEntry1);
break;
}
catch (...) {
// retry
}
}
}
{code}
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
