Hive
  1. Hive
  2. HIVE-7281

DbTxnManager acquiring wrong level of lock for dynamic partitioning

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.14.0
    • Component/s: Locking, Transactions
    • Labels:
      None

      Description

      Currently DbTxnManager.acquireLocks() locks the DUMMY_PARTITION for dynamic partitioning. But this is not adequate. This will not prevent drop operations on partitions being written to. The lock should be at the table level.

        Activity

        Hide
        Alan Gates added a comment -

        Ideally we could lock only partitions that are potentially being written to by the dynamic partitioning write. But this would require significant changes in the TxnHandler to handle the notion of locking partitions matching a pattern. Rather than make those changes, for now it is easier to lock the table.

        Show
        Alan Gates added a comment - Ideally we could lock only partitions that are potentially being written to by the dynamic partitioning write. But this would require significant changes in the TxnHandler to handle the notion of locking partitions matching a pattern. Rather than make those changes, for now it is easier to lock the table.
        Hide
        Alan Gates added a comment -

        Attaching a patch that locks at the table level rather than locking the dummy partition (as before).

        Show
        Alan Gates added a comment - Attaching a patch that locks at the table level rather than locking the dummy partition (as before).
        Hide
        Hive QA added a comment -

        Overall: -1 at least one tests failed

        Here are the results of testing the latest attachment:
        https://issues.apache.org/jira/secure/attachment/12659090/HIVE-7281.patch

        ERROR: -1 due to 1 failed/errored test(s), 5849 tests executed
        Failed tests:

        org.apache.hadoop.hive.cli.TestMinimrCliDriver.testCliDriver_ql_rewrite_gbtoidx
        

        Test results: http://ec2-174-129-184-35.compute-1.amazonaws.com/jenkins/job/PreCommit-HIVE-TRUNK-Build/135/testReport
        Console output: http://ec2-174-129-184-35.compute-1.amazonaws.com/jenkins/job/PreCommit-HIVE-TRUNK-Build/135/console
        Test logs: http://ec2-174-129-184-35.compute-1.amazonaws.com/logs/PreCommit-HIVE-TRUNK-Build-135/

        Messages:

        Executing org.apache.hive.ptest.execution.PrepPhase
        Executing org.apache.hive.ptest.execution.ExecutionPhase
        Executing org.apache.hive.ptest.execution.ReportingPhase
        Tests exited with: TestsFailedException: 1 tests failed
        

        This message is automatically generated.

        ATTACHMENT ID: 12659090

        Show
        Hive QA added a comment - Overall : -1 at least one tests failed Here are the results of testing the latest attachment: https://issues.apache.org/jira/secure/attachment/12659090/HIVE-7281.patch ERROR: -1 due to 1 failed/errored test(s), 5849 tests executed Failed tests: org.apache.hadoop.hive.cli.TestMinimrCliDriver.testCliDriver_ql_rewrite_gbtoidx Test results: http://ec2-174-129-184-35.compute-1.amazonaws.com/jenkins/job/PreCommit-HIVE-TRUNK-Build/135/testReport Console output: http://ec2-174-129-184-35.compute-1.amazonaws.com/jenkins/job/PreCommit-HIVE-TRUNK-Build/135/console Test logs: http://ec2-174-129-184-35.compute-1.amazonaws.com/logs/PreCommit-HIVE-TRUNK-Build-135/ Messages: Executing org.apache.hive.ptest.execution.PrepPhase Executing org.apache.hive.ptest.execution.ExecutionPhase Executing org.apache.hive.ptest.execution.ReportingPhase Tests exited with: TestsFailedException: 1 tests failed This message is automatically generated. ATTACHMENT ID: 12659090
        Hide
        Ashutosh Chauhan added a comment -

        Patch looks fine.
        But, I wonder if Entity type of DummyPartition make sense at all. It seems this entity is created only in DP case to be used for locking and authorization purposes. And since in locking case (as argued in this ticket) as well as auth case (probably) it make sense to use Table entity. I dont see what useful purpose DummyPartition serves. On the contrary, it results in confusion like the topic of this jira. Shall we just delete this DummyPartition entity. cc: Thejas M Nair

        Show
        Ashutosh Chauhan added a comment - Patch looks fine. But, I wonder if Entity type of DummyPartition make sense at all. It seems this entity is created only in DP case to be used for locking and authorization purposes. And since in locking case (as argued in this ticket) as well as auth case (probably) it make sense to use Table entity. I dont see what useful purpose DummyPartition serves. On the contrary, it results in confusion like the topic of this jira. Shall we just delete this DummyPartition entity. cc: Thejas M Nair
        Hide
        Alan Gates added a comment -

        I'm fine with doing that, but do we need to link that change to this? Can we file a separate JIRA for that?

        Show
        Alan Gates added a comment - I'm fine with doing that, but do we need to link that change to this? Can we file a separate JIRA for that?
        Hide
        Ashutosh Chauhan added a comment -

        Filing a separate ticket and unlinking it from this one is fine, only that it will be left to the mercy of someone who will do it : )
        Fixing a root cause is always better (in this case deleting error-prone DummyPartitions) but since immediate bug can be fixed by current patch. +1

        Show
        Ashutosh Chauhan added a comment - Filing a separate ticket and unlinking it from this one is fine, only that it will be left to the mercy of someone who will do it : ) Fixing a root cause is always better (in this case deleting error-prone DummyPartitions) but since immediate bug can be fixed by current patch. +1
        Hide
        Ashutosh Chauhan added a comment -

        Committed to trunk. Thanks, Alan!

        Show
        Ashutosh Chauhan added a comment - Committed to trunk. Thanks, Alan!
        Hide
        Thejas M Nair added a comment -

        This has been fixed in 0.14 release. Please open new jira if you see any issues.

        Show
        Thejas M Nair added a comment - This has been fixed in 0.14 release. Please open new jira if you see any issues.

          People

          • Assignee:
            Alan Gates
            Reporter:
            Alan Gates
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development