Derby
  1. Derby
  2. DERBY-4967

Handle interrupt received while waiting for database lock

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 10.8.1.2
    • Component/s: Store
    • Labels:
      None
    • Issue & fix info:
      Patch Available

      Description

      Subtask of DERBY-4741: this issue tracks the changes needed to handle interrupt received while a thread is waiting for a database lock.

      1. derby-4967-locking-1.diff
        4 kB
        Dag H. Wanvik
      2. derby-4967-locking-1.stat
        0.3 kB
        Dag H. Wanvik
      3. derby-4967-locking-2.stat
        0.2 kB
        Dag H. Wanvik
      4. derby-4967-locking-2.diff
        3 kB
        Dag H. Wanvik
      5. derby-4967-locking-3.stat
        0.2 kB
        Dag H. Wanvik
      6. derby-4967-locking-3.diff
        4 kB
        Dag H. Wanvik
      7. derby-4967-locking-4.diff
        4 kB
        Dag H. Wanvik
      8. derby-4967-locking-4.stat
        0.2 kB
        Dag H. Wanvik

        Issue Links

          Activity

          Hide
          Dag H. Wanvik added a comment - - edited

          Uploading a patch which makes Derby give up waiting for a lock if an interrupt is seen.
          Instead it will throw SQLState.CONN_INTERRUPT (the thread's interrupt flag will still be set).

          Show
          Dag H. Wanvik added a comment - - edited Uploading a patch which makes Derby give up waiting for a lock if an interrupt is seen. Instead it will throw SQLState.CONN_INTERRUPT (the thread's interrupt flag will still be set).
          Hide
          Dag H. Wanvik added a comment -

          Passed suites.All.

          Show
          Dag H. Wanvik added a comment - Passed suites.All.
          Hide
          Dag H. Wanvik added a comment -

          Committed the patch as svn 1058245, resolving.

          Show
          Dag H. Wanvik added a comment - Committed the patch as svn 1058245, resolving.
          Hide
          Dag H. Wanvik added a comment -

          Resolved, post-commit reviews still welcome

          Show
          Dag H. Wanvik added a comment - Resolved, post-commit reviews still welcome
          Hide
          Dag H. Wanvik added a comment - - edited

          Uploading derby-4967-locking-2, which makes the existing test LockInterruptTest
          assert that the interrupt flag is set when we see 08000 (CONN_INTERRUPT).

          This was not the case before derby-4967-locking-1 was committed.

          Note: On Solaris, the test only asserts the flag this if one of the following conditions hold:

          • the flag derbyTesting.safeSolarisInterrupts is true. This can be used for force the assert to run
            for Solaris with Java 1.6 and lower. The assert will only succeed then if the Java flag -XX:-UseVMInterruptibleIO is used.
          • the test is running on Java 1.7 or higher

          If the test is skipped, we do a println to indicate this (iff derby.tests.debug is true).

          On other platforms we always do the assert.

          BaseTestCase has been extended with two methods to allows tests to determine if they are running with safe Solaris: #isSolaris and #isSafeSolarisInterrupts

          Running regressions.

          Show
          Dag H. Wanvik added a comment - - edited Uploading derby-4967-locking-2, which makes the existing test LockInterruptTest assert that the interrupt flag is set when we see 08000 (CONN_INTERRUPT). This was not the case before derby-4967-locking-1 was committed. Note: On Solaris, the test only asserts the flag this if one of the following conditions hold: the flag derbyTesting.safeSolarisInterrupts is true. This can be used for force the assert to run for Solaris with Java 1.6 and lower. The assert will only succeed then if the Java flag -XX:-UseVMInterruptibleIO is used. the test is running on Java 1.7 or higher If the test is skipped, we do a println to indicate this (iff derby.tests.debug is true). On other platforms we always do the assert. BaseTestCase has been extended with two methods to allows tests to determine if they are running with safe Solaris: #isSolaris and #isSafeSolarisInterrupts Running regressions.
          Hide
          Dag H. Wanvik added a comment -

          Regressions passed.

          Show
          Dag H. Wanvik added a comment - Regressions passed.
          Hide
          Knut Anders Hatlen added a comment -

          > - the flag derbyTesting.safeSolarisInterrupts is true. This can be used for force the assert to run
          > for Solaris with Java 1.6 and lower. The assert will only succeed then if the Java flag -XX:-UseVMInterruptibleIO is used.

          Could we detect this dynamically by trying some operation and seeing how it behaves? If that could be done easily, it might be preferable to setting a property to get it tested.

          Show
          Knut Anders Hatlen added a comment - > - the flag derbyTesting.safeSolarisInterrupts is true. This can be used for force the assert to run > for Solaris with Java 1.6 and lower. The assert will only succeed then if the Java flag -XX:-UseVMInterruptibleIO is used. Could we detect this dynamically by trying some operation and seeing how it behaves? If that could be done easily, it might be preferable to setting a property to get it tested.
          Hide
          Dag H. Wanvik added a comment -

          Yes, I agree this would be nicer. I'll run some experiments and see.

          Show
          Dag H. Wanvik added a comment - Yes, I agree this would be nicer. I'll run some experiments and see.
          Hide
          Dag H. Wanvik added a comment -

          Uploading version three of this patch. BaseTestCase now contains a method "hasInterruptibleIO" which will autodetect if IO is interruptible, which makes the test code easier. Running regressions.

          Show
          Dag H. Wanvik added a comment - Uploading version three of this patch. BaseTestCase now contains a method "hasInterruptibleIO" which will autodetect if IO is interruptible, which makes the test code easier. Running regressions.
          Hide
          Dag H. Wanvik added a comment -

          suites.All ran ok.

          Show
          Dag H. Wanvik added a comment - suites.All ran ok.
          Hide
          Knut Anders Hatlen added a comment -

          Thanks, Dag. This way to detect the whether interruptible I/O is enabled looks fine to me.

          Is RAF.close() interruptible, by the way? I see that the interrupt flag isn't cleared until after close() has been called. Maybe it's safer to postpone the closing until after the interrupt flag has been cleared?

          And a tiny nit: If you move "new RandomAccessFile(...)" out of the try clause and up to the declaration, the file can be closed unconditionally in the finally clause (can never be null).

          Show
          Knut Anders Hatlen added a comment - Thanks, Dag. This way to detect the whether interruptible I/O is enabled looks fine to me. Is RAF.close() interruptible, by the way? I see that the interrupt flag isn't cleared until after close() has been called. Maybe it's safer to postpone the closing until after the interrupt flag has been cleared? And a tiny nit: If you move "new RandomAccessFile(...)" out of the try clause and up to the declaration, the file can be closed unconditionally in the finally clause (can never be null).
          Hide
          Dag H. Wanvik added a comment -

          Thanks, Knut. Just checked, RAF.close() doesn't give an interrupt as far as I can see, but it seems safer, I agree, I'll fix that plus the nit, +1 to simplification

          Show
          Dag H. Wanvik added a comment - Thanks, Knut. Just checked, RAF.close() doesn't give an interrupt as far as I can see, but it seems safer, I agree, I'll fix that plus the nit, +1 to simplification
          Hide
          Dag H. Wanvik added a comment -

          Uploading version 4 to handle Knut's comments.

          Show
          Dag H. Wanvik added a comment - Uploading version 4 to handle Knut's comments.
          Hide
          Dag H. Wanvik added a comment -

          Committed patch revision 4 as svn 1060832. resolving.

          Show
          Dag H. Wanvik added a comment - Committed patch revision 4 as svn 1060832. resolving.
          Hide
          Knut Anders Hatlen added a comment -

          [bulk update] Close all resolved issues that haven't been updated for more than one year.

          Show
          Knut Anders Hatlen added a comment - [bulk update] Close all resolved issues that haven't been updated for more than one year.

            People

            • Assignee:
              Dag H. Wanvik
              Reporter:
              Dag H. Wanvik
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development