Sqoop
  1. Sqoop
  2. SQOOP-690

Fix threading issues in SqoopOutputFormatLoadExecutor

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.0.0
    • Fix Version/s: 1.99.1
    • Component/s: None
    • Labels:
      None

      Description

      The threading uses synchronized and wait/notify in this class. wait() is called outside while loops too. We can simply this using higher level concurrency constructs like semaphores or latches.

      1. SQOOP-690-3.patch
        12 kB
        Hari Shreedharan
      2. SQOOP-690-2.patch
        8 kB
        Hari Shreedharan
      3. SQOOP-690-1.patch
        7 kB
        Hari Shreedharan

        Issue Links

          Activity

          Hide
          Hari Shreedharan added a comment -

          Diff r2 from reviewboard

          Show
          Hari Shreedharan added a comment - Diff r2 from reviewboard
          Hide
          Hari Shreedharan added a comment -

          Changes in the patch:

          • SqoopOutputFormatLoadExecutor essentially has one reader and one writer. So use counts and yields to make sure the reader waits for the writer and vice versa.
          • Some exception handling was incorrect earlier - fixed that (instead of caching the exception, it was being rethrown - without setting readerFinished to true.
          • Made the internal content of Data class volatile, since it is essentially immutable - volatile is enough, no synchronization is needed (only for safe publication, not for any thread handling).
          Show
          Hari Shreedharan added a comment - Changes in the patch: SqoopOutputFormatLoadExecutor essentially has one reader and one writer. So use counts and yields to make sure the reader waits for the writer and vice versa. Some exception handling was incorrect earlier - fixed that (instead of caching the exception, it was being rethrown - without setting readerFinished to true. Made the internal content of Data class volatile, since it is essentially immutable - volatile is enough, no synchronization is needed (only for safe publication, not for any thread handling).
          Hide
          Hari Shreedharan added a comment -

          Rewrote the patch using Semaphores.

          Show
          Hari Shreedharan added a comment - Rewrote the patch using Semaphores.
          Hide
          Hari Shreedharan added a comment -

          Thanks for your feedback on rb, Jarcec. Attached is the new patch.

          Show
          Hari Shreedharan added a comment - Thanks for your feedback on rb, Jarcec. Attached is the new patch.
          Hide
          Jarek Jarcec Cecho added a comment -
          Show
          Jarek Jarcec Cecho added a comment - Patch is in: https://git-wip-us.apache.org/repos/asf?p=sqoop.git;a=commit;h=e5ab9a4f3456f74092c1ac7335a236cbd8103f69 Thank you Hari for your contribution! Jarcec

            People

            • Assignee:
              Hari Shreedharan
              Reporter:
              Hari Shreedharan
            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development