Derby
  1. Derby
  2. DERBY-1696

transaction may sometimes keep lock on a row after moving off the resultset in scrollable updatable resultset

    Details

    • Urgency:
      Normal
    • Issue & fix info:
      Repro attached

      Description

      If an application does the following:

      Statement s = con.createStatement(ResultSet.TYPE_SCROLL_INSENSITIVE,
      ResultSet.CONCUR_UPDATABLE);
      ResultSet rs = s.executeQuery("select * from t1");
      rs.afterLast();
      rs.last();
      rs.next();

      After doing this in transaction isolation level read-committed/read-uncommitted, the last row is still locked with an update lock.

      This is detected by running the JUnit testcase ConcurrencyTest.testUpdatePurgedTuple1 in the DerbyNetClient framework.
      (NOTE: the bug is revealed by this test, because the network server does a rs.last() as the first operation on a scrollable updatable resultset to count number of rows)

      What triggers this bug, seems to be the repositioning of the cursor after the underlying all records have been inserted into the hashtable from the source scan. When moving off the result set (to afterLast() or beforeFirst()) no action is done to release the lock of the current row.

      1. DERBY-1696v2.diff
        11 kB
        Andreas Korneliussen
      2. DERBY-1696.stat
        0.5 kB
        Andreas Korneliussen
      3. DERBY-1696.diff
        7 kB
        Andreas Korneliussen

        Issue Links

          Activity

          Hide
          Mike Matrigali added a comment -

          derby 10.9 triage.

          Show
          Mike Matrigali added a comment - derby 10.9 triage.
          Hide
          Knut Anders Hatlen added a comment -

          Triaged for 10.5.2.

          Show
          Knut Anders Hatlen added a comment - Triaged for 10.5.2.
          Hide
          Andreas Korneliussen added a comment -

          I will update the patch for this issue once the DERBY-1799 is reviewed and committed, since these patches do have a minor conflict. Basically, the same approach will be used for this patch (using reopenScan()) to release locks when moving to afterlast /beforefirst).

          Show
          Andreas Korneliussen added a comment - I will update the patch for this issue once the DERBY-1799 is reviewed and committed, since these patches do have a minor conflict. Basically, the same approach will be used for this patch (using reopenScan()) to release locks when moving to afterlast /beforefirst).
          Hide
          Mike Matrigali added a comment -

          Is a subsequent patch coming for this issue, I was going to review but it read as if a new patch was coming. I will be reviewing the patch for DERBY-1799 but also wanted to know if a different patch was coming for this issue or is it just waiting on DERBY-1799?

          Show
          Mike Matrigali added a comment - Is a subsequent patch coming for this issue, I was going to review but it read as if a new patch was coming. I will be reviewing the patch for DERBY-1799 but also wanted to know if a different patch was coming for this issue or is it just waiting on DERBY-1799 ?
          Hide
          Andreas Korneliussen added a comment -

          Linking this issue with DERBY-1799.

          The reason SURQueryMixTest failed with the second patch, is that the patch calls reopenScan() on a BTreeScan. If the ScrollInsensitiveResultSet is not fully populated at this point, the scan has lost its position (i.e it starts from the beginning). To get back the position, we probably need a mechanism similar to positionAtRowLocation() on BTreeScan. This seems also to be required in order to set a lock on the current row (which is DERBY-1799).

          Show
          Andreas Korneliussen added a comment - Linking this issue with DERBY-1799 . The reason SURQueryMixTest failed with the second patch, is that the patch calls reopenScan() on a BTreeScan. If the ScrollInsensitiveResultSet is not fully populated at this point, the scan has lost its position (i.e it starts from the beginning). To get back the position, we probably need a mechanism similar to positionAtRowLocation() on BTreeScan. This seems also to be required in order to set a lock on the current row (which is DERBY-1799 ).
          Hide
          Andreas Korneliussen added a comment -

          Removing patch available flag, since the second patch makes SURQueryMixTest fail.

          Show
          Andreas Korneliussen added a comment - Removing patch available flag, since the second patch makes SURQueryMixTest fail.
          Hide
          Andreas Korneliussen added a comment -

          The attached patch also releases the lock on indexed scans.
          Additionally extended tests.

          Show
          Andreas Korneliussen added a comment - The attached patch also releases the lock on indexed scans. Additionally extended tests.
          Hide
          Andreas Korneliussen added a comment -

          Thanks. Testing better with indexes also uncovered another locking issue w.r.t SUR : DERBY-1799.
          I will upload a new patch for this issue.

          Show
          Andreas Korneliussen added a comment - Thanks. Testing better with indexes also uncovered another locking issue w.r.t SUR : DERBY-1799 . I will upload a new patch for this issue.
          Hide
          Fernanda Pizzorno added a comment -

          The locks are not being released when an index is being used to access the table. You can reproduce this failure by changing the test you have added so that the select statement selects only the table's primary key (indexed column). I think it would be good to run this new test with all the data models used in SURQueryMixTest.

          Show
          Fernanda Pizzorno added a comment - The locks are not being released when an index is being used to access the table. You can reproduce this failure by changing the test you have added so that the select statement selects only the table's primary key (indexed column). I think it would be good to run this new test with all the data models used in SURQueryMixTest.
          Hide
          Andreas Korneliussen added a comment -

          The attached patch addresses this bug by having the SQL-execution layer use ScanController.reopenScan() to release the lock.
          In the store layer, GenericScanController.reopenScan() has been modified to release the read-lock after read, while
          GenericScanController.reopenAfterEndTransaction() now additonally may set the rowLocations invalidated flag even if the scan_state is SCAN_HOLD_INIT. This is because previously the scan_state SCAN_HOLD_INIT would (as it was used) guarantee that no RowLocations had been read from the scan.

          Testing: one new test added to ConcurrencyTest. HoldabilityTest had to be modified: if a compress happens, and there is an open scrollable updatable resultset, no updates will be allowed, even if no scrolling has happened.

          Show
          Andreas Korneliussen added a comment - The attached patch addresses this bug by having the SQL-execution layer use ScanController.reopenScan() to release the lock. In the store layer, GenericScanController.reopenScan() has been modified to release the read-lock after read, while GenericScanController.reopenAfterEndTransaction() now additonally may set the rowLocations invalidated flag even if the scan_state is SCAN_HOLD_INIT. This is because previously the scan_state SCAN_HOLD_INIT would (as it was used) guarantee that no RowLocations had been read from the scan. Testing: one new test added to ConcurrencyTest. HoldabilityTest had to be modified: if a compress happens, and there is an open scrollable updatable resultset, no updates will be allowed, even if no scrolling has happened.
          Hide
          Andreas Korneliussen added a comment -

          To fix this issue, I need a mechanism to notify the store (scancontroller) to move off the row (i.e to afterLast() or beforeFirst()), so that it can release the lock on the current row.

          I do consider the following options:

          Alternative 1:
          Use the method ScanController.positionAtRowLocation(RowLocation rl)

          Here the RowLocation objects could represent the positions beforeFirst and afterLast. I.e one could make use of the
          RecordHandle. RESERVED4_RECORD_HANDLE and
          RecordHandle. RESERVED4_RECORD_HANDLE to represent to beforeFirst and afterLast positions.

          When the method ScanController.positionAtRowLocation(RowLocation rl), is called with a rowlocation with these positions,
          the scan implementation may release the U-lock of the current row

          Alternative 2:
          Add new methods to ScanController interface: moveToAfterLast() and moveToBeforeFirst()

          Show
          Andreas Korneliussen added a comment - To fix this issue, I need a mechanism to notify the store (scancontroller) to move off the row (i.e to afterLast() or beforeFirst()), so that it can release the lock on the current row. I do consider the following options: Alternative 1: Use the method ScanController.positionAtRowLocation(RowLocation rl) Here the RowLocation objects could represent the positions beforeFirst and afterLast. I.e one could make use of the RecordHandle. RESERVED4_RECORD_HANDLE and RecordHandle. RESERVED4_RECORD_HANDLE to represent to beforeFirst and afterLast positions. When the method ScanController.positionAtRowLocation(RowLocation rl), is called with a rowlocation with these positions, the scan implementation may release the U-lock of the current row Alternative 2: Add new methods to ScanController interface: moveToAfterLast() and moveToBeforeFirst()

            People

            • Assignee:
              Unassigned
              Reporter:
              Andreas Korneliussen
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:

                Development