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-1.patch
        7 kB
        Hari Shreedharan
      2. SQOOP-690-2.patch
        8 kB
        Hari Shreedharan
      3. SQOOP-690-3.patch
        12 kB
        Hari Shreedharan

        Issue Links

          Activity

          Hari Shreedharan created issue -
          Hide
          Hari Shreedharan added a comment -

          Diff r2 from reviewboard

          Show
          Hari Shreedharan added a comment - Diff r2 from reviewboard
          Hari Shreedharan made changes -
          Field Original Value New Value
          Attachment SQOOP-690-1.patch [ 12553602 ]
          Hari Shreedharan made changes -
          Remote Link This issue links to "Reviewboard (Web Link)" [ 11444 ]
          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).
          Hari Shreedharan made changes -
          Attachment SQOOP-690-2.patch [ 12553758 ]
          Hari Shreedharan made changes -
          Status Open [ 1 ] Patch Available [ 10002 ]
          Hide
          Hari Shreedharan added a comment -

          Rewrote the patch using Semaphores.

          Show
          Hari Shreedharan added a comment - Rewrote the patch using Semaphores.
          Hari Shreedharan made changes -
          Attachment SQOOP-690-3.patch [ 12553809 ]
          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
          Jarek Jarcec Cecho made changes -
          Status Patch Available [ 10002 ] Resolved [ 5 ]
          Resolution Fixed [ 1 ]
          Jarek Jarcec Cecho made changes -
          Fix Version/s 1.99.1 [ 12323641 ]
          Fix Version/s 2.0.0 [ 12319272 ]
          Transition Time In Source Status Execution Times Last Executer Last Execution Date
          Open Open Patch Available Patch Available
          6d 3h 48m 1 Hari Shreedharan 16/Nov/12 09:48
          Patch Available Patch Available Resolved Resolved
          10h 22m 1 Jarek Jarcec Cecho 16/Nov/12 20:11

            People

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

              Dates

              • Created:
                Updated:
                Resolved:

                Development