db-derby-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Rick Hillegas (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (DERBY-6554) Too much contention followed by assert failure when accessing sequence in transaction that created it
Date Sat, 10 May 2014 22:10:52 GMT

    [ 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)

Mime
View raw message