[
https://issues.apache.org/jira/browse/DERBY-6554?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13992840#comment-13992840
]
Rick Hillegas commented on DERBY-6554:
--------------------------------------
Hi Mike,
Some responses to your comments:
MM> can you comment on why you decided to throw a new exception on self
deadlock rather than just throw
MM> deadlock? Just throwing deadlock would solve the problem if it ever leaks
out, and seems the right choice.
MM>
MM> But, maybe you need the distinct error for a logic path.
Yes, that is the problem. SequenceUpdater needs to distinguish a SelfDeadlock
from an ordinary Deadlock.
MM>
MM> It is hard with java to guarantee that these exceptions which are meant to
always be caught internally
MM> never leak. I think the only completely safe way is to create something
different than a StandardException
MM> and then explicitly declare it - but my guess is that is going to be
painful with such a low level routine
MM> that is likely used by a lot of paths.
I can create a SelfDeadlock subtype of StandardException. StandardException
already has a few subtypes. But that won't address the performance question.
MM> Trying to think about the compress self deadlock issue.
MM>
MM> o Before your change is a lock timeout ever raised in the case of 0 wait
lock request?
MM>
I don't think so. It appears to me that ConcurrentLockSet.lockObject() returns
null if you can't get a lock immediately and (AbstractPool.noLockWait(timeout,
compatibilitySpace) == true). But I don't understand what condition sets up the
special "else" case in AbstractPool.noLockWait():
{noformat}
static boolean noLockWait(int timeout, CompatibilitySpace compat) {
if (timeout == C_LockFactory.NO_WAIT) {
return true;
} else {
LockOwner owner = compat.getOwner();
return owner != null && owner.noWait();
}
}
{noformat}
MM> o Do any of your changes require a self deadlock exception to be raised in
the case of 0 wait lock request?
MM>
The requirement is that SequenceUpdater have a way to figure out that
SelfDeadlock happened. That is, that it was blocked by its own (parent
transaction) lock rather than by a lock held by another Connection. There are a
lot of code layers between SequenceUpdater and ConcurrentLockSet. There may be
some way to signal SelfDeadlock without raising an exception. This would
involve changes to the ConglomerateController interface and maybe some other
interfaces.
MM> I with this new lock functionality that sequences will wait for some time
for the lock now that self deadlock
MM> will return immediately so won't be following the 0 wait lock request code
path.
MM>
MM> If possible it would seem better not to raise an exception at all in this
case where the request to lock
MM> manager asked not to wait at all.
Agreed.
MM> Does the following work/make sense:
MM>
MM> Change lock manger to continue to return no exception if lock can not be
granted if lock is 0 wait. Hopefully
MM> this would then limit impact on other parts of the code until they can be
changed to take advantage of the
MM> new functionality.
MM>
MM> Change lock manger to always check for self deadlock (if it has the
available fields to check), if a
MM> non-0 wait lock can not be granted, and throw exception if it is a self
deadlock.
MM>
MM> Change DataDictionaryImpl.java!updateCurrentSequenceValue to always wait
for the lock, knowing that
MM> it will be a 0 wait if it is a self deadlock, and then change caller to
catch and retry on appropriate exception
MM> thrown. Not many comments in this routine, but I think all the wait inputs
and complication was to take
MM> care of the problem being solved by doing self deadlock detection. So maybe
both caller and this code
MM> can be simplified by always waiting. Not sure how much to wait - either
user set wait, or internal hard
MM> coded, or some combination of both - for instance may want to wait some in
internal transaction even if
MM> user has specified 0 wait for user locks
Not sure I follow. I think what you are suggesting is the following:
1) Have SequenceUpdater wait for the smallest non-zero timeout, 1 millisecond.
The smaller the timeout the better. We want to prevent threads from piling up
like a train wreck behind a request to allocate a new range of sequence values.
2) Only raise SelfDeadlock if the timeout is non-zero.
I think I would like to try another solution first:
1) Currently, all negative values of derby.locks.waitTimeout are collapsed into
a single meaning: WaitForever. We can give Integer.MIN_VALUE a special meaning:
DontWaitButRaiseSelfDeadlockOnFailure
2) The lock manager will raise SelfDeadlock only if the timeout value is
DontWaitButRaiseSelfDeadlockOnFailure.
3) At least initially, SequenceUpdater will be the only caller who requests a
lock with a timeout of DontWaitButRaiseSelfDeadlockOnFailure.
> Too much contention followed by assert failure when accessing sequence in
> transaction that created it
> -----------------------------------------------------------------------------------------------------
>
> Key: DERBY-6554
> URL: https://issues.apache.org/jira/browse/DERBY-6554
> Project: Derby
> Issue Type: Bug
> Components: SQL
> Affects Versions: 10.9.1.0, 10.11.0.0, 10.10.2.0
> Reporter: Knut Anders Hatlen
> Attachments: D6554.java, D6554_2.java,
> derby-6554-01-aa-useCreationTransaction.diff,
> derby-6554-01-ab-useCreationTransaction.diff,
> derby-6554-01-ac-useCreationTransaction.diff, derby-6554-01-ad-bugfixes.diff,
> derby-6554-02-aa-selfDeadlock.diff, derby-6554-02-ab-selfDeadlock.diff,
> derby-6554-02-ac-selfDeadlock.diff,
> derby-6554-02-ae-selfDeadlock_sps_compress.diff
>
>
> {noformat}
> ij version 10.11
> ij> connect 'jdbc:derby:memory:db;create=true' as c1;
> ij> autocommit off;
> ij> create sequence seq;
> 0 rows inserted/updated/deleted
> ij> values next value for seq;
> 1
> -----------
> ERROR X0Y84: Too much contention on sequence SEQ. This is probably caused by
> an uncommitted scan of the SYS.SYSSEQUENCES catalog. Do not query this
> catalog directly. Instead, use the SYSCS_UTIL.SYSCS_PEEK_AT_SEQUENCE function
> to view the current value of a query generator.
> ij> rollback;
> ERROR 08003: No current connection.
> ij> connect 'jdbc:derby:memory:db' as c2;
> ij(C2)> autocommit off;
> ij(C2)> create sequence seq;
> 0 rows inserted/updated/deleted
> ij(C2)> values next value for seq;
> 1
> -----------
> ERROR 38000: The exception
> 'org.apache.derby.shared.common.sanity.AssertFailure: ASSERT FAILED Identity
> being changed on a live cacheable. Old uuidString =
> 0ddd00a9-0145-98ba-79df-000007d88b08' was thrown while evaluating an
> expression.
> ERROR XJ001: Java exception: 'ASSERT FAILED Identity being changed on a live
> cacheable. Old uuidString = 0ddd00a9-0145-98ba-79df-000007d88b08:
> org.apache.derby.shared.common.sanity.AssertFailure'.
> {noformat}
--
This message was sent by Atlassian JIRA
(v6.2#6252)
