Derby
  1. Derby
  2. DERBY-5233

Interrupt of create table or index (i.e. a container) will throw XSDF1 under NIO - connection survives

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 10.2.1.6, 10.2.2.0, 10.3.1.4, 10.3.2.1, 10.3.3.0, 10.4.1.3, 10.4.2.0, 10.5.1.1, 10.5.2.0, 10.5.3.0, 10.6.1.0, 10.6.2.1, 10.7.1.1, 10.8.1.2
    • Fix Version/s: 10.8.2.2, 10.9.1.0
    • Component/s: Store
    • Labels:
      None
    • Issue & fix info:
      Repro attached
    • Bug behavior facts:
      Seen in production

      Description

      Cf the enclosed repro. It would be good to make Derby ignore the interrupt here. Cf DERBY-4741. Note that this is less serious than in the cases referred to in DERBY-4741: here the database does not get shut down, even the connection survives, cf the repro. So, this can be considered a follow-up to DERBY-4741 to further improve Derby's robustness under interrupts.

      1. DERBY-5233-2.stat
        0.2 kB
        Dag H. Wanvik
      2. DERBY-5233-2.diff
        9 kB
        Dag H. Wanvik
      3. DERBY-5233-1.stat
        0.2 kB
        Dag H. Wanvik
      4. DERBY-5233-1.diff
        9 kB
        Dag H. Wanvik
      5. Repro5233.java
        2 kB
        Dag H. Wanvik

        Issue Links

          Activity

          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.
          Hide
          Dag H. Wanvik added a comment -

          Backported to 10.8 branch as svn 1129769, resolving.

          Show
          Dag H. Wanvik added a comment - Backported to 10.8 branch as svn 1129769, resolving.
          Hide
          Dag H. Wanvik added a comment -

          Committed as svn 1129764.

          Show
          Dag H. Wanvik added a comment - Committed as svn 1129764.
          Hide
          Dag H. Wanvik added a comment -

          Uploading version #2.

          Show
          Dag H. Wanvik added a comment - Uploading version #2.
          Hide
          Dag H. Wanvik added a comment -

          Thanks for looking at the patch, Knut! Re a) Actually, I think it gets less clear if I change it around: the normal case is that we'll not loop, indicated by the early setting of the success flag. Only exceptionally will we loop using a continue, so I think I prefer setting success = false there. I'd rather burden the abnormal case with extra logic than the normal case.

          Show
          Dag H. Wanvik added a comment - Thanks for looking at the patch, Knut! Re a) Actually, I think it gets less clear if I change it around: the normal case is that we'll not loop, indicated by the early setting of the success flag. Only exceptionally will we loop using a continue, so I think I prefer setting success = false there. I'd rather burden the abnormal case with extra logic than the normal case.
          Hide
          Dag H. Wanvik added a comment -

          a) I agree it is awkward, I was following a preexisting pattern - I'll change both. b) Yes, I already corrected one of those, I apparently missed one, thanks.

          Show
          Dag H. Wanvik added a comment - a) I agree it is awkward, I was following a preexisting pattern - I'll change both. b) Yes, I already corrected one of those, I apparently missed one, thanks.
          Hide
          Knut Anders Hatlen added a comment -

          The patch looks fine to me. Two minor comments:

          • The use of the success flag feels a little backwards. Perhaps it would be clearer if we only set it to true if the operation has succeeded?
          • A comment in testInterruptShutdown() says "Assert and clear thread's flag:", but the code just below the comment doesn't clear the flag anymore. (It is cleared further down, so only the comment needs to be updated.)
          Show
          Knut Anders Hatlen added a comment - The patch looks fine to me. Two minor comments: The use of the success flag feels a little backwards. Perhaps it would be clearer if we only set it to true if the operation has succeeded? A comment in testInterruptShutdown() says "Assert and clear thread's flag:", but the code just below the comment doesn't clear the flag anymore. (It is cleared further down, so only the comment needs to be updated.)
          Hide
          Dag H. Wanvik added a comment -

          Uploading a patch for this problem. I also adds a new test case to InterruptResilienceTest: testCreateDropInterrupted.

          The patch makes RAFContainer, when seeing an interrupt exception during container creation, close it down and try again, up to MAX_INTERRUPT_RETRIES times. Since RAFContainer should work also under CDC/Foundation 1.1, the exceptions are checked using reflection (NIO classes are excluded there).

          Running regressions.

          Show
          Dag H. Wanvik added a comment - Uploading a patch for this problem. I also adds a new test case to InterruptResilienceTest: testCreateDropInterrupted. The patch makes RAFContainer, when seeing an interrupt exception during container creation, close it down and try again, up to MAX_INTERRUPT_RETRIES times. Since RAFContainer should work also under CDC/Foundation 1.1, the exceptions are checked using reflection (NIO classes are excluded there). Running regressions.
          Hide
          Dag H. Wanvik added a comment -

          Setting "seen in production" flag, cf. duplicated issue DERBY-5237 posted by a user.

          Show
          Dag H. Wanvik added a comment - Setting "seen in production" flag, cf. duplicated issue DERBY-5237 posted by a user.

            People

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

              Dates

              • Created:
                Updated:
                Resolved:

                Development